Skip to content

fix(generator): handle custom request escaping #313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 4, 2022
Merged

fix(generator): handle custom request escaping #313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a1396b9
feat(generator): handle custom request escaping
shortcuts Mar 31, 2022
259c402
include template in PHP cache
shortcuts Mar 31, 2022
0ab6d20
Merge branch 'main' into feat/customRequest-cts
shortcuts Apr 4, 2022
daa1859
bump cache
shortcuts Apr 4, 2022
5a79f54
prevent overlapping paths
shortcuts Apr 4, 2022
558cbf0
remove php config from cache path
shortcuts Apr 4, 2022
370a7ac
Merge branch 'main' into feat/customRequest-cts
shortcuts Apr 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/.cache_version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7
7.0.2
18 changes: 9 additions & 9 deletions .github/actions/cache/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ runs:
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-php
path: clients/algoliasearch-client-php/lib/Api/SearchApi.php
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
Expand All @@ -382,7 +382,7 @@ runs:
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-php
path: clients/algoliasearch-client-php/lib/Api/RecommendApi.php
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will work once we remove the generated code, this is not the only generated file, there are other as listed here,
the easiest is to keep clients/algoliasearch-client-php

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will work once we remove the generated code

This should be tackled once we encounter the issue

the easiest is to keep clients/algoliasearch-client-php

The cache contains the whole folder, not only the changes, so the latest restoration will override all the previous ones

Copy link
Collaborator

@millotp millotp Apr 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should break something to fix it later, is it an issue right now ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache is restored only if nothing has changed in theory, I don't see the issue

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should break something to fix it later, is it an issue right now ?

It does not seem to work right now

This is the commit for the same generated code for same cache path: 1c27d48

key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
Expand All @@ -397,7 +397,7 @@ runs:
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-php
path: clients/algoliasearch-client-php/lib/Api/PersonalizationApi.php
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
Expand All @@ -412,7 +412,7 @@ runs:
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-php
path: clients/algoliasearch-client-php/lib/Api/AnalyticsApi.php
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
Expand All @@ -427,7 +427,7 @@ runs:
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-php
path: clients/algoliasearch-client-php/lib/Api/InsightsApi.php
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
Expand All @@ -442,12 +442,12 @@ runs:
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-php
path: clients/algoliasearch-client-php/lib/Api/AbtestingApi.php
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
'clients/algoliasearch-client-php/lib/Api/AbTestingApi.php',
'clients/algoliasearch-client-php/lib/Configuration/AbTestingConfig.php',
'clients/algoliasearch-client-php/lib/Api/AbtestingApi.php',
'clients/algoliasearch-client-php/lib/Configuration/AbtestingConfig.php',
'specs/bundled/abtesting.yml',
'templates/php/**',
'generators/src/**'
Expand All @@ -457,7 +457,7 @@ runs:
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-php
path: clients/algoliasearch-client-php/lib/Api/QuerySuggestionsApi.php
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
Expand Down
10 changes: 9 additions & 1 deletion .github/workflows/check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,10 @@ jobs:
if: steps.cache.outputs.cache-hit != 'true'
run: yarn cli build clients java ${{ matrix.client.name }}

- name: Show diff for '${{ matrix.client.name }}' client
if: steps.cache.outputs.cache-hit != 'true'
run: git --no-pager diff

client_php:
runs-on: ubuntu-20.04
timeout-minutes: 10
Expand All @@ -289,7 +293,7 @@ jobs:
id: cache
uses: actions/cache@v2
with:
path: ${{ matrix.client.folder }}
path: ${{ format('{0}/lib/Api/{1}.php', matrix.client.folder, matrix.client.api) }}
key: |
${{ env.CACHE_VERSION }}-${{
hashFiles(
Expand All @@ -308,6 +312,10 @@ jobs:
if: steps.cache.outputs.cache-hit != 'true'
run: yarn cli build clients php ${{ matrix.client.name }}

- name: Show diff for '${{ matrix.client.name }}' client
if: steps.cache.outputs.cache-hit != 'true'
run: git --no-pager diff

client_javascript_tests:
runs-on: ubuntu-20.04
timeout-minutes: 10
Expand Down
12 changes: 0 additions & 12 deletions clients/algoliasearch-client-javascript/bundlesize.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,6 @@
{
"path": "packages/recommend/dist/recommend.umd.browser.js",
"maxSize": "3.80KB"
},
{
"path": "packages/client-common/dist/client-common.esm.node.js",
"maxSize": "5.25KB"
},
{
"path": "packages/requester-browser-xhr/dist/requester-browser-xhr.esm.node.js",
"maxSize": "900B"
},
{
"path": "packages/requester-node-http/dist/requester-node-http.esm.node.js",
"maxSize": "1.15KB"
}
]
}
2 changes: 1 addition & 1 deletion openapitools.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@
"invokerPackage": "Algolia\\AlgoliaSearch",
"globalProperty": "apis,supportingFiles",
"additionalProperties": {
"configClassname": "AbTestingConfig",
"configClassname": "AbtestingConfig",
"hasRegionalHost": true,
"allowedRegions": "us-de",
"variableNamingConvention": "camelCase",
Expand Down
6 changes: 4 additions & 2 deletions templates/java/libraries/okhttp-gson/api.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,10 @@ public class {{classname}} extends ApiClient {
Object bodyObj = {{#bodyParam}}{{paramName}}{{/bodyParam}}{{^bodyParam}}null{{/bodyParam}};

// create path and map variables
String requestPath = "{{{path}}}"{{#pathParams}}
.replaceAll("\\{" + "{{baseName}}" + "\\}", this.escapeString({{{paramName}}}.toString())){{/pathParams}};
String requestPath = "{{{path}}}"{{#vendorExtensions}}{{#pathParams}}.replaceAll(
{{=<% %>=}}"\\{<%baseName%>\\}"<%={{ }}=%>,
{{#x-is-custom-request}}{{{paramName}}}.toString(){{/x-is-custom-request}}{{^x-is-custom-request}}this.escapeString({{{paramName}}}.toString()){{/x-is-custom-request}}
){{/pathParams}}{{/vendorExtensions}};

{{javaUtilPrefix}}List<Pair> queryParams = new {{javaUtilPrefix}}ArrayList<Pair>();
{{javaUtilPrefix}}Map<String, String> headers = new {{javaUtilPrefix}}HashMap<String, String>();
Expand Down
9 changes: 3 additions & 6 deletions templates/javascript/api-single.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,9 @@ export function create{{capitalizedApiName}}Api(options: CreateClientOptions{{#h
{{/allParams.0}}
requestOptions?: RequestOptions
) : Promise<{{{returnType}}}> {
const requestPath = '{{{path}}}'{{#pathParams}}.replace(
{{=<% %>=}}
'{<%baseName%>}',
<%={{ }}=%>
encodeURIComponent(String({{paramName}}))
){{/pathParams}};
const requestPath = '{{{path}}}'{{#vendorExtensions}}{{#pathParams}}.replace(
{{=<% %>=}}'{<%baseName%>}'<%={{ }}=%>,{{#x-is-custom-request}}String({{paramName}}){{/x-is-custom-request}}{{^x-is-custom-request}}encodeURIComponent(String({{paramName}})){{/x-is-custom-request}}
){{/pathParams}}{{/vendorExtensions}};
const headers: Headers = {};
const queryParameters: QueryParameters = {};

Expand Down
8 changes: 4 additions & 4 deletions templates/php/api.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ use {{invokerPackage}}\RetryStrategy\ClusterHosts;
}
{{/isExplode}}
{{/queryParams}}
{{#pathParams}}
{{#vendorExtensions}}{{#pathParams}}
// path params
{{#collectionFormat}}
if (is_array(${{paramName}})) {
Expand All @@ -240,12 +240,12 @@ use {{invokerPackage}}\RetryStrategy\ClusterHosts;
{{/collectionFormat}}
if (${{paramName}} !== null) {
$resourcePath = str_replace(
'{' . '{{baseName}}' . '}',
ObjectSerializer::toPathValue(${{paramName}}),
{{=<% %>=}}'{<%baseName%>}'<%={{ }}=%>,
{{#x-is-custom-request}}{{paramName}}{{/x-is-custom-request}}{{^x-is-custom-request}}ObjectSerializer::toPathValue(${{paramName}}){{/x-is-custom-request}},
$resourcePath
);
}
{{/pathParams}}
{{/pathParams}}{{/vendorExtensions}}

{{#bodyParams}}
if (isset(${{paramName}})) {
Expand Down