feat(java): generic support for hits #829

millotp · 2022-07-15T15:01:15Z

🧭 What and Why

🎟 JIRA Ticket: APIC-575

Generic support for all models in theory, only works for hits and the java client for now.
The goal was to make it as easy as possible to add a new generic, only add x-is-generic to a model in the spec and the property will be propagated to all of it's parent, up to the return type of the operation.

Because java erases type at runtime, we need to pass a additional parameters to inform jackson about the type.

Changes included:

Create GenericPropagator (name subject to discussion, but it rhymes with alligator so it's cool)
Add parameter innerType to generic endpoint

🧪 Test

CI

algolia-bot · 2022-07-15T15:01:50Z

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.
You can still access the code generated on main via this commit.

netlify · 2022-07-15T15:03:51Z

✅ Deploy Preview for api-clients-automation canceled.

Name	Link
🔨 Latest commit	`b3cd596`
🔍 Latest deploy log	https://app.netlify.com/sites/api-clients-automation/deploys/62d9431b472cad0009e08d79

shortcuts

(GG!!!!)

first pass, looks like it's working for Java, what's the issue on the JS support side?

generators/src/main/java/com/algolia/codegen/GenericPropagator.java

scripts/ci/githubActions/createMatrix.ts

generators/src/main/java/com/algolia/codegen/GenericPropagator.java

shortcuts

Looks correct on the tree traversing side, I feel like we definitely need more comments otherwise we will forget if something break

generators/src/main/java/com/algolia/codegen/AlgoliaJavaGenerator.java

generators/src/main/java/com/algolia/codegen/GenericPropagator.java

shortcuts · 2022-07-20T14:06:48Z

generators/src/main/java/com/algolia/codegen/GenericPropagator.java

+  private GenericPropagator() {}
+
+  /**
+   * Add the property x-true-generic to a model or property, meaning it should be replaced with T


What would be a false generic? Couldn't this be the x-is-generic?

True generic means that it can be replaced with T directly, and I needed something different than x-is-generic otherwise the tree traversal might eat it's own tail in markTrueGeneric and go further than wanted.
I didn't actually try it so x-is-generic might be enough but I wanted to be sure not to mix things between spec and generation

Ok make sense! true would imply that we encounter false one so maybe just a naming thing

do you have a better name ? 😁

Maybe rename x-is-generic to x-is-base-generic and all of the propagated ones are x-is-generic, or call them x-is-propagated-generic so the one in the spec remains

the other one are propagated too ahah, maybe x-is-original-generic, or x-is-replaceable-generic ?

IMO only the method and end object (the one in the spec) should be the "base" so x-is-generic is good, the rest should be the propagated ones so x-propagate-generic or keep the child logic

It's just for the naming so no big deal pick whatever sounds good to your ears, I just felt like true was confusing :D

generators/src/main/java/com/algolia/codegen/GenericPropagator.java

generators/src/main/java/com/algolia/codegen/AlgoliaJavaGenerator.java

generators/src/main/java/com/algolia/codegen/GenericPropagator.java

shortcuts

Small typos, feel free to ignore the suggestion, looks nice!! :D

generators/src/main/java/com/algolia/codegen/GenericPropagator.java

shortcuts

Seems good! Nice implem :D

shortcuts · 2022-07-21T12:32:30Z

generators/src/main/java/com/algolia/codegen/GenericPropagator.java

+  /**
+   * Add the property x-propagated-generic to a model or property, meaning it should be replaced
+   * with T directly
+   */
+  private static void setPropagatedGeneric(IJsonSchemaValidationProperties property) {
+    setVendorExtension(property, "x-propagated-generic", true);
+  }
+
+  /**
+   * Add the property x-has-child-generic to a model or property, meaning one of its members is
+   * generic and it should propagate the T
+   */
+  private static void setHasChildGeneric(IJsonSchemaValidationProperties property) {
+    setVendorExtension(property, "x-has-child-generic", true);
+  }


We don't even need them but I can understand you want to keep it to make it clearer

feat(java): generic support for hits

a847f5c

millotp self-assigned this Jul 18, 2022

fix(ci): only remove relevant generated tests

bf31c83

millotp requested a review from shortcuts July 18, 2022 09:20

shortcuts reviewed Jul 18, 2022

View reviewed changes

millotp added 5 commits July 18, 2022 12:05

remove logic

9ad41df

Merge branch 'main' into feat/generic-java

598cd3a

add the type parameter

ceaed41

test cts

77d8333

format

77d307d

millotp requested a review from shortcuts July 20, 2022 12:39

remove log

2f054dd

shortcuts reviewed Jul 20, 2022

View reviewed changes

Merge branch 'main' into feat/generic-java

fafdabe

shortcuts mentioned this pull request Jul 20, 2022

javascript: certain types aren't generic #853

Open

millotp added 5 commits July 20, 2022 18:14

first review

19c2e55

Merge branch 'main' into feat/generic-java

8251e51

rollback

73c38ec

format

6003c0b

rename true generic

f95d80d

millotp requested a review from shortcuts July 21, 2022 09:01

format

a7eb202

shortcuts reviewed Jul 21, 2022

View reviewed changes

millotp added 2 commits July 21, 2022 14:09

Merge branch 'main' into feat/generic-java

989b677

review

b3cd596

millotp requested a review from shortcuts July 21, 2022 12:29

shortcuts approved these changes Jul 21, 2022

View reviewed changes

millotp merged commit d3fb62c into main Jul 21, 2022

millotp deleted the feat/generic-java branch July 21, 2022 12:33

millotp mentioned this pull request Mar 8, 2023

chore: release chore/release-please-test #1386

Closed

feat(java): generic support for hits #829

feat(java): generic support for hits #829

Uh oh!

Conversation

millotp commented Jul 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧭 What and Why

Changes included:

🧪 Test

Uh oh!

algolia-bot commented Jul 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✗ The generated branch has been deleted.

Uh oh!

netlify bot commented Jul 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for api-clients-automation canceled.

Uh oh!

shortcuts left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shortcuts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shortcuts Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

millotp Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

shortcuts Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

millotp Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

shortcuts Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

millotp Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

shortcuts Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shortcuts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shortcuts left a comment

Choose a reason for hiding this comment

Uh oh!

shortcuts Jul 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

millotp commented Jul 15, 2022 •

edited

Loading

algolia-bot commented Jul 15, 2022 •

edited

Loading

netlify bot commented Jul 15, 2022 •

edited

Loading

shortcuts left a comment •

edited

Loading

shortcuts Jul 21, 2022 •

edited

Loading