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

[SR-9385] Encoded data was lost when trying to encode with 2 separated nested keys container with 2 difference Coding Keys on the same key with JSONEncoder and PListEncoder #51851

Closed
pitiphong-p opened this issue Nov 30, 2018 · 5 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. Codable Area → standard library: `Codable` and co. standard library Area: Standard library umbrella

Comments

@pitiphong-p
Copy link
Contributor

Previous ID SR-9385
Radar rdar://46368053
Original Reporter @pitiphong-p
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment

Xcode 10.1 Swift 4.2 macOS 10.14.1

Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug, Codable
Assignee @itaiferber
Priority Medium

md5: f5f48b9464d016e83e4a5e7f0faf4821

Issue Description:

I think I found an edge case bug of the JSONEncoder and the PListEncoder in Swift Standard Library

I have a model for an API of our service which will return a JSON data in a specific format. I'm trying to design a Swift model to conform to the Swift API Design guideline. In order to achieve that, we want to flatten the properties up to the type. We can use the nestedKey variant for that. However since we have some specific properties based on the type of the data, we decide to have another type for holding those information. The bug is that we need to call nestedKey variant on the same key but for 2 Coding Keys types while encoding the type, the second container will always replace the data encoding by the first container. I believe that it's due to This line of code where the JSONEncoder will create a new container when the nestedKeys method is called.

However the decoding works correctly

{
  "installment_boa": {
    "currencies": [
      "USD"
    ],
    "allowed_installment_terms": [
      3,
      4,
      6,
      9,
      10
    ],
    "amount": {
      "min": 2000,
      "max": 100000000
    }
  }
}

We want to convert the above JSON data to be something like this

public struct Payment: Codable, Equatable {
  public let channel: Channel
  public let limit: Limit?
  public let supportedCurrencies: Set<Currency>
  
  public enum Channel : Equatable {
    case card(Set<CardBrand>)
    case installment(InstallmentBrand, availableNumberOfTerms: IndexSet)
    case mobile(Mobile)
    case alipay
  }
  
  public struct Limit : Codable, Equatable, Hashable {
    public let max: Int64
    public let min: Int64
    
    public var range: ClosedRange<Int64> {
      return min...max
    }
  }
}

I think we should fix the Encoders logic not to replace the value encoded by the prior container, this will match with the behavior of the Decoder

@itaiferber
Copy link
Contributor

Indeed, we should be looking up and reusing any existing containers if possible.

@pitiphong-p
Copy link
Contributor Author

@itaiferber I think I can try to help on fixing this bug if you want me to do

@itaiferber
Copy link
Contributor

@pitiphong-p If you have the time, by all means! That would be much appreciated. The fix should be relatively simple as you note; instead of always writing into self.container, we need to look up whether self.container[key] already exists.

One complication: previously, asking for multiple containers overwrote, and it didn't matter what type they were. Now, if you ask for a keyed container, then an unkeyed for the same key, we can't reuse it. So we need to preconditionFailure descriptively if this happens.

@pitiphong-p
Copy link
Contributor Author

@itaiferber I try to fix this bug and open a #20951. Please have a look and review it

@itaiferber
Copy link
Contributor

Pitiphong did this back in #20951

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. Codable Area → standard library: `Codable` and co. standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

2 participants