Basic primitives for typed params #679

mickjermsurawong-stripe · 2019-02-19T16:28:52Z

This PR adds support for autogen typed params. The usage examples of generated code in [wip] typed param example #680
It adds abstract class ApiRequestParams to be extended by top-level params, and an inner interface ApiRequestParams.Enum. df04964
- ApiRequestParams exposes to toMap making it compatible with the form-encoding logic. This map is obtained from GSON using the serialized name annotation generated.
- ApiRequestParams.Enum has getter for its raw value, this is used in custom serializer to convert EMPTY enum into null value in the param map. The EMPTY enum (used to unset values in some API) is equivalent to empty string "" in OpenAPI spec, but our current implementation requires it to be explicit null

public class FooParams extends ApiRequestParams {
  String fooString;
  FooEnum fooEnum;

  public FooEnum implements ApiRequestParams.Enum {
    @SerializedName("my_foo")
    MY_FOO("my_foo"),
    @SerializedName("")
    EMPTY("my_foo");
    // implemented method required by enum
    @Getter private final String value
  }
}

This PR also does refactoring in extracting common logic from converting GSON to map that is used in EventDataDeserializer. 6c020c8.
The final logic added here is to add comparison logic in the ParamMapMatcher in testing. This allows more comparison of untyped Map that is robust to difference in Object[] vs List<Object, and Long vs Integer f22e048
r? @ob-stripe
cc @stripe/api-libraries

mickjermsurawong-stripe · 2019-02-19T16:47:00Z

src/main/java/com/stripe/net/ApiRequestParams.java

+  private static final UntypedMapDeserializer UNTYPED_MAP_DESERIALIZER =
+      new UntypedMapDeserializer();
+
+  private static class HasEmptyEnumTypeAdapterFactory implements TypeAdapterFactory {


Main logic of handling serializing EMPTY enum to null

mickjermsurawong-stripe · 2019-02-19T16:47:40Z

src/main/java/com/stripe/net/ApiResource.java

+  public static <T extends StripeCollectionInterface<?>> T requestCollection(
+      String url, ApiRequestParams params, Class<T> clazz, RequestOptions options)
+      throws StripeException {
+    return requestCollection(url, params.toMap(), clazz, options);


Change in ApiResource request invocation to support ApiRequestParams

mickjermsurawong-stripe · 2019-02-19T16:48:16Z

src/main/java/com/stripe/net/UntypedMapDeserializer.java

+import java.util.Map;
+
+
+public class UntypedMapDeserializer {


Old logic ported from EventDataDeserializer. New tests are added to UntypedMapDeserializerTest

And similarly here, can we do a docblock?

mickjermsurawong-stripe · 2019-02-19T16:49:34Z

src/test/java/com/stripe/BaseStripeTest.java

+    }
+  }
+
+  private static Boolean compareParamObjects(Object thisValue, Object otherValue) {


Additional logic to make testing of typed param and comparison with old untyped map easier.
In the subsequent PR, I rely on the existing tests that has untyped params. I want to make sure that using typed params will have the same effect.

mickjermsurawong-stripe · 2019-02-19T16:50:34Z

src/test/java/com/stripe/functional/StripeResponseTest.java

        .setIdempotencyKey(idempotencyKey)
        .build();
-    final Customer customer = Customer.create(null, requestOptions);
+    final Customer customer = Customer.create((Map<String, Object>) null, requestOptions);


Compiler complains that null now can imply both null for untyped param Map or typed ApiRequestParams. Existing integration with null map will likely break loudly with complier checks.

extract common logic

add api param request

…/long comparison

ob-stripe · 2019-02-21T13:33:30Z

src/main/java/com/stripe/net/UntypedMapDeserializer.java

+public class UntypedMapDeserializer {
+  /**
+   * Deserialize JSON into untyped map.
+   * {@code JsonArray} is represented as {@code Object[]}.


I'd prefer that we use lists instead of arrays everywhere in the library. Lists are (IMHO) more flexible and easier to reason about, and make the code easier to maintain. See also #605.

ob-stripe

Left one comment regarding arrays, but I cannot overstate how absolutely awesome this is. Great job Mick, this is going to make a lot of people very happy!

I also really like the implementation. The toMap() method on ApiRequestParams lets us reuse our existing encoding logic without any changes -- very clean!

mickjermsurawong-stripe · 2019-02-21T18:08:56Z

thank youu @ob-stripe for the reviews so far on this stripe-java!
I've updated the untyped deserializer to use List, and the comments in EventData accordingly.
Will add this change to the migration guide as it is not obvious from the API and we might silently break merchant.

mickjermsurawong-stripe · 2019-02-21T18:09:07Z

ptal @ob-stripe

ob-stripe

Awesome, thanks @mickjermsurawong-stripe.

This is complex enough that we might want another set of eyes on this (cc @brandur-stripe), but the tests you've added seem comprehensive enough to me. Feel free to merge!

brandur-stripe

Nice @mickjermsurawong-stripe, and echoing OB — thank you for adding tests!

Left a few minor comments, but otherwise LGTM.

ptal @mickjermsurawong-stripe

brandur-stripe · 2019-02-22T17:09:37Z

src/main/java/com/stripe/net/ApiRequestParams.java

+import java.io.IOException;
+import java.util.Map;
+
+public abstract class ApiRequestParams {


Mind giving this a docblock explaining what it's for?

brandur-stripe · 2019-02-22T17:10:14Z

src/main/java/com/stripe/net/UntypedMapDeserializer.java

+import java.util.Map;
+
+
+public class UntypedMapDeserializer {


And similarly here, can we do a docblock?

brandur-stripe · 2019-02-22T17:10:51Z

src/test/java/com/stripe/BaseStripeTest.java

 import java.util.List;
 import java.util.Map;

+import java.util.Set;


Nit, but can you tighten up the spacing here? I assume java.util.Set belongs with the group above instead of with lombok.*.

Tiny follow along nit: I think what the original author was trying to do here was separate out different "groups" of imports. So all the java would land together, and then you'd have com.*, org.*, etc. in their own sections. If that's the case, I think the convention would be a double space between java.* and lombok.*.

ah sorry @brandur-stripe: I simply used auto-formatting and assumed that was the standard one. I now fixed the import statement in my intellij setting to follow the order of the grouping here. With that though I didn't add double space between java.* and lombok.* as it the current one doesn't.

mickjermsurawong-stripe · 2019-02-22T23:02:05Z

thank you @brandur-stripe and @ob-stripe for the review!
I've updated the java doc, and also import grouping.
ptal @brandur-stripe

brandur-stripe · 2019-02-22T23:23:33Z

Thanks @mickjermsurawong-stripe! Added one more tiny comment, but LGTM.

mickjermsurawong-stripe · 2019-02-22T23:55:57Z

Thank you @brandur-stripe! I fixed the import order in other files I touched here too.

mickjermsurawong-stripe · 2019-02-22T23:58:07Z

thank you! merging this to beta branch

* 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

This reverts commit a1c2175.

* 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

This reverts commit a1c2175.

* 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

This reverts commit a1c2175.

* remove updated resource * src: remove class specific deserializers from ApiResource.java * test: use PaymentSource instead of ExternalAccount for BankAccount and Card in `/v1/customers/...` * test: remove Charge `markSafe/markFraudulent` method in favor of normal update * test: remove LoginLinkCollection for create method, in favor of LoginLink `craeteOnAccount` * test: rename Reversal to TransferReversal Reversal to Transfer Reversal * test: remove UsageRecord `create` taking ID argument in parameters, in favor of `createOnSubscriptionItem` * test: use StripeError and PaymentSource for PaymentIntent instead of PaymentIntentLastPaymentError and ExternalAccount * test: remove `getTypeData` in SourceTransaction and SourceMandateNotification in favor of new typed classes specific to each source type * test: use normal getters for auto-expand issuing.Card in Authorization and CardDetails instead of expandable idiom * test: fix Topup test using incorrect stripe-mock url * test: add deserialzation tests for unknown subtypes on BalanceTransactionSource and ExternalAccount * test: type-cast nulls for ambiguous method calls supporting single argument for both RequestOptions and param Map * src/test: [version-pin] pin API_VERSION and update RequestOptions and EphemeralKey * remove resources to be regenerated * test: add deserialization test to OrderItem expanding parent * "[generated] source: spec3.sdk.yaml@spec-214186957a2324edd15aba2a7d8d03e408f22669 in master"" * src/test: [version-pin] remove Stripe.apiVersion * Interface for handling event deserialization failure due to api version mismatch (#664) * make event data backward compat * implement versioned data * rename work * rename exception * improve deserializeUnsafeWith * add comments using static pinned version * "[generated] source: spec3.sdk.yaml@spec-c6ce0ce334c19193dc22b6b63f47388f25f30d57 in master"" (#666) * "[generated] source: spec3.sdk.yaml@spec-3b52a80 in master"" (#668) * remove resource * remove updated resource * remove updated resource * remove resources * [generated] source: spec3.sdk.yaml@non-master-spec-a8fea08 in HEAD (#671) * [generated] source: spec3.sdk.yaml@non-master-spec-d2b863d in mickjermsurawong/subscription-schedules (#672) * test: Person remove old document expansion * test: Subscription default source becomes PaymentSource * test: typed param setup - fix ambiguous call with null casting test: CardTest ambiguous call with typed param * test: LegalEntity class is removed fix person * 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 * fix: RequestOptions use lombok equal/hashcode instead * test: Standardization consider request param as map param * Update resources to 8.0-beta (#687) * [generated] source: spec3.sdk.yaml@non-master-spec-e27a05e in mickjermsurawong/no-v-change * [generated] source: spec3.sdk.yaml@spec-82bb471 in master * [generated] source: spec3.sdk.yaml@spec-3869a7f in master * [generated] source: spec3.sdk.yaml@non-master-spec-02ef86a in mickjermsurawong/undoc-sensitive-enum * Revert "test: Standardization consider request param as map param" This reverts commit c3c1157. * Revert "Basic primitives for typed params (#679)" This reverts commit a1c2175. * Revert "test: typed param setup - fix ambiguous call with null casting" This reverts commit 1eb0f45. * [generated] source: spec3.sdk.yaml@spec-9d9decd in master (#690) * [generated] source: spec3.sdk.yaml@spec-2662e57 in master (#692) * [generated] source: spec3.sdk.yaml@spec-436464f in master (#693) * remove parts that should be done by script (#696)

* 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

@OverRide

* 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

@OverRide

* 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

@OverRide

* 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

mickjermsurawong-stripe changed the title ~~Support typed params~~ Basic primitives for typed params Feb 19, 2019

mickjermsurawong-stripe mentioned this pull request Feb 19, 2019

[wip] typed param example #680

Closed

stripe-ci assigned ob-stripe Feb 19, 2019

mickjermsurawong-stripe commented Feb 19, 2019

View reviewed changes

mickjermsurawong-stripe force-pushed the autogen/8.0-beta branch from cc57b0a to 5e4fca8 Compare February 21, 2019 05:39

mickjermsurawong-stripe added 3 commits February 20, 2019 21:40

extract common deserializer method

6c020c8

extract common logic

implement ApiParamRequest

df04964

add api param request

Add to base test lenient request params comparison for list/array int…

f22e048

…/long comparison

mickjermsurawong-stripe force-pushed the mickjermsurawong/support-typed-params branch from b5c3ee3 to f22e048 Compare February 21, 2019 05:40

ob-stripe reviewed Feb 21, 2019

View reviewed changes

ob-stripe suggested changes Feb 21, 2019

View reviewed changes

stripe-ci assigned mickjermsurawong-stripe and unassigned ob-stripe Feb 21, 2019

use List<Object> instead of Object[] in untyped deserializer

05e17b7

mickjermsurawong-stripe mentioned this pull request Feb 21, 2019

Migration guide to v8 #667

Closed

ob-stripe approved these changes Feb 21, 2019

View reviewed changes

stripe-ci added the approved label Feb 21, 2019

brandur-stripe reviewed Feb 22, 2019

View reviewed changes

stripe-ci removed the approved label Feb 22, 2019

mickjermsurawong-stripe force-pushed the mickjermsurawong/support-typed-params branch from c7b346d to 4add333 Compare February 22, 2019 22:54

provide documentation to deserializer and api request param

a2e28d2

mickjermsurawong-stripe force-pushed the mickjermsurawong/support-typed-params branch from 4add333 to a2e28d2 Compare February 22, 2019 23:00

mickjermsurawong-stripe added 2 commits February 22, 2019 15:44

reorder import statments

434c9a8

fix other imports in new files

222e82c

brandur-stripe approved these changes Feb 22, 2019

View reviewed changes

stripe-ci added the approved label Feb 22, 2019

mickjermsurawong-stripe merged commit c40db5e into autogen/8.0-beta Feb 22, 2019

mickjermsurawong-stripe added a commit that referenced this pull request Mar 15, 2019

Revert "Basic primitives for typed params (#679)"

ad76780

This reverts commit a1c2175.

mickjermsurawong-stripe added a commit that referenced this pull request Mar 19, 2019

Revert "Basic primitives for typed params (#679)"

80c1853

This reverts commit a1c2175.

mickjermsurawong-stripe added a commit that referenced this pull request Mar 19, 2019

Revert "Basic primitives for typed params (#679)"

5c2fbde

This reverts commit a1c2175.

mickjermsurawong-stripe mentioned this pull request Mar 20, 2019

Autogen params #705

Merged

ob-stripe deleted the mickjermsurawong/support-typed-params branch April 10, 2019 16:34

Basic primitives for typed params #679

Basic primitives for typed params #679

Uh oh!

Conversation

mickjermsurawong-stripe commented Feb 19, 2019 • edited by ob-stripe Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ob-stripe left a comment

Choose a reason for hiding this comment

Uh oh!

mickjermsurawong-stripe commented Feb 21, 2019

Uh oh!

mickjermsurawong-stripe commented Feb 21, 2019

Uh oh!

ob-stripe left a comment

Choose a reason for hiding this comment

Uh oh!

brandur-stripe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mickjermsurawong-stripe commented Feb 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brandur-stripe commented Feb 22, 2019

Uh oh!

mickjermsurawong-stripe commented Feb 22, 2019

Uh oh!

mickjermsurawong-stripe commented Feb 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mickjermsurawong-stripe commented Feb 19, 2019 •

edited by ob-stripe

Loading

mickjermsurawong-stripe commented Feb 22, 2019 •

edited

Loading