Skip to content

Commit

Permalink
refactor: Represent query params as an array in order to allow multip…
Browse files Browse the repository at this point in the history
…le params with the same key

Some APIs want you to pass multiple values for the same key by duplicating the key value pair
in the query string (`?type=one&type=two`).  In order to support this, query params must be
stored in something other than a map (which enforces key uniqueness). Other options like passing
an array or list to a single key is not viable because other APIs choose to encode this into
their APIs.  In short, `?type=one,two`, `?type=['one','two']` and `?type=one&type=two` are
all valid use cases depending on the API.
  • Loading branch information
elpete committed Jul 29, 2022
1 parent 25383d5 commit 2cb963e
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 24 deletions.
6 changes: 3 additions & 3 deletions models/CfhttpHttpClient.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ component implements="HyperHttpClientInterface" {
}

var queryParams = req.getQueryParams();
for ( var name in queryParams ) {
for ( var param in queryParams ) {
cfhttpparam(
type = "url",
name = name,
value = queryParams[ name ]
name = param.name,
value = param.value
);
}

Expand Down
84 changes: 66 additions & 18 deletions models/HyperRequest.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ component accessors="true" {
function init( httpClient = new CfhttpHttpClient() ) {
variables.requestID = createUUID();
variables.httpClient = arguments.httpClient;
variables.queryParams = createObject( "java", "java.util.LinkedHashMap" ).init();
variables.queryParams = [];
variables.headers = createObject( "java", "java.util.LinkedHashMap" ).init();
variables.headers.put( "Content-Type", "application/json" );
variables.files = [];
Expand Down Expand Up @@ -189,7 +189,8 @@ component accessors="true" {
setUrl( arguments.url );
}
if ( !isNull( arguments.queryParams ) ) {
setQueryParams( arguments.queryParams );
setQueryParams( [] );
withQueryParams( arguments.queryParams );
}
setMethod( "GET" );
return send();
Expand All @@ -208,7 +209,8 @@ component accessors="true" {
setUrl( arguments.url );
}
if ( !isNull( arguments.queryParams ) ) {
setQueryParams( arguments.queryParams );
setQueryParams( [] );
withQueryParams( arguments.queryParams );
}
setMethod( "GET" );
return sendAsync();
Expand Down Expand Up @@ -398,14 +400,34 @@ component accessors="true" {

/**
* Set a query parameter for the request.
* This will delete any existing query params for the key first.
*
* @name The name of the query parameter.
* @value The value of the query parameter.
*
* @returns The HyperRequest instance.
*/
function setQueryParam( name, value ) {
variables.queryParams[ name ] = value;
for ( var i = 1; i <= variables.queryParams.len(); i++ ) {
var param = variables.queryParams[ i ];
if ( param.name == arguments.name ) {
variables.queryParams.deleteAt( i );
}
}
appendQueryParam( arguments.name, arguments.value );
return this;
}

/**
* Append a query parameter for the request.
*
* @name The name of the query parameter.
* @value The value of the query parameter.
*
* @returns The HyperRequest instance.
*/
function appendQueryParam( name, value ) {
variables.queryParams.append( { "name": arguments.name, "value": arguments.value } );
return this;
}

Expand All @@ -417,19 +439,45 @@ component accessors="true" {
* @returns True if the query parameter exists.
*/
function hasQueryParam( name ) {
return variables.queryParams.containsKey( name );
for ( var i = 1; i <= variables.queryParams.len(); i++ ) {
var param = variables.queryParams[ i ];
if ( param.name == arguments.name ) {
return true;
}
}
return false;
}

/**
* Get the value for a certian query parameter.
* Get the first value for a certian query parameter.
*
* @name The name of the query parameter to retrieve its value.
*
* @returns The value of the query parameter.
* Returns an empty string if the query parameter does not exist.
*/
function getQueryParam( name ) {
return hasQueryParam( name ) ? variables.queryParams.get( name ) : "";
for ( var i = 1; i <= variables.queryParams.len(); i++ ) {
var param = variables.queryParams[ i ];
if ( param.name == arguments.name ) {
return param.value;
}
}
return "";
}

/**
* Get all the values for a certian query parameter.
*
* @name The name of the query parameter to retrieve its value.
*
* @returns An array of values for the query parameter.
* Returns an empty array if the query parameter does not exist.
*/
function getAllQueryParam( name ) {
return variables.queryParams.filter( function( param ) {
return param.name == name;
} );
}

/**
Expand Down Expand Up @@ -802,7 +850,7 @@ component accessors="true" {
setBody( "" );
setBodyFormat( "json" );
setReferrer( javacast( "null", "" ) );
variables.queryParams = createObject( "java", "java.util.LinkedHashMap" ).init();
variables.queryParams = [];
variables.headers = createObject( "java", "java.util.LinkedHashMap" ).init();
variables.headers.put( "Content-Type", "application/json" );
variables.files = [];
Expand Down Expand Up @@ -861,7 +909,7 @@ component accessors="true" {
req.setBodyFormat( variables.bodyFormat );
req.setReferrer( isNull( variables.referrer ) ? javacast( "null", "" ) : variables.referrer );
req.setHeaders( variables.headers.clone() );
req.setQueryParams( variables.queryParams.clone() );
req.setQueryParams( duplicate( variables.queryParams ) );
req.setFiles( duplicate( variables.files ) );
req.setThrowOnError( variables.throwOnError );
req.setClientCert( isNull( variables.clientCert ) ? javacast( "null", "" ) : variables.clientCert );
Expand Down Expand Up @@ -890,7 +938,7 @@ component accessors="true" {
for ( var paramString in queryParams ) {
var name = decodeFromURL( listFirst( paramString, "=" ) );
var value = decodeFromURL( listRest( paramString, "=" ) );
setQueryParam( name, value );
appendQueryParam( name, value );
}
return listFirst( arguments.url, "?" );
}
Expand All @@ -904,14 +952,14 @@ component accessors="true" {
if ( variables.queryParams.isEmpty() ) {
return "";
}
var queryParamNames = [];
for ( var name in variables.queryParams ) {
queryParamNames.append( name );
}
queryParamNames.sort( "textnocase" );
return "?" & queryParamNames
.map( function( name ) {
return len( variables.queryParams[ name ] ) ? "#encodeForURL( name )#=#encodeForURL( variables.queryParams[ name ] )#" : "#encodeForURL( name )#";

variables.queryParams.sort( function( paramA, paramB ) {
return compare( paramA.name, paramB.name );
} );

return "?" & variables.queryParams
.map( function( param ) {
return len( param.value ) ? "#encodeForURL( param.name )#=#encodeForURL( param.value )#" : "#encodeForURL( param.name )#";
} )
.toList( "&" );
}
Expand Down
6 changes: 3 additions & 3 deletions tests/specs/integration/InterceptorsSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ component extends="tests.resources.ModuleIntegrationSpec" appMapping="/app" {
var res = hyper.get( "https://jsonplaceholder.typicode.com/posts/1" );
expect( variables.onHyperRequestCalls ).toHaveLength( 1 );
var originalParams = res.getRequest().getQueryParams();
expect( originalParams ).toBeStruct();
expect( originalParams ).toBeArray();
expect( originalParams ).toHaveLength( 1 );
expect( originalParams ).toHaveKey( "added_in_interceptor" );
expect( originalParams[ "added_in_interceptor" ] ).toBe( "true" );
expect( originalParams[ 1 ].name ).toBe( "added_in_interceptor" );
expect( originalParams[ 1 ].value ).toBe( true );
} );

it( "can run an interceptor onHyperResponse", function() {
Expand Down

0 comments on commit 2cb963e

Please sign in to comment.