-
Notifications
You must be signed in to change notification settings - Fork 368
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
Add non-autogen typed params Event/File/EphemeralKeys #730
Add non-autogen typed params Event/File/EphemeralKeys #730
Conversation
update import order on files refactor file test
@@ -0,0 +1,81 @@ | |||
package com.stripe.param; |
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.
There's no change to this file from the generator. But I removed the comment that is generated.
// Generated by com.stripe.generator.entity.SdkBuilder
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.
👍
@@ -0,0 +1,295 @@ | |||
package com.stripe.param; |
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.
No change to this list params from the generated
* <p>Note this field is marked as transient to avoid JSON deserializer. Override | ||
* {@link FileCreateParams#toMap()} makes sure that the returned map has this same file instance. | ||
*/ | ||
transient Object file; |
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.
Change in params. From the OpenAPI spec, we would accept String value for the file.
Instead here, it is Object
because the builder allows setting File
and FileInputStream
. See the builder setters below
public Builder setFile(java.io.FileInputStream file) { | ||
this.file = file; | ||
return this; | ||
} |
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.
Changes here from setting string to setting File.
Object fileObject = this.file; | ||
Map<String, Object> untypedParamWithPrimitiveTypes = super.toMap(); | ||
untypedParamWithPrimitiveTypes.put("file", fileObject); | ||
return untypedParamWithPrimitiveTypes; |
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.
toMap
needs to contain the same file instance set in the builder.
The file
field is set as transient and not serialized by GSON
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.
👍
import java.util.List; | ||
import lombok.Getter; | ||
|
||
public class FileListParams extends ApiRequestParams { |
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.
No change in this params.
@@ -166,6 +166,7 @@ public static String urlEncode(String str) throws UnsupportedEncodingException { | |||
public static <T> T request(ApiResource.RequestMethod method, | |||
String url, ApiRequestParams params, Class<T> clazz, | |||
RequestOptions options) throws StripeException { | |||
checkNullTypedParams(url, params); |
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.
Custom error message here instead of failing in the next line toMap
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.
👍 Nice!
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.
Awesome Mick. Thanks for adding these, and tests too :)
🚢
@@ -166,6 +166,7 @@ public static String urlEncode(String str) throws UnsupportedEncodingException { | |||
public static <T> T request(ApiResource.RequestMethod method, | |||
String url, ApiRequestParams params, Class<T> clazz, | |||
RequestOptions options) throws StripeException { | |||
checkNullTypedParams(url, params); |
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.
👍 Nice!
@@ -0,0 +1,81 @@ | |||
package com.stripe.param; |
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.
👍
Object fileObject = this.file; | ||
Map<String, Object> untypedParamWithPrimitiveTypes = super.toMap(); | ||
untypedParamWithPrimitiveTypes.put("file", fileObject); | ||
return untypedParamWithPrimitiveTypes; |
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.
👍
thank you ob! |
* Drop support for Java 1.7 * Upgrade dev dependencies (except JUnit) * Upgrade to JUnit 5 * Use ErrorProne in builds * Return unsafe deserialized event data object as Option (#723) * return as option * update doc * Autogen params (#705) * test: typed param setup - fix ambiguous call with null casting test: CardTest ambiguous call with typed param * Basic primitives for typed params (#679) * extract common deserializer method extract common logic * implement ApiParamRequest add api param request * Add to base test lenient request params comparison for list/array int/long comparison * use List<Object> instead of Object[] in untyped deserializer * provide documentation to deserializer and api request param * reorder import statments * fix other imports in new files * test: Standardization consider request param as map param * rename and add more test (#718) * Fix remaining ErrorProne warnings * Verify deserialized boolean params to map (#722) * verify boolean behavior * add suppress warning * Generated for V9.0 [generated] source: spec3.sdk.yaml@non-master-spec-fb07de8 in mickjer… (#711) * test: typed params path query expand and limit * test: typed params on methods previously had params as string constant * test: typed params on polymoprhic of EMPTY and array test: use new empty param * test: typed params on inner object * test: typed params by collection methods * test: typed param fix to singular * test: typed param create token with different instruments test: Token with ObjectType enum test: typed params create token * test: typed params create charge test: typed params charge * generated: param * generated: model * generated: param * [generated] source: spec3.sdk.yaml@non-master-spec-d9e92b9 in mickjermsurawong/working-autogen * [ErrorProne fix: add @OverRide to enum get value][generated] source: spec3.sdk.yaml@non-master-spec-d9e92b9 in mickjermsurawong/working-autogen * [Rename Number to card details][generated] source: spec3.sdk.yaml@non-master-spec-c5dbddd in mickjermsurawong/working-autogen * [Sort setter methods][generated] source: spec3.sdk.yaml@non-master-spec-97a62f7 in gen-param-default * test: remove getters from params * test: create webhook with event enums * test: typed param addAll/putAll * [generated] source: spec3.sdk.yaml@non-master-spec-6e9fab7 in mickjermsurawong/default-gen-param * Update doc on previous attributes array representation (#726) * update doc about array representation * fix another typo * Another dependency version bump * Fix docs on getting stripe object from `EventDataObjectDeserializer` (#727) * update docs on get object * Add missing Javadoc * Add non-autogen typed params Event/File/EphemeralKeys (#730) * file create params with handling of file object * create file with typed params * list file with typed params update import order on files refactor file test * add ephemeral key create params * add event list params * null check for typed parameters
This PR implements typed params for the non-autogen classes:
Event
,File
andEphemeralKey
I used generator in this branch to get parameters for the excluded files, and manually remove the special-case logic that we have
For file, there's additional handling of
File
andFileInputStream
for typed params to make it compatible to the current untyped params usage.For ones with custom logic in the method itself, I defer the calls to existing logic, and multi-part request that currently doesn't have interface for typed parameters.
I did not add
EventRetrieve
andFileRetrieve
params because they are essentially no-op params, having only expand while the model actually has nothing expandable.It's a large PR but reviewing by commit would be helpful.
r? @ob-stripe @brandur-stripe
cc @stripe/api-libraries