Adyen / adyen-openapi

OpenAPI specification for the Adyen APIs
MIT License
67 stars 50 forks source link

Enums have duplicate entries in BalancePlatformService-v1.json #22

Closed andrei-ivanov closed 3 years ago

andrei-ivanov commented 3 years ago

Hi, I'm trying to use an OpenAPI Java generator on BalancePlatformService-v1.json and it says that some enums have duplicate entries: AccountHolder.status: [ "Active", "Closed", "Inactive", "Suspended", "active", "closed", "inactive", "suspended" ]

Phone.type: [ "Landline", "Mobile", "landline", "mobile" ]

PaymentInstrument.status: [ "Active", "Closed", "Inactive", "Lost", "Requested", "Stolen", "Suspended", "active", "blocked", "discarded", "inactive" ]

PaymentInstrumentUpdateRequest.status: [ "Active", "Closed", "Inactive", "Lost", "Requested", "Stolen", "Suspended", "active", "blocked", "discarded", "inactive" ]

PaymentInstrumentInfo.status: [ "Active", "Closed", "Inactive", "Lost", "Requested", "Stolen", "Suspended", "active", "blocked", "discarded", "inactive" ]

I'm an OpenAPI newbie so I'm not sure if these are actually allowed and the generator has to handle them somehow...

andrei-ivanov commented 3 years ago

Also the values don't seem to match the ones described in https://docs.adyen.com/api-explorer/#/balanceplatform/latest/overview

a-akimov commented 3 years ago

Hi @andrei-ivanov,

This is currently done to support migration of these values from PascalCase to camelCase. In the future versions of the API we will leave only camelCase values.

However, this shouldn't cause any problems with the tooling, because, as far as we know, this format is correct from the OpenAPI and JSON Schema points of view (here https://json-schema.org/draft/2020-12/json-schema-validation.html#rfc.section.6.1.2 it says that values should be unique, and these values ARE unique as they are case sensitive).

What is the OpenAPI Java generator that you are using? Maybe we can raise this issue there?

a-akimov commented 3 years ago

Also the values don't seem to match the ones described in https://docs.adyen.com/api-explorer/#/balanceplatform/latest/overview

We've updated the specs today, they should match now. If you still see any discrepancies, please let us know the exact values that are not matching, so we can look more into this.

andrei-ivanov commented 3 years ago

Hi, The generator that I've tried is the java one and in Java the enums look like this:

public enum StatusEnum {
    ACTIVE("Active"),

    CLOSED("Closed"),

    INACTIVE("Inactive"),

    LOST("Lost"),

    REQUESTED("Requested"),

    STOLEN("Stolen"),

    SUSPENDED("Suspended");

    private String value;

    StatusEnum(String value) {
      this.value = value;
    }

    @JsonValue
    public String getValue() {
      return value;
    }

    @Override
    public String toString() {
      return String.valueOf(value);
    }

    @JsonCreator
    public static StatusEnum fromValue(String value) {
      for (StatusEnum b : StatusEnum.values()) {
        if (b.value.equals(value)) {
          return b;
        }
      }
      throw new IllegalArgumentException("Unexpected value '" + value + "'");
    }
}

So while the same value could be written with multiple cases (lower/upper), the generated enum entry is the same.

a-akimov commented 3 years ago

@andrei-ivanov thank you. We are looking into this issue to see how we can improve our OpenAPI generator to avoid value duplication, even if they are in different case.

andrei-ivanov commented 3 years ago

Any conclusion on this issue?

andrei-ivanov commented 3 years ago

Btw, isn't this a new API? How come it has legacy values for enums and why does it has to handle them if it's a new API?

a-akimov commented 3 years ago

@andrei-ivanov yes, it's a bug in our OpenAPI generator, we are looking into the fix.

The reason is that internally it's the same enum for both versions - old and new.

We will publish an update soon.

a-akimov commented 3 years ago

@andrei-ivanov I've pushed the fix, could you please verify that everything is correct now?

andrei-ivanov commented 3 years ago

It works now, but I was under the impression that the enums should use camelCase now 🙂 But then I noticed that the services actually still use the PascalCase values.

andrei-ivanov commented 3 years ago

For example, when creating a test payment instrument, the returned value has a status of inactive, and that doesn't match the value in the spec.

JSON decoding error: Cannot construct instance of com.adyen.balanceplatform.model.PaymentInstrument$StatusEnum, problem: Unexpected value 'inactive'

Same for active, after activating a card.

JSON decoding error: Cannot construct instance of com.adyen.balanceplatform.model.PaymentInstrument$StatusEnum, problem: Unexpected value 'active'

So it seems the services really use camelCase and the spec should match, otherwise the generated code will not work.

a-akimov commented 3 years ago

@andrei-ivanov are you using v1 or v2 of the API?

andrei-ivanov commented 3 years ago

v2, as recommended 😁

andrei-ivanov commented 3 years ago

Testing now with v1, indeed, I shouldn't have mixed them

andrei-ivanov commented 3 years ago

Ok, with v1, the only flow I have, create card and then activate card, works with these values 🙂

Thanks.