fix(clients): make POST body related to the endpoint, assert body in …

…tests (#849)
algolia · Jul 20, 2022 · a4346a7 · a4346a7
1 parent 3b0ddfc
commit a4346a7
Show file tree

Hide file tree

Showing 19 changed files with 142 additions and 150 deletions.
diff --git a/...s/algoliasearch-client-java-2/algoliasearch-core/src/main/java/com/algolia/ApiClient.java b/...s/algoliasearch-client-java-2/algoliasearch-core/src/main/java/com/algolia/ApiClient.java
@@ -239,12 +239,17 @@ public RequestBody serialize(Object obj) throws AlgoliaRuntimeException {
 
     if (obj != null) {
       try {
-        content = json.writeValueAsString(obj);
+        if (obj.getClass().getName().equals("java.lang.Object")) {
+          content = "{}";
+        } else {
+          content = json.writeValueAsString(obj);
+        }
       } catch (JsonProcessingException e) {
         throw new AlgoliaRuntimeException(e);
       }
     } else {
-      content = null;
+      // We can't send a null body with okhttp, so we default it to an empty string
+      content = "";
     }
 
     return RequestBody.create(content, MediaType.parse(this.contentType));
@@ -343,16 +348,10 @@ public Request buildRequest(
     processHeaderParams(headerParams, hasRequestOptions ? requestOptions.getExtraHeaders() : null, reqBuilder);
 
     RequestBody reqBody;
-    if (!HttpMethod.permitsRequestBody(method)) {
+    // We rely on `permitsRequestBody` to tell us if we should provide a body
+    // but also set it for DELETE methods
+    if (!HttpMethod.permitsRequestBody(method) || (method.equals("DELETE") && body == null)) {
       reqBody = null;
-    } else if (body == null) {
-      if ("DELETE".equals(method)) {
-        // allow calling DELETE without sending a request body
-        reqBody = null;
-      } else {
-        // use an empty request body (for POST, PUT and PATCH)
-        reqBody = RequestBody.create("", MediaType.parse(this.contentType));
-      }
     } else {
       reqBody = serialize(body);
     }

diff --git a/clients/algoliasearch-client-javascript/bundlesize.config.json b/clients/algoliasearch-client-javascript/bundlesize.config.json
@@ -2,11 +2,11 @@
   "files": [
     {
       "path": "packages/algoliasearch/dist/algoliasearch.umd.js",
-      "maxSize": "8.35KB"
+      "maxSize": "8.30KB"
     },
     {
       "path": "packages/algoliasearch/dist/lite/lite.umd.js",
-      "maxSize": "3.90KB"
+      "maxSize": "3.85KB"
     },
     {
       "path": "packages/client-abtesting/dist/client-abtesting.umd.js",
@@ -18,15 +18,15 @@
     },
     {
       "path": "packages/client-insights/dist/client-insights.umd.js",
-      "maxSize": "3.85KB"
+      "maxSize": "3.80KB"
     },
     {
       "path": "packages/client-personalization/dist/client-personalization.umd.js",
-      "maxSize": "4.00KB"
+      "maxSize": "3.95KB"
     },
     {
       "path": "packages/client-query-suggestions/dist/client-query-suggestions.umd.js",
-      "maxSize": "4.05KB"
+      "maxSize": "4.00KB"
     },
     {
       "path": "packages/client-search/dist/client-search.umd.js",
@@ -38,7 +38,7 @@
     },
     {
       "path": "packages/predict/dist/predict.umd.js",
-      "maxSize": "3.90KB"
+      "maxSize": "3.85KB"
     },
     {
       "path": "packages/recommend/dist/recommend.umd.js",

diff --git a/clients/algoliasearch-client-javascript/packages/client-common/src/createEchoRequester.ts b/clients/algoliasearch-client-javascript/packages/client-common/src/createEchoRequester.ts
@@ -1,4 +1,4 @@
-import type { EchoResponse, EndRequest, Request, Response } from './types';
+import type { EchoResponse, EndRequest, Requester, Response } from './types';
 
 export type EchoRequesterParams = {
   getURL: (url: string) => URL;
@@ -8,7 +8,8 @@ export type EchoRequesterParams = {
 function getUrlParams({
   host,
   searchParams: urlSearchParams,
-}: URL): Pick<EchoResponse, 'algoliaAgent' | 'host' | 'searchParams'> {
+  pathname,
+}: URL): Pick<EchoResponse, 'algoliaAgent' | 'host' | 'path' | 'searchParams'> {
   const algoliaAgent = urlSearchParams.get('x-algolia-agent') || '';
   const searchParams = {};
 
@@ -25,39 +26,34 @@ function getUrlParams({
     algoliaAgent,
     searchParams:
       Object.keys(searchParams).length === 0 ? undefined : searchParams,
+    path: pathname,
   };
 }
 
-// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
 export function createEchoRequester({
   getURL,
   status = 200,
-}: EchoRequesterParams) {
-  function send(
-    { headers, url, connectTimeout, responseTimeout }: EndRequest,
-    { data, ...originalRequest }: Request
-  ): Promise<Response> {
-    const { host, searchParams, algoliaAgent } = getUrlParams(getURL(url));
-    const originalData =
-      data && Object.keys(data).length > 0 ? data : undefined;
+}: EchoRequesterParams): Requester {
+  function send(request: EndRequest): Promise<Response> {
+    const { host, searchParams, algoliaAgent, path } = getUrlParams(
+      getURL(request.url)
+    );
+
+    const content: EchoResponse = {
+      ...request,
+      data: request.data ? JSON.parse(request.data) : undefined,
+      path,
+      host,
+      algoliaAgent: encodeURI(algoliaAgent),
+      searchParams,
+    };
 
     return Promise.resolve({
-      content: JSON.stringify({
-        ...originalRequest,
-        host,
-        headers,
-        connectTimeout,
-        responseTimeout,
-        algoliaAgent: encodeURI(algoliaAgent),
-        searchParams,
-        data: originalData,
-      }),
+      content: JSON.stringify(content),
       isTimedOut: false,
       status,
     });
   }
 
   return { send };
 }
-
-export type EchoRequester = ReturnType<typeof createEchoRequester>;
diff --git a/...goliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts b/...goliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts
@@ -85,17 +85,20 @@ export function createTransporter({
 
   async function retryableRequest<TResponse>(
     request: Request,
-    requestOptions: RequestOptions
+    requestOptions: RequestOptions,
+    isRead = true
   ): Promise<TResponse> {
     const stackTrace: StackFrame[] = [];
-    const isRead = request.useReadTransporter || request.method === 'GET';
 
     /**
      * First we prepare the payload that do not depend from hosts.
      */
     const data = serializeData(request, requestOptions);
-    const headers = serializeHeaders(baseHeaders, requestOptions);
-    const method = request.method;
+    const headers = serializeHeaders(
+      baseHeaders,
+      request.headers,
+      requestOptions.headers
+    );
 
     // On `GET`, the data is proxied to query parameters.
     const dataQueryParameters: QueryParameters =
@@ -109,6 +112,7 @@ export function createTransporter({
     const queryParameters: QueryParameters = {
       'x-algolia-agent': algoliaAgent.value,
       ...baseQueryParameters,
+      ...request.queryParameters,
       ...dataQueryParameters,
     };
 
@@ -152,7 +156,7 @@ export function createTransporter({
       const payload: EndRequest = {
         data,
         headers,
-        method,
+        method: request.method,
         url: serializeUrl(host, request.path, queryParameters),
         connectTimeout: getTimeout(timeoutsCount, timeouts.connect),
         responseTimeout: getTimeout(timeoutsCount, responseTimeout),
@@ -236,41 +240,20 @@ export function createTransporter({
   }
 
   function createRequest<TResponse>(
-    baseRequest: Request,
-    baseRequestOptions: RequestOptions = {}
+    request: Request,
+    requestOptions: RequestOptions = {}
   ): Promise<TResponse> {
-    const mergedData: Request['data'] = Array.isArray(baseRequest.data)
-      ? baseRequest.data
-      : {
-          ...baseRequest.data,
-          ...baseRequestOptions.data,
-        };
-    const request: Request = {
-      ...baseRequest,
-      data: mergedData,
-    };
-    const requestOptions: RequestOptions = {
-      cacheable: baseRequestOptions.cacheable,
-      timeout: baseRequestOptions.timeout,
-      queryParameters: {
-        ...baseRequest.queryParameters,
-        ...baseRequestOptions.queryParameters,
-      },
-      headers: {
-        Accept: 'application/json',
-        ...baseRequest.headers,
-        ...baseRequestOptions.headers,
-      },
-    };
-
+    /**
+     * A read request is either a `GET` request, or a request that we make
+     * via the `read` transporter (e.g. `search`).
+     */
     const isRead = request.useReadTransporter || request.method === 'GET';
-
     if (!isRead) {
       /**
        * On write requests, no cache mechanisms are applied, and we
        * proxy the request immediately to the requester.
        */
-      return retryableRequest<TResponse>(request, requestOptions);
+      return retryableRequest<TResponse>(request, requestOptions, isRead);
     }
 
     const createRetryableRequest = (): Promise<TResponse> => {
@@ -287,7 +270,7 @@ export function createTransporter({
      * request is "cacheable" - should be cached. Note that, once again,
      * the user can force this option.
      */
-    const cacheable = Boolean(requestOptions.cacheable || request.cacheable);
+    const cacheable = requestOptions.cacheable || request.cacheable;
 
     /**
      * If is not "cacheable", we immediately trigger the retryable request, no
@@ -306,8 +289,8 @@ export function createTransporter({
       request,
       requestOptions,
       transporter: {
-        queryParameters: requestOptions.queryParameters,
-        headers: requestOptions.headers,
+        queryParameters: baseQueryParameters,
+        headers: baseHeaders,
       },
     };
 

diff --git a/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/helpers.ts b/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/helpers.ts
@@ -78,11 +78,14 @@ export function serializeData(
 
 export function serializeHeaders(
   baseHeaders: Headers,
-  requestOptions: RequestOptions
+  requestHeaders: Headers,
+  requestOptionsHeaders?: Headers
 ): Headers {
   const headers: Headers = {
+    Accept: 'application/json',
     ...baseHeaders,
-    ...requestOptions.headers,
+    ...requestHeaders,
+    ...requestOptionsHeaders,
   };
   const serializedHeaders: Headers = {};
 

diff --git a/clients/algoliasearch-client-javascript/packages/client-common/src/types/Requester.ts b/clients/algoliasearch-client-javascript/packages/client-common/src/types/Requester.ts
@@ -42,11 +42,9 @@ export type Requester = {
   send: (request: EndRequest, originalRequest: Request) => Promise<Response>;
 };
 
-export type EchoResponse = Request & {
-  connectTimeout: number;
-  host: string;
-  headers: Headers;
-  responseTimeout: number;
-  algoliaAgent: string;
-  searchParams?: Record<string, string>;
-};
+export type EchoResponse = Omit<EndRequest, 'data'> &
+  Pick<Request, 'data' | 'path'> & {
+    host: string;
+    algoliaAgent: string;
+    searchParams?: Record<string, string>;
+  };
diff --git a/clients/algoliasearch-client-javascript/packages/requester-browser-xhr/src/echoRequester.ts b/clients/algoliasearch-client-javascript/packages/requester-browser-xhr/src/echoRequester.ts
@@ -1,6 +1,6 @@
 import { createEchoRequester } from '@algolia/client-common';
-import type { EchoRequester } from '@algolia/client-common';
+import type { Requester } from '@algolia/client-common';
 
-export function echoRequester(status: number = 200): EchoRequester {
+export function echoRequester(status: number = 200): Requester {
   return createEchoRequester({ getURL: (url: string) => new URL(url), status });
 }
diff --git a/clients/algoliasearch-client-javascript/packages/requester-node-http/src/echoRequester.ts b/clients/algoliasearch-client-javascript/packages/requester-node-http/src/echoRequester.ts
@@ -1,8 +1,8 @@
 import { URL } from 'url';
 
 import { createEchoRequester } from '@algolia/client-common';
-import type { EchoRequester } from '@algolia/client-common';
+import type { Requester } from '@algolia/client-common';
 
-export function echoRequester(status: number = 200): EchoRequester {
+export function echoRequester(status: number = 200): Requester {
   return createEchoRequester({ getURL: (url: string) => new URL(url), status });
 }
diff --git a/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php b/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php
@@ -50,7 +50,8 @@ private function normalize($options)
             'headers' => [
                 'x-algolia-application-id' => $this->config->getAppId(),
                 'x-algolia-api-key' => $this->config->getAlgoliaApiKey(),
-                'User-Agent' => $this->config->getAlgoliaAgent() !== null
+                'User-Agent' =>
+                    $this->config->getAlgoliaAgent() !== null
                         ? $this->config->getAlgoliaAgent()
                         : AlgoliaAgent::get($this->config->getClientName()),
                 'Content-Type' => 'application/json',

diff --git a/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapper.php b/clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapper.php
@@ -79,15 +79,10 @@ public function sendRequest(
          */
         $isRead = $useReadTransporter || 'GET' === mb_strtoupper($method);
 
-        if ($isRead) {
+        if ($isRead || 'DELETE' === mb_strtoupper($method)) {
             $requestOptions = $this->requestOptionsFactory->createBodyLess(
                 $requestOptions
             );
-        } elseif ('DELETE' === mb_strtoupper($method)) {
-            $requestOptions = $this->requestOptionsFactory->createBodyLess(
-                $requestOptions
-            );
-            $data = [];
         } else {
             $requestOptions = $this->requestOptionsFactory->create(
                 $requestOptions
@@ -139,7 +134,9 @@ private function request(
             ->withQuery($requestOptions->getBuiltQueryParameters())
             ->withScheme('https');
 
-        $body = array_merge($data, $requestOptions->getBody());
+        $body = isset($data)
+            ? array_merge($data, $requestOptions->getBody())
+            : $data;
 
         $logParams = [
             'body' => $body,
@@ -277,10 +274,9 @@ private function createRequest(
         $protocolVersion = '1.1'
     ) {
         if (is_array($body)) {
-            // Send an empty body instead of "[]" in case there are
-            // no content/params to send
+            // Send an empty valid JSON object
             if (empty($body)) {
-                $body = '';
+                $body = '{}';
             } else {
                 $body = \json_encode($body, $this->jsonOptions);
                 if (JSON_ERROR_NONE !== json_last_error()) {