JCU/Say what actually happened when the REST API reduces a page size (#1448)

The generic wording ends with "This could mean there's an issue with the REST
endpoint", which points at the backend. For a reduced page size that is the wrong
place to look: the API did nothing wrong, the caller asked for a bigger page than
it will serve.

  The request for '.../bundles?size=9999' asked for a page of 9999 elements, but
  the REST API served 1000. Ask for at most MAX_PAGE_SIZE elements

Anything else keeps the generic message, including a page that came back larger
than requested.

Also drops a stray note from the spec that was left in by #1440.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: milanmajchrak

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

@@ -22,6 +22,7 @@ describe('DspaceRestResponseParsingService', () => {
   let objectCache: ObjectCacheService;
 
   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 =>
@@ -67,7 +68,6 @@ describe('DspaceRestResponseParsingService', () => {
       });
 
       it('should not warn when the self link echoes embed params and the request has no other params', () => {
-        // observed in the browser against a DSpace 9.1 backend
         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));
 
@@ -100,19 +100,35 @@ describe('DspaceRestResponseParsingService', () => {
 
     describe('differences that point at a problem with the endpoint', () => {
 
-      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
+      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 warn when the returned page size is larger than the requested one', () => {
+      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',

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

@@ -54,6 +54,11 @@ export function isRestPaginatedList(halObj: any): boolean {
     hasValue(halObj.page.number);
 }
 
+/**
+ * The url param holding the page size
+ */
+const PAGE_SIZE_PARAM = 'size=';
+
 /**
  * Split a url into parts
  *
@@ -74,8 +79,8 @@ const urlPartsDiffer = (expected: string[], actual: string[]): boolean => {
 };
 
 /**
- * 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.
+ * Percent decode each url part, so `uri=http%3A%2F%2Fx` and `uri=http://x` compare equal. Decoding
+ * after the split keeps a decoded `&` from merging two params.
  */
 const decodeUrlParts = (parts: string[]): string[] => {
   return parts.map((part: string) => {
@@ -88,20 +93,39 @@ const decodeUrlParts = (parts: string[]): string[] => {
 };
 
 /**
- * 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. Takes the requested url already split, since the caller has it.
- *
- * 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.
+ * The page size a url asks for, or undefined when it doesn't ask for a usable one
+ */
+const getPageSize = (parts: string[]): number | undefined => {
+  return parts.filter((part: string) => part.startsWith(PAGE_SIZE_PARAM))
+    .map((part: string) => Number(part.substring(PAGE_SIZE_PARAM.length)))
+    .find((size: number) => Number.isInteger(size) && size > 0);
+};
+
+/**
+ * Return the parts without the one holding the page size
  */
-const isUnexpectedSelfLink = (requestedUrlParts: string[], selfLink: string): boolean => {
-  return urlPartsDiffer(
-    decodeUrlParts(requestedUrlParts),
-    decodeUrlParts(splitUrlInParts(getUrlWithoutEmbedParams(selfLink))),
-  );
+const withoutPageSize = (parts: string[]): string[] => {
+  return parts.filter((part: string) => !part.startsWith(PAGE_SIZE_PARAM));
+};
+
+/**
+ * Return the warning to log for a self link, or undefined when it describes the same request as the
+ * url it was requested with. A reduced page size gets its own message, since the generic one blames
+ * the endpoint for something the caller did.
+ */
+const selfLinkWarning = (requestedUrl: string, requestedUrlParts: string[], selfLink: string): string | undefined => {
+  const expected = decodeUrlParts(requestedUrlParts);
+  const actual = decodeUrlParts(splitUrlInParts(getUrlWithoutEmbedParams(selfLink)));
+  if (!urlPartsDiffer(expected, actual)) {
+    return undefined;
+  }
+  const requestedSize = getPageSize(expected);
+  const servedSize = getPageSize(actual);
+  if (hasValue(requestedSize) && hasValue(servedSize) && servedSize < requestedSize
+    && !urlPartsDiffer(withoutPageSize(expected), withoutPageSize(actual))) {
+    return `The request for '${requestedUrl}' asked for a page of ${requestedSize} elements, but the REST API served ${servedSize}. Ask for at most MAX_PAGE_SIZE elements`;
+  }
+  return `The response for '${requestedUrl}' has the self link '${selfLink}'. These don't match. This could mean there's an issue with the REST endpoint`;
 };
 
 @Injectable({ providedIn: 'root' })
@@ -210,9 +234,10 @@ export class DspaceRestResponseParsingService implements ResponseParsingService
         const expected = splitUrlInParts(urlWithoutEmbedParams);
         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(expected, 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`);
+          // the self link is normalized either way, only the warning is conditional
+          const warning = selfLinkWarning(urlWithoutEmbedParams, expected, selfLink);
+          if (hasValue(warning)) {
+            console.warn(warning);
           }
           response.payload._links = Object.assign({}, response.payload._links, {
             self: {