Report a reduced page size again, now that nothing asks for an impossible one

Filtering the page size out of the self link comparison treated the symptom: the
frontend went on asking for 9999, the API went on reducing it to 1000, and the code
here just stopped saying so. With every call site now within MAX_PAGE_SIZE, a page
size the API reduced means a caller asked for something it was never going to get -
which is exactly what this warning exists to surface.

Drops isReducedPageSize, getPageSizes and PAGE_SIZE_PARAM, and with them the need to
pass the response payload into isUnexpectedSelfLink at all.

What stays is the part that is a genuine comparison bug rather than a filter: embed
params and percent encoding are two ways of writing the same request, so both sides
are brought to the same form before being compared. A difference in an actual value
is still reported.

Spec goes from 25 cases to 17: the nine that pinned the page size filtering are gone,
replaced by one asserting a reduced page size warns.

Author: claude

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

@@ -72,27 +72,6 @@ describe('DspaceRestResponseParsingService', () => {
         expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085/bundles');
       });
 
-      it('should not warn when the REST API reduced the requested page size', () => {
-        // https://github.com/dataquest-dev/dspace-customers/issues/862
-        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?embed=primaryBitstream&size=9999');
-        const response = service.callEnsureSelfLink(request, responseWithSelfLink(
-          'https://rest.api/core/items/eba1c085/bundles?embed=primaryBitstream&size=1000',
-          { number: 0, size: 1000, totalPages: 1, totalElements: 2 }));
-
-        expect(console.warn).not.toHaveBeenCalled();
-        expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085/bundles?size=9999');
-      });
-
-      it('should not warn when the REST API reduced the page size and no embeds are involved', () => {
-        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=9999');
-        const response = service.callEnsureSelfLink(request, responseWithSelfLink(
-          'https://rest.api/core/items/eba1c085/bundles?size=1000',
-          { number: 0, size: 1000, totalPages: 1, totalElements: 2 }));
-
-        expect(console.warn).not.toHaveBeenCalled();
-        expect(response.payload._links.self.href).toBe('https://rest.api/core/items/eba1c085/bundles?size=9999');
-      });
-
       it('should not warn when the self link only percent decoded a param value', () => {
         // https://github.com/dataquest-dev/dspace-customers/issues/862
         const request = requestFor('https://rest.api/statistics/usagereports/search/object?page=-1&size=10&uri=https%3A%2F%2Frest.api%2Fcore%2Fsites%2F8f842a80');
@@ -118,20 +97,13 @@ describe('DspaceRestResponseParsingService', () => {
 
     describe('differences that point at a problem with the endpoint', () => {
 
-      it('should warn when the page size shrank but the page block contradicts the self link', () => {
-        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=100');
+      it('should warn when the REST API reduced the requested page size', () => {
+        // callers are expected to stay within MAX_PAGE_SIZE, so a reduced size means a caller asked
+        // for a page the API was never going to serve
+        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=9999');
         service.callEnsureSelfLink(request, responseWithSelfLink(
-          'https://rest.api/core/items/eba1c085/bundles?size=20',
-          { number: 0, size: 100, totalPages: 1, totalElements: 2 }));
-
-        expect(console.warn).toHaveBeenCalledTimes(1);
-        expect(console.warn).toHaveBeenCalledWith(MISMATCH);
-      });
-
-      it('should warn when the page size shrank but the response has no page block to confirm it', () => {
-        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=100');
-        service.callEnsureSelfLink(request,
-          responseWithSelfLink('https://rest.api/core/items/eba1c085/bundles?size=20'));
+          '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(MISMATCH);
@@ -147,16 +119,6 @@ describe('DspaceRestResponseParsingService', () => {
         expect(console.warn).toHaveBeenCalledWith(MISMATCH);
       });
 
-      it('should warn when the url is ambiguous about the page size', () => {
-        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=5&size=10');
-        service.callEnsureSelfLink(request, responseWithSelfLink(
-          'https://rest.api/core/items/eba1c085/bundles?size=3',
-          { number: 0, size: 3, 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,
@@ -166,46 +128,6 @@ describe('DspaceRestResponseParsingService', () => {
         expect(console.warn).toHaveBeenCalledWith(MISMATCH);
       });
 
-      it('should warn when a reduced page size hides another param that 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',
-          { number: 3, size: 1000, totalPages: 4, totalElements: 3200 }));
-
-        expect(console.warn).toHaveBeenCalledTimes(1);
-        expect(console.warn).toHaveBeenCalledWith(MISMATCH);
-      });
-
-      it('should warn when the self link claims an empty page', () => {
-        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?size=10');
-        service.callEnsureSelfLink(request, responseWithSelfLink(
-          'https://rest.api/core/items/eba1c085/bundles?size=0',
-          { number: 0, size: 0, totalPages: 0, totalElements: 0 }));
-
-        expect(console.warn).toHaveBeenCalledTimes(1);
-        expect(console.warn).toHaveBeenCalledWith(MISMATCH);
-      });
-
-      it('should not treat a param that merely ends in `size` as the page size', () => {
-        const request = requestFor('https://rest.api/core/items/eba1c085/bundles?pagesize=9999');
-        service.callEnsureSelfLink(request, responseWithSelfLink(
-          'https://rest.api/core/items/eba1c085/bundles?pagesize=1000',
-          { number: 0, size: 1000, totalPages: 1, totalElements: 2 }));
-
-        expect(console.warn).toHaveBeenCalledTimes(1);
-        expect(console.warn).toHaveBeenCalledWith(MISMATCH);
-      });
-
-      it('should not accept a page block that confirms the reduced size only as a string', () => {
-        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(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,

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

@@ -62,11 +62,6 @@ const urlPartsDiffer = (expected: string[], actual: string[]): boolean => {
     || actual.some((part: string) => !expected.includes(part));
 };
 
-/**
- * Matches a page size query param, e.g. `size=100`
- */
-const PAGE_SIZE_PARAM = /^size=(\d+)$/;
-
 /**
  * Percent decode each url part, so `uri=http%3A%2F%2Fx` and `uri=http://x` compare equal. Parts are
  * decoded one by one, after the url was split, so a decoded `&` can't merge two params.
@@ -82,46 +77,20 @@ const decodeUrlParts = (parts: string[]): string[] => {
 };
 
 /**
- * Return the page sizes among the given url parts. More than one means the url is ambiguous
- */
-const getPageSizes = (parts: string[]): number[] => {
-  return parts
-    .map((part: string) => part.match(PAGE_SIZE_PARAM))
-    .filter((matches) => hasValue(matches))
-    .map((matches) => Number(matches[1]));
-};
-
-/**
- * Return true if the self link only shrank the page size to a value the response confirms itself.
+ * Return true if the self link differs from the requested url in a way that isn't just a different
+ * way of writing the same request.
  *
- * The REST API silently resets a `size` over its configured maximum. The frontend can't know that
- * maximum, so it requires the `page` block to corroborate the smaller size, and reports a self link
- * that contradicts the payload it describes. Zero is never a maximum, so an empty page isn't a
- * clamp either.
+ * Both sides are brought to the same form first: `embed`/`embed.size` params are stripped, because
+ * the frontend treats them as not part of a resource's identity and indexes without them, and both
+ * are percent decoded. Anything still differing is a real difference between what was asked for and
+ * what came back, including a page size the API reduced — callers are expected to stay within
+ * `MAX_PAGE_SIZE` rather than have that reported difference filtered out here.
  */
-const isReducedPageSize = (expected: string[], actual: string[], payload: any): boolean => {
-  const requestedSizes = getPageSizes(expected);
-  const effectiveSizes = getPageSizes(actual);
-  return requestedSizes.length === 1 && effectiveSizes.length === 1
-    && effectiveSizes[0] > 0 && effectiveSizes[0] < requestedSizes[0]
-    && hasValue(payload.page) && payload.page.size === effectiveSizes[0];
-};
-
-/**
- * Return true if the self link differs from the requested url in a way the REST API isn't expected
- * to introduce by itself. Not reported: `embed` params the API echoes back (they are stripped from
- * the requested url but not from the self link), percent encoding, and a confirmed page size clamp.
- */
-const isUnexpectedSelfLink = (requestedUrl: string, selfLink: string, payload: any): boolean => {
-  const expected = decodeUrlParts(splitUrlInParts(requestedUrl));
-  const actual = decodeUrlParts(splitUrlInParts(getUrlWithoutEmbedParams(selfLink)));
-
-  if (isReducedPageSize(expected, actual, payload)) {
-    const withoutPageSize = (parts: string[]): string[] =>
-      parts.filter((part: string) => !PAGE_SIZE_PARAM.test(part));
-    return urlPartsDiffer(withoutPageSize(expected), withoutPageSize(actual));
-  }
-  return urlPartsDiffer(expected, actual);
+const isUnexpectedSelfLink = (requestedUrl: string, selfLink: string): boolean => {
+  return urlPartsDiffer(
+    decodeUrlParts(splitUrlInParts(requestedUrl)),
+    decodeUrlParts(splitUrlInParts(getUrlWithoutEmbedParams(selfLink))),
+  );
 };
 
 @Injectable({ providedIn: 'root' })
@@ -231,7 +200,7 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
         const actual = splitUrlInParts(selfLink);
         if (expected[0] === actual[0] && urlPartsDiffer(expected, actual)) {
           // the self link is normalized either way, only the warning is filtered
-          if (isUnexpectedSelfLink(urlWithoutEmbedParams, selfLink, response.payload)) {
+          if (isUnexpectedSelfLink(urlWithoutEmbedParams, selfLink)) {
             console.warn(`The response for '${urlWithoutEmbedParams}' has the self link '${selfLink}'. These don't match. This could mean there's an issue with the REST endpoint`);
           }
           response.payload._links = Object.assign({}, response.payload._links, {