Fix the causes of the self link mismatch console warnings

src/app/admin/admin-registries/registry/registry.service.ts

@@ -1,6 +1,9 @@
 import { Injectable } from '@angular/core';
 import { RequestParam } from '@dspace/core/cache/models/request-param.model';
-import { FindListOptions } from '@dspace/core/data/find-list-options.model';
+import {
+  FindListOptions,
+  MAX_PAGE_SIZE,
+} from '@dspace/core/data/find-list-options.model';
 import { MetadataFieldDataService } from '@dspace/core/data/metadata-field-data.service';
 import { MetadataSchemaDataService } from '@dspace/core/data/metadata-schema-data.service';
 import { PaginatedList } from '@dspace/core/data/paginated-list.model';
@@ -94,7 +97,7 @@ export class RegistryService {
   public getMetadataSchemaByPrefix(prefix: string, useCachedVersionIfAvailable = true, reRequestOnStale = true, ...linksToFollow: FollowLinkConfig<MetadataSchema>[]): Observable<RemoteData<MetadataSchema>> {
     // Temporary options to get ALL metadataschemas until there's a rest api endpoint for fetching a specific schema
     const options: FindListOptions = Object.assign(new FindListOptions(), {
-      elementsPerPage: 10000,
+      elementsPerPage: MAX_PAGE_SIZE,
     });
     return this.getMetadataSchemas(options).pipe(
       getFirstSucceededRemoteDataPayload(),

src/app/admin/admin-reports/filtered-items/filtered-items.component.ts

@@ -15,6 +15,7 @@ import { RestRequestMethod } from '@dspace/config/rest-request-method';
 import { CollectionDataService } from '@dspace/core/data/collection-data.service';
 import { CommunityDataService } from '@dspace/core/data/community-data.service';
 import { AuthorizationDataService } from '@dspace/core/data/feature-authorization/authorization-data.service';
+import { MAX_PAGE_SIZE } from '@dspace/core/data/find-list-options.model';
 import { MetadataFieldDataService } from '@dspace/core/data/metadata-field-data.service';
 import { MetadataSchemaDataService } from '@dspace/core/data/metadata-schema-data.service';
 import { ScriptDataService } from '@dspace/core/data/processes/script-data.service';
@@ -135,15 +136,15 @@ export class FilteredItemsComponent implements OnInit {
     const wholeRepo$ = this.translateService.stream('admin.reports.items.wholeRepo');
     this.collections.push(OptionVO.collectionLoc('', wholeRepo$));
 
-    this.communityService.findAll({ elementsPerPage: 10000, currentPage: 1 }).pipe(
+    this.communityService.findAll({ elementsPerPage: MAX_PAGE_SIZE, currentPage: 1 }).pipe(
       getFirstSucceededRemoteListPayload(),
     ).subscribe(
       (communitiesRest: Community[]) => {
         communitiesRest.forEach(community => {
           const commVO = OptionVO.collection(community.uuid, community.name, true);
           this.collections.push(commVO);
 
-          this.collectionService.findByParent(community.uuid, { elementsPerPage: 10000, currentPage: 1 }).pipe(
+          this.collectionService.findByParent(community.uuid, { elementsPerPage: MAX_PAGE_SIZE, currentPage: 1 }).pipe(
             getFirstSucceededRemoteListPayload(),
           ).subscribe(
             (collectionsRest: Collection[]) => {
@@ -207,12 +208,12 @@ export class FilteredItemsComponent implements OnInit {
     this.metadataFieldsWithAny = [];
     const anyField$ = this.translateService.stream('admin.reports.items.anyField');
     this.metadataFieldsWithAny.push(OptionVO.itemLoc('*', anyField$));
-    this.metadataSchemaService.findAll({ elementsPerPage: 10000, currentPage: 1 }).pipe(
+    this.metadataSchemaService.findAll({ elementsPerPage: MAX_PAGE_SIZE, currentPage: 1 }).pipe(
       getFirstSucceededRemoteListPayload(),
     ).subscribe(
       (schemasRest: MetadataSchema[]) => {
         schemasRest.forEach(schema => {
-          this.metadataFieldService.findBySchema(schema, { elementsPerPage: 10000, currentPage: 1 }).pipe(
+          this.metadataFieldService.findBySchema(schema, { elementsPerPage: MAX_PAGE_SIZE, currentPage: 1 }).pipe(
             getFirstSucceededRemoteListPayload(),
           ).subscribe(
             (fieldsRest: MetadataField[]) => {

src/app/core/browse/browse.service.ts

@@ -20,6 +20,7 @@ import {
 } from 'rxjs/operators';
 
 import { SortDirection } from '../cache/models/sort-options.model';
+import { MAX_PAGE_SIZE } from '../data/find-list-options.model';
 import { HrefOnlyDataService } from '../data/href-only-data.service';
 import { PaginatedList } from '../data/paginated-list.model';
 import { RemoteData } from '../data/remote-data';
@@ -88,7 +89,7 @@ export class BrowseService {
    */
   getBrowseDefinitions(): Observable<RemoteData<PaginatedList<BrowseDefinition>>> {
     // TODO properly support pagination
-    return this.browseDefinitionDataService.findAll({ elementsPerPage: 9999 }).pipe(
+    return this.browseDefinitionDataService.findAll({ elementsPerPage: MAX_PAGE_SIZE }).pipe(
       getFirstSucceededRemoteData(),
     );
   }

src/app/core/data/bundle-data.service.ts

@@ -25,7 +25,10 @@ import {
   PatchDataImpl,
 } from './base/patch-data';
 import { DSOChangeAnalyzer } from './dso-change-analyzer.service';
-import { FindListOptions } from './find-list-options.model';
+import {
+  FindListOptions,
+  MAX_PAGE_SIZE,
+} from './find-list-options.model';
 import { PaginatedList } from './paginated-list.model';
 import { RemoteData } from './remote-data';
 import { GetRequest } from './request.models';
@@ -87,7 +90,7 @@ export class BundleDataService extends IdentifiableDataService<Bundle> implement
   findByItemAndName(item: Item, bundleName: string, useCachedVersionIfAvailable = true, reRequestOnStale = true, options?: FindListOptions, ...linksToFollow: FollowLinkConfig<Bundle>[]): Observable<RemoteData<Bundle>> {
     //Since we filter by bundleName where the pagination options are not indicated we need to load all the possible bundles.
     // This is a workaround, in substitution of the previously recursive call with expand
-    const paginationOptions = options ?? { elementsPerPage: 9999 };
+    const paginationOptions = options ?? { elementsPerPage: MAX_PAGE_SIZE };
     return this.findAllByItem(item, paginationOptions, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow).pipe(
       map((rd: RemoteData<PaginatedList<Bundle>>) => {
         if (hasValue(rd.payload) && hasValue(rd.payload.page)) {

src/app/core/data/dspace-rest-response-parsing.service.spec.ts

@@ -0,0 +1,260 @@
+import { Injectable } from '@angular/core';
+import { TestBed } from '@angular/core/testing';
+import { APP_CONFIG } from '@dspace/config/app-config.interface';
+
+import { ObjectCacheService } from '../cache/object-cache.service';
+import { RawRestResponse } from '../dspace-rest/raw-rest-response.model';
+import { getMockObjectCacheService } from '../testing/object-cache.service.mock';
+import { DspaceRestResponseParsingService } from './dspace-rest-response-parsing.service';
+import {
+  GetRequest,
+  PostRequest,
+} from './request.models';
+import { RestRequest } from './rest-request.model';
+
+/**
+ * Exposes the protected {@link DspaceRestResponseParsingService#ensureSelfLink} so it can be
+ * tested in isolation.
+ */
+@Injectable()
+class TestService extends DspaceRestResponseParsingService {
+  public callEnsureSelfLink(request: RestRequest, response: RawRestResponse): RawRestResponse {
+    return this.ensureSelfLink(request, response);
+  }
+}
+
+describe('DspaceRestResponseParsingService', () => {
+  let service: TestService;
+
+  const MISMATCH = jasmine.stringMatching(/These don't match/);
+  const REDUCED_PAGE = jasmine.stringMatching(/asked for a page of 9999 elements, but the REST API served 1000/);
+  const NO_SELF_LINK = jasmine.stringMatching(/doesn't have a self link/);
+
+  const requestFor = (href: string): RestRequest =>
+    new GetRequest('c4f0b1b7-3ffa-4b1a-9f5f-8bd6b1c4de71', href);
+
+  const responseWithSelfLink = (href: string, page?: any): RawRestResponse => ({
+    payload: {
+      _links: {
+        self: { href },
+      },
+      ...(page ? { page } : {}),
+    },
+    statusCode: 200,
+    statusText: 'OK',
+  });
+
+  beforeEach(() => {
+    TestBed.configureTestingModule({
+      providers: [
+        { provide: ObjectCacheService, useValue: getMockObjectCacheService() },
+        { provide: APP_CONFIG, useValue: { rest: { baseUrl: 'https://rest.api' } } },
+        TestService,
+      ],
+    });
+    service = TestBed.inject(TestService);
+    spyOn(console, 'warn');
+  });
+
+  describe('ensureSelfLink', () => {
+
+    describe('differences the REST API is expected to introduce', () => {
+
+      it('should not warn when the self link matches the requested url', () => {
+        const href = 'https://rest.api/core/bundles/9d18168a/bitstreams?page=0&size=5';
+        const response = service.callEnsureSelfLink(requestFor(href), responseWithSelfLink(href));
+
+        expect(console.warn).not.toHaveBeenCalled();
+        expect(response.payload._links.self.href).toBe(href);
+      });
+
+      it('should not warn when the self link only echoes the embed params of the request', () => {
+        const href = 'https://rest.api/core/bundles/9d18168a/bitstreams?page=0&embed=accessStatus&size=5';
+        const response = service.callEnsureSelfLink(requestFor(href), responseWithSelfLink(href));
+
+        expect(console.warn).not.toHaveBeenCalled();
+        // the self link is still normalized, because that's the url the response is cached under
+        expect(response.payload._links.self.href).toBe('https://rest.api/core/bundles/9d18168a/bitstreams?page=0&size=5');
+      });
+
+      it('should not warn when the self link echoes embed params and the request has no other params', () => {
+        const href = 'https://rest.api/core/items/eba1c085/bundles?embed=primaryBitstream&embed=bitstreams/format&embed.size=bitstreams=5';
+        const response = service.callEnsureSelfLink(requestFor(href), responseWithSelfLink(href));
+
+        expect(console.warn).not.toHaveBeenCalled();
+        expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085/bundles');
+      });
+
+      it('should not warn when the self link only percent decoded a param value', () => {
+        const request = requestFor('https://rest.api/statistics/usagereports/search/object?page=-1&size=10&uri=https%3A%2F%2Frest.api%2Fcore%2Fsites%2F8f842a80');
+        const response = service.callEnsureSelfLink(request, responseWithSelfLink(
+          'https://rest.api/statistics/usagereports/search/object?page=-1&size=10&uri=https://rest.api/core/sites/8f842a80'));
+
+        expect(console.warn).not.toHaveBeenCalled();
+        expect(response.payload._links.self.href)
+          .toBe('https://rest.api/statistics/usagereports/search/object?page=-1&size=10&uri=https%3A%2F%2Frest.api%2Fcore%2Fsites%2F8f842a80');
+      });
+
+      it('should not warn or normalize when params are only in a different order', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?page=0&size=5');
+        const response = service.callEnsureSelfLink(request,
+          responseWithSelfLink('https://rest.api/core/items/eba1c085/bundles?size=5&page=0'));
+
+        expect(console.warn).not.toHaveBeenCalled();
+        // the urls hold the same params, so nothing is rewritten here
+        expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085/bundles?size=5&page=0');
+      });
+
+    });
+
+    describe('differences that point at a problem with the endpoint', () => {
+
+      it('should say so when the REST API reduced the requested page size', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=9999');
+        service.callEnsureSelfLink(request, responseWithSelfLink(
+          'https://rest.api/core/items/eba1c085/bundles?size=1000',
+          { number: 0, size: 1000, totalPages: 1, totalElements: 2 }));
+
+        expect(console.warn).toHaveBeenCalledTimes(1);
+        expect(console.warn).toHaveBeenCalledWith(REDUCED_PAGE);
+      });
+
+      it('should report a reduced page size alongside other params without confusing the two', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?page=0&size=9999&sort=name,ASC');
+        service.callEnsureSelfLink(request, responseWithSelfLink(
+          'https://rest.api/core/items/eba1c085/bundles?page=0&size=1000&sort=name,ASC'));
+
+        expect(console.warn).toHaveBeenCalledTimes(1);
+        expect(console.warn).toHaveBeenCalledWith(REDUCED_PAGE);
+      });
+
+      it('should fall back to the generic warning when more than the page size differs', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?page=0&size=9999');
+        service.callEnsureSelfLink(request, responseWithSelfLink(
+          'https://rest.api/core/items/eba1c085/bundles?page=3&size=1000'));
+
+        expect(console.warn).toHaveBeenCalledTimes(1);
+        expect(console.warn).toHaveBeenCalledWith(MISMATCH);
+      });
+
+      it('should use the generic warning when the returned page size is larger than requested', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=5');
+        service.callEnsureSelfLink(request, responseWithSelfLink(
+          'https://rest.api/core/items/eba1c085/bundles?size=50',
+          { number: 0, size: 50, totalPages: 1, totalElements: 2 }));
+
+        expect(console.warn).toHaveBeenCalledTimes(1);
+        expect(console.warn).toHaveBeenCalledWith(MISMATCH);
+      });
+
+      it('should still warn when a param value differs beyond its encoding', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?uri=https%3A%2F%2Frest.api%2Fcore%2Fsites%2Faaa');
+        service.callEnsureSelfLink(request,
+          responseWithSelfLink('https://rest.api/core/items/eba1c085/bundles?uri=https://rest.api/core/sites/bbb'));
+
+        expect(console.warn).toHaveBeenCalledTimes(1);
+        expect(console.warn).toHaveBeenCalledWith(MISMATCH);
+      });
+
+      it('should report the normalized request url and the raw self link in the warning', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?page=0&embed=primaryBitstream&size=5');
+        service.callEnsureSelfLink(request,
+          responseWithSelfLink('https://rest.api/core/items/eba1c085/bundles?page=3&embed=primaryBitstream&size=5'));
+
+        expect(console.warn).toHaveBeenCalledWith(
+          'The response for \'https://rest.api/core/items/eba1c085/bundles?page=0&size=5\' has the self link ' +
+          '\'https://rest.api/core/items/eba1c085/bundles?page=3&embed=primaryBitstream&size=5\'. ' +
+          'These don\'t match. This could mean there\'s an issue with the REST endpoint');
+      });
+
+      it('should warn when a non-embed param differs', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?page=0&embed=primaryBitstream&size=5');
+        service.callEnsureSelfLink(request,
+          responseWithSelfLink('https://rest.api/core/items/eba1c085/bundles?page=3&embed=primaryBitstream&size=5'));
+
+        expect(console.warn).toHaveBeenCalledTimes(1);
+        expect(console.warn).toHaveBeenCalledWith(MISMATCH);
+      });
+
+      it('should warn when the self link has a param the request did not have', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=5');
+        service.callEnsureSelfLink(request,
+          responseWithSelfLink('https://rest.api/core/items/eba1c085/bundles?size=5&sort=name,ASC'));
+
+        expect(console.warn).toHaveBeenCalledTimes(1);
+        expect(console.warn).toHaveBeenCalledWith(MISMATCH);
+      });
+
+      it('should warn and fill in the requested url when the response has no self link', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?embed=primaryBitstream&size=5');
+        const response = service.callEnsureSelfLink(request, {
+          payload: { _links: {} },
+          statusCode: 200,
+          statusText: 'OK',
+        });
+
+        expect(console.warn).toHaveBeenCalledTimes(1);
+        expect(console.warn).toHaveBeenCalledWith(NO_SELF_LINK);
+        expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085/bundles?size=5');
+      });
+
+    });
+
+    describe('normalization of the self link', () => {
+
+      it('should normalize the self link when it differs, so it matches the cache key', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?page=0&embed=primaryBitstream&size=5');
+        const response = service.callEnsureSelfLink(request,
+          responseWithSelfLink('https://rest.api/core/items/eba1c085/bundles?page=3&embed=primaryBitstream&size=5'));
+
+        expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085/bundles?page=0&size=5');
+      });
+
+      it('should keep the other links when it normalizes the self link', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?page=0&size=5');
+        const response = service.callEnsureSelfLink(request, {
+          payload: {
+            _links: {
+              self: { href: 'https://rest.api/core/items/eba1c085/bundles?page=3&size=5' },
+              primaryBitstream: { href: 'https://rest.api/core/bitstreams/6a5f' },
+            },
+          },
+          statusCode: 200,
+          statusText: 'OK',
+        });
+
+        expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085/bundles?page=0&size=5');
+        expect(response.payload._links.primaryBitstream.href).toBe('https://rest.api/core/bitstreams/6a5f');
+      });
+
+      it('should not touch a self link on a different host', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=5');
+        const response = service.callEnsureSelfLink(request,
+          responseWithSelfLink('https://other.api/core/items/eba1c085/bundles?size=5'));
+
+        expect(console.warn).not.toHaveBeenCalled();
+        expect(response.payload._links.self.href).toBe('https://other.api/core/items/eba1c085/bundles?size=5');
+      });
+
+      it('should not touch a self link that points at a different path', () => {
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=5');
+        const response = service.callEnsureSelfLink(request,
+          responseWithSelfLink('https://rest.api/core/items/eba1c085?size=5'));
+
+        expect(console.warn).not.toHaveBeenCalled();
+        expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085?size=5');
+      });
+
+      it('should leave non-GET requests alone', () => {
+        const request = new PostRequest('c4f0b1b7-3ffa-4b1a-9f5f-8bd6b1c4de71', 'https://rest.api/core/items/eba1c085/bundles?size=5');
+        const response = service.callEnsureSelfLink(request,
+          responseWithSelfLink('https://rest.api/core/items/eba1c085/bundles?size=1000'));
+
+        expect(console.warn).not.toHaveBeenCalled();
+        expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085/bundles?size=1000');
+      });
+
+    });
+
+  });
+});