Fix/domain list next page token (#393)

* moving nextPageToken parsing into bodyTransform and responseTransform layer

* adding null check

* cleanup console log

* reduce domain list page size now that pagination is properly fixed.

* fixed breaking unit test

* refactoring combine function to switch argument positions. reuse cliTransform. rename w next page token request transform.

* removing unused function

* removing special condition checks

* removing unused function

* fixing integration tests

* removing responseTransform for ListDomains

* refactoring code for nextPageToken for all places used in node server. This is replaced with 1 common bodyTransform function.

* removing debug log

Author: just-at-uber

client/containers/domain-autocomplete/getters.js

@@ -50,11 +50,11 @@ const getters = {
       domainList,
     });
 
-    return combine(combinedDomainList)(
+    return combine(
       sortDomainList,
       filterTopDomainList,
       formatDomainList
-    );
+    )(combinedDomainList);
   },
   [DOMAIN_AUTOCOMPLETE_DOMAIN_LIST]: state =>
     sortDomainList(get(state, statePrefix('domainList')) || []),

client/helpers/combine.js

@@ -19,7 +19,7 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-const combine = data => (...callbacks) =>
+const combine = (...callbacks) => data =>
   callbacks.reduce(
     (accumulator, callback) => (accumulator = callback(accumulator)),
     data

server/router/helpers/combine.js server/helpers/combine.jsserver/router/helpers/combine.js renamed to server/helpers/combine.js

@@ -19,7 +19,7 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-const combine = data => (...callbacks) =>
+const combine = (...callbacks) => data =>
   callbacks.reduce(
     (accumulator, callback) => (accumulator = callback(accumulator)),
     data

server/helpers/index.js

@@ -0,0 +1,26 @@
+// Copyright (c) 2021 Uber Technologies Inc.
+//
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+const combine = require('./combine');
+
+module.exports = {
+  combine,
+};

server/middleware/tchannel-client/helpers/index.js

@@ -27,8 +27,8 @@ const lookupAsync = require('./lookup-async');
 const makeChannels = require('./make-channels');
 const makeRequest = require('./make-request');
 const uiTransform = require('./ui-transform');
-const withDomainPagingAndWorkflowExecution = require('./with-domain-paging-and-workflow-execution');
 const withDomainPaging = require('./with-domain-paging');
+const withNextPageTokenBodyTransform = require('./with-next-page-token-body-transform');
 const withVerboseWorkflowExecution = require('./with-verbose-workflow-execution');
 const withWorkflowExecution = require('./with-workflow-execution');
 
@@ -41,8 +41,8 @@ module.exports = {
   makeChannels,
   makeRequest,
   uiTransform,
-  withDomainPagingAndWorkflowExecution,
   withDomainPaging,
+  withNextPageTokenBodyTransform,
   withVerboseWorkflowExecution,
   withWorkflowExecution,
 };

server/middleware/tchannel-client/helpers/make-request.js

@@ -23,13 +23,14 @@ const formatBody = require('./format-body');
 const formatMethod = require('./format-method');
 const formatRequestName = require('./format-request-name');
 const uiTransform = require('./ui-transform');
+const withNextPageTokenBodyTransform = require('./with-next-page-token-body-transform');
 
 const makeRequest = ({ authTokenHeaders, channels, ctx, requestConfig }) => ({
-  bodyTransform,
+  bodyTransform = withNextPageTokenBodyTransform,
   channelName = 'cadence',
   method,
   requestName,
-  responseTransform,
+  responseTransform = uiTransform,
   serviceName = 'WorkflowService',
 }) => body =>
   new Promise((resolve, reject) => {
@@ -50,7 +51,7 @@ const makeRequest = ({ authTokenHeaders, channels, ctx, requestConfig }) => ({
             if (error) {
               reject(error);
             } else if (response.ok) {
-              resolve((responseTransform || uiTransform)(response.body));
+              resolve(responseTransform(response.body));
             } else {
               ctx.throw(
                 response.typeName === 'entityNotExistError' ? 404 : 400,

…-domain-paging-and-workflow-execution.js …s/with-next-page-token-body-transform.jsserver/middleware/tchannel-client/helpers/with-domain-paging-and-workflow-execution.js renamed to server/middleware/tchannel-client/helpers/with-next-page-token-body-transform.js server/middleware/tchannel-client/helpers/with-domain-paging-and-workflow-execution.js renamed to server/middleware/tchannel-client/helpers/with-next-page-token-body-transform.js

@@ -19,10 +19,18 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-const withDomainPaging = require('./with-domain-paging');
-const withWorkflowExecution = require('./with-workflow-execution');
+const withNextPageTokenBodyTransform = body => {
+  if (!body || typeof body !== 'object') {
+    return body;
+  }
 
-const withDomainPagingAndWorkflowExecution = ctx => body =>
-  Object.assign(withDomainPaging(ctx)(body), withWorkflowExecution(ctx)(body));
+  const { nextPageToken } = body;
 
-module.exports = withDomainPagingAndWorkflowExecution;
+  body.nextPageToken = nextPageToken
+    ? Buffer.from(nextPageToken, 'base64')
+    : undefined;
+
+  return body;
+};
+
+module.exports = withNextPageTokenBodyTransform;

server/middleware/tchannel-client/index.js

@@ -23,14 +23,16 @@
 
 const TChannel = require('tchannel');
 
+const { combine } = require('../../helpers');
+
 const {
   cliTransform,
   makeChannels,
   makeRequest,
   withDomainPaging,
+  withNextPageTokenBodyTransform,
   withWorkflowExecution,
   withVerboseWorkflowExecution,
-  withDomainPagingAndWorkflowExecution,
 } = require('./helpers');
 
 const tchannelClient = ({ peers, requestConfig }) =>
@@ -50,12 +52,18 @@ const tchannelClient = ({ peers, requestConfig }) =>
       archivedWorkflows: request({
         method: 'ListArchivedWorkflowExecutions',
         requestName: 'list',
-        bodyTransform: withDomainPaging(ctx),
+        bodyTransform: combine(
+          withDomainPaging(ctx),
+          withNextPageTokenBodyTransform
+        ),
       }),
       closedWorkflows: request({
         method: 'ListClosedWorkflowExecutions',
         requestName: 'list',
-        bodyTransform: withDomainPaging(ctx),
+        bodyTransform: combine(
+          withDomainPaging(ctx),
+          withNextPageTokenBodyTransform
+        ),
       }),
       describeCluster: request({
         channelName: 'admin',
@@ -78,13 +86,21 @@ const tchannelClient = ({ peers, requestConfig }) =>
       exportHistory: request({
         method: 'GetWorkflowExecutionHistory',
         requestName: 'get',
-        bodyTransform: withDomainPagingAndWorkflowExecution(ctx),
+        bodyTransform: combine(
+          withDomainPaging(ctx),
+          withWorkflowExecution(ctx),
+          withNextPageTokenBodyTransform
+        ),
         responseTransform: cliTransform,
       }),
       getHistory: request({
         method: 'GetWorkflowExecutionHistory',
         requestName: 'get',
-        bodyTransform: withDomainPagingAndWorkflowExecution(ctx),
+        bodyTransform: combine(
+          withDomainPaging(ctx),
+          withWorkflowExecution(ctx),
+          withNextPageTokenBodyTransform
+        ),
       }),
       listDomains: request({
         method: 'ListDomains',
@@ -96,12 +112,18 @@ const tchannelClient = ({ peers, requestConfig }) =>
       listWorkflows: request({
         method: 'ListWorkflowExecutions',
         requestName: 'list',
-        bodyTransform: withDomainPaging(ctx),
+        bodyTransform: combine(
+          withDomainPaging(ctx),
+          withNextPageTokenBodyTransform
+        ),
       }),
       openWorkflows: request({
         method: 'ListOpenWorkflowExecutions',
         requestName: 'list',
-        bodyTransform: withDomainPaging(ctx),
+        bodyTransform: combine(
+          withDomainPaging(ctx),
+          withNextPageTokenBodyTransform
+        ),
       }),
       queryWorkflow: request({
         method: 'QueryWorkflow',

server/router/helpers/index.js

@@ -20,7 +20,6 @@
 // THE SOFTWARE.
 
 const buildQueryString = require('./build-query-string');
-const combine = require('./combine');
 const delay = require('./delay');
 const injectDomainIntoWorkflowList = require('./inject-domain-into-workflow-list');
 const isAdvancedVisibilityEnabled = require('./is-advanced-visibility-enabled');
@@ -31,7 +30,6 @@ const replacer = require('./replacer');
 
 module.exports = {
   buildQueryString,
-  combine,
   delay,
   injectDomainIntoWorkflowList,
   isAdvancedVisibilityEnabled,

server/router/helpers/list-workflows.js

@@ -47,8 +47,6 @@ async function listWorkflows({ clusterService, state }, ctx) {
   const earliestTime = momentToLong(startTime);
   const latestTime = momentToLong(endTime);
   const { nextPageToken, status, workflowId, workflowName } = query;
-  const nextPageTokenBuffer =
-    nextPageToken && Buffer.from(nextPageToken, 'base64');
 
   const requestArgs = advancedVisibility
     ? {
@@ -65,7 +63,7 @@ async function listWorkflows({ clusterService, state }, ctx) {
         ...(workflowName && { typeFilter: { name: workflowName } }),
         ...(workflowId && { executionFilter: { workflowId } }),
         ...(status && { statusFilter: status }),
-        ...(nextPageTokenBuffer && { nextPageToken: nextPageTokenBuffer }),
+        nextPageToken,
       };
 
   const requestApi = advancedVisibility ? 'listWorkflows' : state + 'Workflows';