Skip to content

Commit

Permalink
Fix request body compression
Browse files Browse the repository at this point in the history
- Url params are now omitted from the url passed to the api client
- This was breaking some startsWith logic for requests that we compress
based on both url params and the uri
- To fix this, we now match against both the url string and the param
object
  • Loading branch information
Luke-Sikina committed May 19, 2022
1 parent 7fed092 commit d69ce5d
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 10 deletions.
52 changes: 51 additions & 1 deletion src/config/config.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ServerConfigHelpers } from './config';
import { ServerConfigHelpers, UrlParamPair, pairMatchesPath } from './config';
import { assert } from 'chai';

describe('ServerConfigHelpers', () => {
Expand Down Expand Up @@ -29,4 +29,54 @@ describe('ServerConfigHelpers', () => {
});
});
});

describe('pairMatchesPath', () => {
it('should match exact match', () => {
const pair: UrlParamPair = { url: '/foo/', params: { a: '1' } };
const ans = pairMatchesPath(pair, '/foo/bar/baz', { a: '1' });

assert.isTrue(ans);
});

it('should match exact match with empty params', () => {
const pair: UrlParamPair = { url: '/foo/', params: {} };
const ans = pairMatchesPath(pair, '/foo/bar/baz', {});

assert.isTrue(ans);
});

it('should match exact match with extra params', () => {
const pair: UrlParamPair = { url: '/foo/', params: { a: '1' } };
const ans = pairMatchesPath(pair, '/foo/bar/baz', {
a: '1',
b: '1',
});

assert.isTrue(ans);
});

it('should not match exact when missing all params', () => {
const pair: UrlParamPair = { url: '/foo/', params: { a: '1' } };
const ans = pairMatchesPath(pair, '/foo/bar/baz', {});

assert.isFalse(ans);
});

it('should not match exact when missing some params', () => {
const pair: UrlParamPair = {
url: '/foo/',
params: { a: '1', b: '1' },
};
const ans = pairMatchesPath(pair, '/foo/bar/baz', { a: '1' });

assert.isFalse(ans);
});

it('should not match when param has different val', () => {
const pair: UrlParamPair = { url: '/foo/', params: { a: '1' } };
const ans = pairMatchesPath(pair, '/foo/bar/baz', { a: '2' });

assert.isFalse(ans);
});
});
});
49 changes: 40 additions & 9 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,24 @@ function cachePostMethods(
);
}

type UrlParamPair = {
url: string;
params: { [key: string]: string };
};

function pairMatchesPath(
pair: UrlParamPair,
url: string,
params: any
): boolean {
return (
url.startsWith(pair.url) &&
Object.keys(pair.params).filter(
k => params[k] && params[k] === pair.params[k]
).length == Object.keys(pair.params).length
);
}

export function initializeAPIClients() {
// we need to set the domain of our api clients
(client as any).domain = getCbioPortalApiUrl();
Expand Down Expand Up @@ -187,12 +205,18 @@ export function initializeAPIClients() {
compressRequestBodies(
CBioPortalAPI,
[
'/mutations/fetch',
'/patients/fetch',
'/molecular-data/fetch',
'/clinical-data/fetch?clinicalDataType=SAMPLE',
'/gene-panel-data/fetch',
'/clinical-data/fetch?clinicalDataType=PATIENT',
{ url: '/mutations/fetch', params: {} },
{ url: '/patients/fetch', params: {} },
{ url: '/molecular-data/fetch', params: {} },
{
url: '/clinical-data/fetch',
params: { clinicalDataType: 'SAMPLE' },
},
{ url: '/gene-panel-data/fetch', params: {} },
{
url: '/clinical-data/fetch',
params: { clinicalDataType: 'PATIENT' },
},
],
getCbioPortalApiUrl()
);
Expand All @@ -209,10 +233,15 @@ export function initializeAPIClients() {
*/
function compressRequestBodies(
apiClient: any,
urlsToCompress: string[],
urlsToCompress: UrlParamPair[],
domain: string
): void {
urlsToCompress = urlsToCompress.map(url => domain + url);
urlsToCompress = urlsToCompress.map(pair => {
return {
url: domain + pair.url,
params: pair.params,
};
});

const oldRequestFunc = apiClient.prototype.request;

Expand All @@ -229,7 +258,9 @@ function compressRequestBodies(
) => {
if (
method === 'POST' &&
urlsToCompress.filter(match => url.startsWith(match)).length > 0
urlsToCompress.filter(pair =>
pairMatchesPath(pair, url, queryParameters)
).length > 0
) {
headers['Content-Encoding'] = 'gzip';
body = pako.gzip(JSON.stringify(body)).buffer;
Expand Down

0 comments on commit d69ce5d

Please sign in to comment.