Skip to content
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

fix(specs): recommendHit is generic #4358

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix(specs): recommendHit is generic #4358

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78509da
fix(specs): recommendHit is generic
millotp Jan 14, 2025
721826c
refacto
millotp Jan 15, 2025
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
4 changes: 4 additions & 0 deletions generators/src/main/java/com/algolia/codegen/cts/manager/JavaCTSManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,9 @@ public void addSnippetsSupportingFiles(List<SupportingFile> supportingFiles, Str
public void addDataToBundle(Map<String, Object> bundle) throws GeneratorException {
bundle.put("packageVersion", getVersion());
bundle.put("import", Helpers.camelize(this.client).toLowerCase());
bundle.put("defaultGeneric", "Hit.class");
if (this.client.equals("recommend")) {
bundle.put("defaultGeneric", "RecommendHit.class");
}
Comment on lines +49 to +52
Copy link
Member

Choose a reason for hiding this comment

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

should it be controlled at the spec level with something like x-defaultGeneric maybe? I guess that's rare enough to not optimize it

}
}
52 changes: 30 additions & 22 deletions generators/src/main/java/com/algolia/codegen/utils/GenericPropagator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,14 @@ private static boolean hasGeneric(IJsonSchemaValidationProperties property) {
if (property instanceof CodegenModel) {
return (
(boolean) ((CodegenModel) property).vendorExtensions.getOrDefault("x-propagated-generic", false) ||
(boolean) ((CodegenModel) property).vendorExtensions.getOrDefault("x-has-child-generic", false)
(boolean) ((CodegenModel) property).vendorExtensions.getOrDefault("x-has-child-generic", false) ||
(boolean) ((CodegenModel) property).vendorExtensions.getOrDefault("x-is-generic", false)
);
} else if (property instanceof CodegenProperty) {
return (
(boolean) ((CodegenProperty) property).vendorExtensions.getOrDefault("x-propagated-generic", false) ||
(boolean) ((CodegenProperty) property).vendorExtensions.getOrDefault("x-has-child-generic", false)
(boolean) ((CodegenProperty) property).vendorExtensions.getOrDefault("x-has-child-generic", false) ||
(boolean) ((CodegenProperty) property).vendorExtensions.getOrDefault("x-is-generic", false)
);
}
return false;
Expand All @@ -78,16 +80,13 @@ private static boolean markPropagatedGeneric(
}
// if items itself isn't generic, we recurse on its items and properties until we reach the
// end or find a generic property
if (
items != null &&
((boolean) items.vendorExtensions.getOrDefault("x-is-generic", false) || markPropagatedGeneric(items, getVar, skipOneOf))
) {
if (items != null && (hasGeneric(items) || markPropagatedGeneric(items, getVar, skipOneOf))) {
setPropagatedGeneric(model);
return true;
}
for (CodegenProperty variable : getVar.apply(model)) {
// same thing for the variable, if it's not a generic, we recurse on it until we find one
if ((boolean) variable.vendorExtensions.getOrDefault("x-is-generic", false) || markPropagatedGeneric(variable, getVar, skipOneOf)) {
if (hasGeneric(items) || markPropagatedGeneric(variable, getVar, skipOneOf)) {
setPropagatedGeneric(model);
return true;
}
Expand Down Expand Up @@ -122,12 +121,17 @@ private static boolean propagateGenericRecursive(
return false;
}

private static void setGenericToComposedSchema(Map<String, CodegenModel> models, List<CodegenProperty> composedSchemas) {
private static void setGenericToComposedSchema(
Map<String, CodegenModel> models,
CodegenModel model,
List<CodegenProperty> composedSchemas
) {
if (composedSchemas == null) {
return;
}
for (CodegenProperty prop : composedSchemas) {
if (hasGeneric(propertyToModel(models, prop))) {
if (hasGeneric(prop) || hasGeneric(propertyToModel(models, prop))) {
setHasChildGeneric(model);
setHasChildGeneric(prop);
}
}
Expand All @@ -138,9 +142,9 @@ private static void propagateToComposedSchema(Map<String, CodegenModel> models,
if (composedSchemas == null) {
return;
}
setGenericToComposedSchema(models, composedSchemas.getOneOf());
setGenericToComposedSchema(models, composedSchemas.getAllOf());
setGenericToComposedSchema(models, composedSchemas.getAnyOf());
setGenericToComposedSchema(models, model, composedSchemas.getOneOf());
setGenericToComposedSchema(models, model, composedSchemas.getAllOf());
setGenericToComposedSchema(models, model, composedSchemas.getAnyOf());
}

private static Map<String, CodegenModel> convertToMap(Map<String, ModelsMap> models) {
Expand Down Expand Up @@ -172,18 +176,22 @@ public static void propagateGenericsToModels(Map<String, ModelsMap> modelsMap, b

Map<String, CodegenModel> models = convertToMap(modelsMap);

for (CodegenModel model : models.values()) {
markPropagatedGeneric(model, m -> m.getVars(), skipOneOf);
markPropagatedGeneric(model, m -> m.getRequiredVars(), skipOneOf);
}
// we don't know in which order the model will come, so we iterate multiple times to make sure
// models at any depth are propagated
for (int i = 0; i < 5; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

🧙🏻

for (CodegenModel model : models.values()) {
markPropagatedGeneric(model, m -> m.getVars(), skipOneOf);
markPropagatedGeneric(model, m -> m.getRequiredVars(), skipOneOf);
}

for (CodegenModel model : models.values()) {
propagateGenericRecursive(models, model, m -> m.getVars());
propagateGenericRecursive(models, model, m -> m.getRequiredVars());
}
for (CodegenModel model : models.values()) {
propagateGenericRecursive(models, model, m -> m.getVars());
propagateGenericRecursive(models, model, m -> m.getRequiredVars());
}

for (CodegenModel model : models.values()) {
propagateToComposedSchema(models, model);
for (CodegenModel model : models.values()) {
propagateToComposedSchema(models, model);
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions specs/recommend/common/schemas/RecommendationsResponse.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ recommendationsHits:
- hits

recommendationsHit:
x-is-generic: true
oneOf:
- $ref: '#/recommendHit'
- $ref: '#/trendingFacetHit'

recommendHit:
type: object
description: Recommend hit.
x-is-generic: true
additionalProperties: true
required:
- objectID
Expand Down
1 change: 0 additions & 1 deletion specs/recommend/paths/batchRecommendRules.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ post:
title: rules
type: array
description: Recommend rules.
additionalProperties: false
items:
$ref: '../common/schemas/RecommendRule.yml#/RecommendRule'

Expand Down
2 changes: 1 addition & 1 deletion templates/java/tests/method.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
client.{{method}}({{#parametersWithDataType}}{{> tests/generateParams}}{{^-last}},{{/-last}}{{/parametersWithDataType}}{{#isGeneric}}, Hit.class{{/isGeneric}}{{#hasRequestOptions}}, new RequestOptions()
client.{{method}}({{#parametersWithDataType}}{{> tests/generateParams}}{{^-last}},{{/-last}}{{/parametersWithDataType}}{{#isGeneric}}, {{defaultGeneric}}{{/isGeneric}}{{#hasRequestOptions}}, new RequestOptions()
{{#requestOptions.queryParameters.parametersWithDataType}}
.addExtraQueryParameters("{{{key}}}", {{> tests/generateInnerParams}})
{{/requestOptions.queryParameters.parametersWithDataType}}
Expand Down
4 changes: 3 additions & 1 deletion templates/javascript/clients/client/model/types/isGeneric.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
{{#vendorExtensions.x-has-child-generic}}
<T = Record<string, unknown>>
{{/vendorExtensions.x-has-child-generic}}
{{^vendorExtensions.x-has-child-generic}}
{{#vendorExtensions.x-is-generic}}
<T = Record<string, unknown>>
{{/vendorExtensions.x-is-generic}}
{{/vendorExtensions.x-is-generic}}
{{/vendorExtensions.x-has-child-generic}}
18 changes: 9 additions & 9 deletions tests/output/javascript/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ __metadata:
"@types/node": "npm:22.10.5"
algoliasearch: "link:../../../clients/algoliasearch-client-javascript/packages/algoliasearch"
dotenv: "npm:16.4.7"
typescript: "npm:5.7.2"
typescript: "npm:5.7.3"
vitest: "npm:2.1.8"
languageName: unknown
linkType: soft
Expand Down Expand Up @@ -1553,23 +1553,23 @@ __metadata:
languageName: node
linkType: hard

"typescript@npm:5.7.2":
version: 5.7.2
resolution: "typescript@npm:5.7.2"
"typescript@npm:5.7.3":
version: 5.7.3
resolution: "typescript@npm:5.7.3"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: 10/4caa3904df69db9d4a8bedc31bafc1e19ffb7b24fbde2997a1633ae1398d0de5bdbf8daf602ccf3b23faddf1aeeb9b795223a2ed9c9a4fdcaf07bfde114a401a
checksum: 10/6a7e556de91db3d34dc51cd2600e8e91f4c312acd8e52792f243c7818dfadb27bae677175fad6947f9c81efb6c57eb6b2d0c736f196a6ee2f1f7d57b74fc92fa
languageName: node
linkType: hard

"typescript@patch:typescript@npm%3A5.7.2#optional!builtin<compat/typescript>":
version: 5.7.2
resolution: "typescript@patch:typescript@npm%3A5.7.2#optional!builtin<compat/typescript>::version=5.7.2&hash=5786d5"
"typescript@patch:typescript@npm%3A5.7.3#optional!builtin<compat/typescript>":
version: 5.7.3
resolution: "typescript@patch:typescript@npm%3A5.7.3#optional!builtin<compat/typescript>::version=5.7.3&hash=5786d5"
bin:
tsc: bin/tsc
tsserver: bin/tsserver
checksum: 10/d75ca10141afc64fd3474b41a8b082b640555bed388d237558aed64e5827ddadb48f90932c7f4205883f18f5bcab8b6a739a2cfac95855604b0dfeb34bc2f3eb
checksum: 10/dc58d777eb4c01973f7fbf1fd808aad49a0efdf545528dab9b07d94fdcb65b8751742804c3057e9619a4627f2d9cc85547fdd49d9f4326992ad0181b49e61d81
languageName: node
linkType: hard

Expand Down
Loading