WEBDEV-5544 Add uid and client_url PPS params (#21)

* Add uid and client_url parameters to the models * Add tests for uid/client_url and update other tests (other tests updated to account for client_url defaulting true) * Update demo app to show query params w/ uid and client_url * Adjust search param tests to compare specific query params, not entire query strings * Better document client_url param * Remove includeClientUrl param from one test where it was missed * Fix formatting * Update search-backend tests to compare query params rather than urls * Missed a couple of tests * Add tests for true/false cases of includeClientUrl * Clarify options for includeClientUrl in doc comments
internetarchive · Oct 25, 2022 · 3d341a9 · 3d341a9
1 parent 1b34208
commit 3d341a9
Show file tree

Hide file tree

Showing 6 changed files with 212 additions and 49 deletions.
diff --git a/demo/app-root.ts b/demo/app-root.ts
@@ -8,6 +8,7 @@ import { SearchType } from '../src/search-type';
 import { SearchParams, SortDirection } from '../src/search-params';
 import { Aggregation, Bucket } from '../src/models/aggregation';
 import { SearchBackendOptionsInterface } from '../src/search-backend/search-backend-options';
+import { SearchParamURLGenerator } from '../src/search-param-url-generator';
 
 @customElement('app-root')
 export class AppRoot extends LitElement {
@@ -44,6 +45,12 @@ export class AppRoot extends LitElement {
   @state()
   private loadingAggregations = false;
 
+  @state()
+  private lastSearchParams?: string;
+
+  @state()
+  private lastAggregationParams?: string;
+
   @state()
   private defaultAggregationsChecked = true;
 
@@ -212,6 +219,18 @@ export class AppRoot extends LitElement {
 
   private get resultsTemplate(): TemplateResult {
     return html`
+      ${this.lastSearchParams
+        ? html`<div>
+            Last search params:
+            <pre>${this.lastSearchParams}</pre>
+          </div>`
+        : nothing}
+      ${this.lastAggregationParams
+        ? html`<div>
+            Last aggregation params:
+            <pre>${this.lastAggregationParams}</pre>
+          </div>`
+        : nothing}
       ${this.loadingSearchResults
         ? html`<h3>Loading search results...</h3>`
         : [this.minimalSearchResultsTemplate, this.fullSearchResultsTemplate]}
@@ -348,6 +367,7 @@ export class AppRoot extends LitElement {
       sort: sortParam,
       aggregations: { omit: true },
       debugging: includeDebugging,
+      uid: 'demo',
     };
 
     this.loadingSearchResults = true;
@@ -356,6 +376,9 @@ export class AppRoot extends LitElement {
 
     if (result?.success) {
       this.searchResponse = result?.success;
+      this.lastSearchParams = SearchParamURLGenerator.generateURLSearchParams(
+        searchParams
+      ).toString();
     } else {
       alert(`Oh noes: ${result?.error?.message}`);
       console.error('Error searching', result?.error);
@@ -383,6 +406,7 @@ export class AppRoot extends LitElement {
       rows: 0,
       aggregationsSize: numAggs,
       debugging: includeDebugging,
+      uid: 'demo',
     };
 
     if (!this.defaultAggregationsChecked) {
@@ -395,6 +419,9 @@ export class AppRoot extends LitElement {
 
     if (result?.success) {
       this.aggregationsResponse = result?.success;
+      this.lastAggregationParams = SearchParamURLGenerator.generateURLSearchParams(
+        searchParams
+      ).toString();
     } else {
       alert(`Oh noes: ${result?.error?.message}`);
       console.error('Error searching', result?.error);
@@ -410,6 +437,10 @@ export class AppRoot extends LitElement {
       .field-row {
         margin: 0.3rem 0;
       }
+
+      fieldset {
+        margin-bottom: 0.5rem;
+      }
     `;
   }
 }
diff --git a/src/search-param-url-generator.ts b/src/search-param-url-generator.ts
@@ -101,6 +101,14 @@ export class SearchParamURLGenerator {
       params.append('debugging', 'true');
     }
 
+    if (searchParams.uid) {
+      params.append('uid', searchParams.uid);
+    }
+
+    if (searchParams.includeClientUrl !== false) {
+      params.append('client_url', window.location.href);
+    }
+
     return params;
   }
 }
diff --git a/src/search-params.ts b/src/search-params.ts
@@ -122,4 +122,25 @@ export interface SearchParams {
    * Whether to include debugging info in the returned PPS response.
    */
   debugging?: boolean;
+
+  /**
+   * A unique request ID to pass to the service.
+   * Will be returned unmodified in the response, for ease of tracking responses,
+   * e.g., to quickly determine whether a given response has obsolete data.
+   */
+  uid?: string;
+
+  /**
+   * Whether to include the client URL in the request.
+   * This is useful for PPS debugging as it allows the backend folks to compare
+   * any PPS response issues with the client URLs that generated them,
+   * especially for issues related to parameter parsing & normalization.
+   *
+   * This defaults to true, as these should be sent on every request unless
+   * there is a specific need not to include them for certain request types. Thus:
+   *  * `includeClientUrl: undefined` causes `client_url` param to be included in the request, by default.
+   *  * `includeClientUrl: true` causes `client_url` param to be included, explicitly.
+   *  * `includeClientUrl: false` causes `client_url` param to _not_ be included in the request.
+   */
+  includeClientUrl?: boolean;
 }
diff --git a/test/search-backend/fulltext-search-backend.test.ts b/test/search-backend/fulltext-search-backend.test.ts
@@ -83,9 +83,9 @@ describe('FulltextSearchBackend', () => {
       });
       await backend.performSearch({ query: 'boop' });
 
-      expect(urlCalled!.toString()).to.equal(
-        'https://foo.bar/baz/?service_backend=fts&user_query=boop&debugging=true'
-      );
+      const queryParams = new URL(urlCalled!.toString()).searchParams;
+      expect(queryParams.get('user_query')).to.equal('boop');
+      expect(queryParams.get('debugging')).to.equal('true');
     });
 
     it('can disable default debugging on individual searches', async () => {
@@ -94,11 +94,14 @@ describe('FulltextSearchBackend', () => {
         servicePath: '/baz',
         debuggingEnabled: true,
       });
-      await backend.performSearch({ query: 'boop', debugging: false });
+      await backend.performSearch({
+        query: 'boop',
+        debugging: false,
+      });
 
-      expect(urlCalled!.toString()).to.equal(
-        'https://foo.bar/baz/?service_backend=fts&user_query=boop'
-      );
+      const queryParams = new URL(urlCalled!.toString()).searchParams;
+      expect(queryParams.get('user_query')).to.equal('boop');
+      expect(queryParams.get('debugging')).to.be.null;
     });
   });
 

diff --git a/test/search-backend/metadata-search-backend.test.ts b/test/search-backend/metadata-search-backend.test.ts
@@ -83,9 +83,9 @@ describe('MetadataSearchBackend', () => {
       });
       await backend.performSearch({ query: 'boop' });
 
-      expect(urlCalled!.toString()).to.equal(
-        'https://foo.bar/baz/?service_backend=metadata&user_query=boop&debugging=true'
-      );
+      const queryParams = new URL(urlCalled!.toString()).searchParams;
+      expect(queryParams.get('user_query')).to.equal('boop');
+      expect(queryParams.get('debugging')).to.equal('true');
     });
 
     it('can disable default debugging on individual searches', async () => {
@@ -94,11 +94,14 @@ describe('MetadataSearchBackend', () => {
         servicePath: '/baz',
         debuggingEnabled: true,
       });
-      await backend.performSearch({ query: 'boop', debugging: false });
+      await backend.performSearch({
+        query: 'boop',
+        debugging: false,
+      });
 
-      expect(urlCalled!.toString()).to.equal(
-        'https://foo.bar/baz/?service_backend=metadata&user_query=boop'
-      );
+      const queryParams = new URL(urlCalled!.toString()).searchParams;
+      expect(queryParams.get('user_query')).to.equal('boop');
+      expect(queryParams.get('debugging')).to.be.null;
     });
   });