-
Notifications
You must be signed in to change notification settings - Fork 441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide a setting to use a different REST url during SSR execution #3358
base: main
Are you sure you want to change the base?
Changes from all commits
8ec8eed
268cf64
a1cbe3b
a9223ea
ed4e794
c12a3e6
c8694e1
428f859
dae1e68
1ee754e
6b9bc66
c35964a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,11 +19,32 @@ ui: | |
|
||
# Angular Server Side Rendering (SSR) settings | ||
ssr: | ||
# A boolean flag indicating whether the SSR configuration is enabled | ||
# Defaults to true. | ||
enabled: boolean; | ||
|
||
# Enable request performance profiling data collection and printing the results in the server console. | ||
# Defaults to false. | ||
enablePerformanceProfiler: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please change
Also is there any risk in enabling this in production? This sounds like a debugging tool, so we may want to mention that we don't recommend enabling in production |
||
|
||
# Whether to tell Angular to inline "critical" styles into the server-side rendered HTML. | ||
# Determining which styles are critical is a relatively expensive operation; this option is | ||
# disabled (false) by default to boost server performance at the expense of loading smoothness. | ||
inlineCriticalCss: false | ||
|
||
# Enable state transfer from the server-side application to the client-side application. | ||
# Defaults to true. | ||
# Note: When using an external application cache layer, it's recommended not to transfer the state to avoid caching it. | ||
# Disabling it ensures that dynamic state information is not inadvertently cached, which can improve security and | ||
# ensure that users always use the most up-to-date state. | ||
transferState: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, this should have the default value listed:
|
||
|
||
# When a different REST base URL is used for the server-side application, the generated state contains references to | ||
# REST resources with the internal URL configured, so it is not transferred to the client application, by default. | ||
# Enabling this setting transfers the state to the client application and replaces internal URLs with the public | ||
# URLs used by the client application. | ||
replaceRestUrl: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, this should have the default value listed:
Why is this Based on the discussion with @ybnd above , it sounds like setting this to (I like how for |
||
|
||
# The REST API server settings | ||
# NOTE: these settings define which (publicly available) REST API to use. They are usually | ||
# 'synced' with the 'dspace.server.url' setting in your backend's local.cfg. | ||
|
@@ -33,6 +54,9 @@ rest: | |
port: 443 | ||
# NOTE: Space is capitalized because 'namespace' is a reserved string in TypeScript | ||
nameSpace: /server | ||
# Provide a different REST url to be used during SSR execution. It must contain the whole url including protocol, server port and | ||
# server namespace (uncomment to use it). | ||
#ssrBaseUrl: http://localhost:8080/server | ||
|
||
# Caching settings | ||
cache: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,194 @@ | ||
import { | ||
HTTP_INTERCEPTORS, | ||
HttpClient, | ||
} from '@angular/common/http'; | ||
import { | ||
HttpClientTestingModule, | ||
HttpTestingController, | ||
} from '@angular/common/http/testing'; | ||
import { PLATFORM_ID } from '@angular/core'; | ||
import { TestBed } from '@angular/core/testing'; | ||
|
||
import { | ||
APP_CONFIG, | ||
AppConfig, | ||
} from '../../../config/app-config.interface'; | ||
import { DspaceRestInterceptor } from './dspace-rest.interceptor'; | ||
import { DspaceRestService } from './dspace-rest.service'; | ||
|
||
describe('DspaceRestInterceptor', () => { | ||
let httpMock: HttpTestingController; | ||
let httpClient: HttpClient; | ||
const appConfig: Partial<AppConfig> = { | ||
rest: { | ||
ssl: false, | ||
host: 'localhost', | ||
port: 8080, | ||
nameSpace: '/server', | ||
baseUrl: 'http://api.example.com/server', | ||
}, | ||
}; | ||
const appConfigWithSSR: Partial<AppConfig> = { | ||
rest: { | ||
ssl: false, | ||
host: 'localhost', | ||
port: 8080, | ||
nameSpace: '/server', | ||
baseUrl: 'http://api.example.com/server', | ||
ssrBaseUrl: 'http://ssr.example.com/server', | ||
}, | ||
}; | ||
|
||
describe('When SSR base URL is not set ', () => { | ||
describe('and it\'s in the browser', () => { | ||
beforeEach(() => { | ||
TestBed.configureTestingModule({ | ||
imports: [HttpClientTestingModule], | ||
providers: [ | ||
DspaceRestService, | ||
{ | ||
provide: HTTP_INTERCEPTORS, | ||
useClass: DspaceRestInterceptor, | ||
multi: true, | ||
}, | ||
{ provide: APP_CONFIG, useValue: appConfig }, | ||
{ provide: PLATFORM_ID, useValue: 'browser' }, | ||
], | ||
}); | ||
|
||
httpMock = TestBed.inject(HttpTestingController); | ||
httpClient = TestBed.inject(HttpClient); | ||
}); | ||
|
||
it('should not modify the request', () => { | ||
const url = 'http://api.example.com/server/items'; | ||
httpClient.get(url).subscribe((response) => { | ||
expect(response).toBeTruthy(); | ||
}); | ||
|
||
const req = httpMock.expectOne(url); | ||
expect(req.request.url).toBe(url); | ||
req.flush({}); | ||
httpMock.verify(); | ||
}); | ||
}); | ||
|
||
describe('and it\'s in SSR mode', () => { | ||
beforeEach(() => { | ||
TestBed.configureTestingModule({ | ||
imports: [HttpClientTestingModule], | ||
providers: [ | ||
DspaceRestService, | ||
{ | ||
provide: HTTP_INTERCEPTORS, | ||
useClass: DspaceRestInterceptor, | ||
multi: true, | ||
}, | ||
{ provide: APP_CONFIG, useValue: appConfig }, | ||
{ provide: PLATFORM_ID, useValue: 'server' }, | ||
], | ||
}); | ||
|
||
httpMock = TestBed.inject(HttpTestingController); | ||
httpClient = TestBed.inject(HttpClient); | ||
}); | ||
|
||
it('should not replace the base URL', () => { | ||
const url = 'http://api.example.com/server/items'; | ||
|
||
httpClient.get(url).subscribe((response) => { | ||
expect(response).toBeTruthy(); | ||
}); | ||
|
||
const req = httpMock.expectOne(url); | ||
expect(req.request.url).toBe(url); | ||
req.flush({}); | ||
httpMock.verify(); | ||
}); | ||
}); | ||
}); | ||
|
||
describe('When SSR base URL is set ', () => { | ||
describe('and it\'s in the browser', () => { | ||
beforeEach(() => { | ||
TestBed.configureTestingModule({ | ||
imports: [HttpClientTestingModule], | ||
providers: [ | ||
DspaceRestService, | ||
{ | ||
provide: HTTP_INTERCEPTORS, | ||
useClass: DspaceRestInterceptor, | ||
multi: true, | ||
}, | ||
{ provide: APP_CONFIG, useValue: appConfigWithSSR }, | ||
{ provide: PLATFORM_ID, useValue: 'browser' }, | ||
], | ||
}); | ||
|
||
httpMock = TestBed.inject(HttpTestingController); | ||
httpClient = TestBed.inject(HttpClient); | ||
}); | ||
|
||
it('should not modify the request', () => { | ||
const url = 'http://api.example.com/server/items'; | ||
httpClient.get(url).subscribe((response) => { | ||
expect(response).toBeTruthy(); | ||
}); | ||
|
||
const req = httpMock.expectOne(url); | ||
expect(req.request.url).toBe(url); | ||
req.flush({}); | ||
httpMock.verify(); | ||
}); | ||
}); | ||
|
||
describe('and it\'s in SSR mode', () => { | ||
beforeEach(() => { | ||
TestBed.configureTestingModule({ | ||
imports: [HttpClientTestingModule], | ||
providers: [ | ||
DspaceRestService, | ||
{ | ||
provide: HTTP_INTERCEPTORS, | ||
useClass: DspaceRestInterceptor, | ||
multi: true, | ||
}, | ||
{ provide: APP_CONFIG, useValue: appConfigWithSSR }, | ||
{ provide: PLATFORM_ID, useValue: 'server' }, | ||
], | ||
}); | ||
|
||
httpMock = TestBed.inject(HttpTestingController); | ||
httpClient = TestBed.inject(HttpClient); | ||
}); | ||
|
||
it('should replace the base URL', () => { | ||
const url = 'http://api.example.com/server/items'; | ||
const ssrBaseUrl = appConfigWithSSR.rest.ssrBaseUrl; | ||
|
||
httpClient.get(url).subscribe((response) => { | ||
expect(response).toBeTruthy(); | ||
}); | ||
|
||
const req = httpMock.expectOne(ssrBaseUrl + '/items'); | ||
expect(req.request.url).toBe(ssrBaseUrl + '/items'); | ||
req.flush({}); | ||
httpMock.verify(); | ||
}); | ||
|
||
it('should not replace any query param containing the base URL', () => { | ||
const url = 'http://api.example.com/server/items?url=http://api.example.com/server/item/1'; | ||
const ssrBaseUrl = appConfigWithSSR.rest.ssrBaseUrl; | ||
|
||
httpClient.get(url).subscribe((response) => { | ||
expect(response).toBeTruthy(); | ||
}); | ||
|
||
const req = httpMock.expectOne(ssrBaseUrl + '/items?url=http://api.example.com/server/item/1'); | ||
expect(req.request.url).toBe(ssrBaseUrl + '/items?url=http://api.example.com/server/item/1'); | ||
req.flush({}); | ||
httpMock.verify(); | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import { isPlatformBrowser } from '@angular/common'; | ||
import { | ||
HttpEvent, | ||
HttpHandler, | ||
HttpInterceptor, | ||
HttpRequest, | ||
} from '@angular/common/http'; | ||
import { | ||
Inject, | ||
Injectable, | ||
PLATFORM_ID, | ||
} from '@angular/core'; | ||
import { Observable } from 'rxjs'; | ||
|
||
import { | ||
APP_CONFIG, | ||
AppConfig, | ||
} from '../../../config/app-config.interface'; | ||
import { isEmpty } from '../../shared/empty.util'; | ||
|
||
@Injectable() | ||
/** | ||
* This Interceptor is used to use the configured base URL for the request made during SSR execution | ||
*/ | ||
export class DspaceRestInterceptor implements HttpInterceptor { | ||
|
||
/** | ||
* Contains the configured application base URL | ||
* @protected | ||
*/ | ||
protected baseUrl: string; | ||
protected ssrBaseUrl: string; | ||
|
||
constructor( | ||
@Inject(APP_CONFIG) protected appConfig: AppConfig, | ||
@Inject(PLATFORM_ID) private platformId: string, | ||
) { | ||
this.baseUrl = this.appConfig.rest.baseUrl; | ||
this.ssrBaseUrl = this.appConfig.rest.ssrBaseUrl; | ||
} | ||
|
||
intercept(request: HttpRequest<unknown>, next: HttpHandler): Observable<HttpEvent<unknown>> { | ||
if (isPlatformBrowser(this.platformId) || isEmpty(this.ssrBaseUrl) || this.baseUrl === this.ssrBaseUrl) { | ||
return next.handle(request); | ||
} | ||
|
||
// Different SSR Base URL specified so replace it in the current request url | ||
const url = request.url.replace(this.baseUrl, this.ssrBaseUrl); | ||
const newRequest: HttpRequest<any> = request.clone({ url }); | ||
return next.handle(newRequest); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this. While it is possible to disable SSR in this manner, I don't think we should encourage anyone to disable SSR entirely -- it will have a detrimental effect on SEO.
So, this
enabled
setting was purposefully excluded fromconfig.example.yml
to discourage anyone from disabling SSR entirely.