Skip to content
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 SaleToAcquirerDataSerializer deserialisation #1038

Merged
merged 1 commit into from
Jun 2, 2023

Conversation

scriptease
Copy link
Contributor

@scriptease scriptease commented May 23, 2023

Description
Adds missing deserialization of SaleToAcquirerData for SaleToPOIResponses, e.g. when using the payment provider Twint in Switzerland

Tested scenarios

  • Base 64 Encodeded SaleToAcquirerData was parsed correctly.

Fixed issue: #1037

@jillingk
Copy link
Contributor

Hi @scriptease,

Thanks a lot for taking the time to contribute! Quick question, what kind of request did Twint send to get a SaleToAcquirerData object back in the response? I'm asking because I haven't seen this field in a response before so I just wanted to double check we actually do need to deserialize this.

Your PR looks all good though! Maybe a unit test would be nice, but I can help you with that as well.

Best, Jilling
Adyen

@scriptease
Copy link
Contributor Author

scriptease commented May 24, 2023

It happened in live/production so I don't think I can share the whole response here, however the base64 and the decoded version is just meta data with a applicationInfo.

      "SaleData" : {
        "SaleTransactionID" : {
          "TransactionID" : "...",
          "TimeStamp" : "2023-04-15T14:13:07.000Z"
        },
        "SaleToAcquirerData" : "ewogICJhcHBsaWNhdGlvbkluZm8iOiB7CiAgICAiYWR5ZW5MaWJyYXJ5IjogewogICAgICAibmFtZSI6ICJhZHllbi1qYXZhLWFwaS1saWJyYXJ5IiwKICAgICAgInZlcnNpb24iOiAiMTcuMC4wIgogICAgfQogIH0KfQ=="
      },
{
  "applicationInfo": {
    "adyenLibrary": {
      "name": "adyen-java-api-library",
      "version": "17.0.0"
    }
  }
}

But in my option the model should be transitive in any way, so that a SaleToPOIResponse can be serialized and deserialized without an exception caused by the missing inverse operation in SaleToAcquirerDataSerializer.

In our own unit test the twint response can now be deserialized, serialized and are equal to the original afterwards.

@scriptease
Copy link
Contributor Author

scriptease commented May 24, 2023

I just wanted to double check we actually do need to deserialize this.

We could also ignore it using @ Expose while parsing the json, but without the fix it throws a json processing exception which prohibited us from processing payments, so we already created our own bugfix release and deployed it.

I created the ticket & pr mostly for fellow developers that might run into a similar edge case, I never heard about twint before investigating the issue.

@jillingk
Copy link
Contributor

Okay makes sense to me, I'll approve it. I think in the past we would not encounter these responses, hence why we didn't have this yet. Thanks again for the quick PR!

@jillingk jillingk requested a review from a team June 1, 2023 11:49
@wboereboom wboereboom enabled auto-merge (squash) June 2, 2023 06:42
@wboereboom wboereboom disabled auto-merge June 2, 2023 07:01
@wboereboom wboereboom merged commit d0c8591 into Adyen:develop Jun 2, 2023
2 of 3 checks passed
@jillingk jillingk added the Fix Indicates a bug fix label Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Indicates a bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants