-
Notifications
You must be signed in to change notification settings - Fork 14
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.
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
feat(clients): add generateSecuredApiKey to java #3167
Conversation
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great
.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) | ||
.disable(SerializationFeature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS) | ||
.disable(DeserializationFeature.READ_DATE_TIMESTAMPS_AS_NANOSECONDS) | ||
.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be configurable no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disabled by default is nice anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually this is only used for the generateSecuredApiKey
helper, the other json serializer used for http body is configurable.
I'm not sure if I can make this one configurable since its static
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is how it's done in the legacy client too, it's just a way to have a json serializer in hard to reach places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah oki i'm fine with that then
@@ -181,10 +181,10 @@ public GetApiKeyResponse waitForApiKey( | |||
} | |||
|
|||
/** | |||
* Helper: Wait for an API key to be added, updated or deleted based on a given `operation`. | |||
* Helper: Wait for an API key to be added or deleted based on a given `operation`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how come it's not updated anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is just an override, because the apiKey param is optional, if you don't provide it you can't do an update, but just below you have the version with apiKey, they supports update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahhh yes my bad then all good
*/ | ||
public Duration getSecuredApiKeyRemainingValidity(@Nonnull String securedApiKey) { | ||
if (securedApiKey == null || securedApiKey.trim().isEmpty()) { | ||
throw new AlgoliaRuntimeException("securedAPIKey must not be empty, null or whitespaces"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically not possible if they used generateSecuredApiKey, right? we should maybe be more precise here and recommend to report the bug or something
non blocking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing forces the user to use this with generateSecuredApiKey
, this is just a safety check, in other language we don't do any check, I'm not sure how to guide the user better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:yes:
🧭 What and Why
🎟 JIRA Ticket: DI-1775
Add
generateSecuredApiKey
andgetSecuredApiKeyRemainingValidity
for java, it is tested in #2798.