Skip to content

Commit

Permalink
Updates to ensure MetaData is encoded correctly in all cases
Browse files Browse the repository at this point in the history
Also ensures we can't bypass the validation by instantiating the enum case directly.
  • Loading branch information
hichamboushaba committed Nov 2, 2024
1 parent 4c429f1 commit 432aa36
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 36 deletions.
4 changes: 4 additions & 0 deletions Networking/Networking.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,7 @@
93D8BBFF226BC1DA00AD2EB3 /* me-settings.json in Resources */ = {isa = PBXBuildFile; fileRef = 93D8BBFE226BC1DA00AD2EB3 /* me-settings.json */; };
93D8BC01226BC20600AD2EB3 /* AccountSettingsRemoteTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 93D8BC00226BC20600AD2EB3 /* AccountSettingsRemoteTests.swift */; };
95122E3E2CB82C0A0079FF0A /* generate-product-success-wrapped.json in Resources */ = {isa = PBXBuildFile; fileRef = 95122E3D2CB82C0A0079FF0A /* generate-product-success-wrapped.json */; };
953477AB2CD678A500038AED /* MetaDataEncoderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 953477AA2CD678A500038AED /* MetaDataEncoderTests.swift */; };
A69FE19D2588D70E0059A96B /* order-with-deleted-refunds.json in Resources */ = {isa = PBXBuildFile; fileRef = A69FE19C2588D70E0059A96B /* order-with-deleted-refunds.json */; };
AE1950F3296DB2C2004D37D2 /* ProductsBulkUpdateMapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = AE1950F2296DB2C2004D37D2 /* ProductsBulkUpdateMapper.swift */; };
AE2D6623272A8F6E004A2C3A /* TelemetryRemoteTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AE2D6622272A8F6E004A2C3A /* TelemetryRemoteTests.swift */; };
Expand Down Expand Up @@ -1788,6 +1789,7 @@
93D8BBFE226BC1DA00AD2EB3 /* me-settings.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "me-settings.json"; sourceTree = "<group>"; };
93D8BC00226BC20600AD2EB3 /* AccountSettingsRemoteTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AccountSettingsRemoteTests.swift; sourceTree = "<group>"; };
95122E3D2CB82C0A0079FF0A /* generate-product-success-wrapped.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "generate-product-success-wrapped.json"; sourceTree = "<group>"; };
953477AA2CD678A500038AED /* MetaDataEncoderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MetaDataEncoderTests.swift; sourceTree = "<group>"; };
9BD9C6C44CAC220B3C3B90B7 /* Pods-Networking.release-alpha.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Networking.release-alpha.xcconfig"; path = "../Pods/Target Support Files/Pods-Networking/Pods-Networking.release-alpha.xcconfig"; sourceTree = "<group>"; };
A69FE19C2588D70E0059A96B /* order-with-deleted-refunds.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "order-with-deleted-refunds.json"; sourceTree = "<group>"; };
AE1950F2296DB2C2004D37D2 /* ProductsBulkUpdateMapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductsBulkUpdateMapper.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3862,6 +3864,7 @@
EE1217DB2AFE04A500E6CAB1 /* ProductVariationEncoderTests.swift */,
028F3F932B0DF9A800F8E227 /* OrderEncoderTests.swift */,
8646A9B62B4522E7001F606C /* BlazeForecastedImpressionsInputEncoderTests.swift */,
953477AA2CD678A500038AED /* MetaDataEncoderTests.swift */,
);
path = Encoder;
sourceTree = "<group>";
Expand Down Expand Up @@ -5381,6 +5384,7 @@
FE28F6EC268436C9004465C7 /* UserRemoteTests.swift in Sources */,
DE74F29E27E0A6800002FE59 /* SiteSettingMapperTests.swift in Sources */,
020D07C023D8587700FD9580 /* MediaRemoteTests.swift in Sources */,
953477AB2CD678A500038AED /* MetaDataEncoderTests.swift in Sources */,
DE6F308727966FEF004E1C9A /* CouponReportListMapperTests.swift in Sources */,
DEA493772B39987B00EED015 /* BlazeTargetOptionMapperTests.swift in Sources */,
4599FC5A24A626B70056157A /* ProductTagListMapperTests.swift in Sources */,
Expand Down
94 changes: 65 additions & 29 deletions Networking/Networking/Model/MetaData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public struct MetaData: Codable, Equatable, Sendable, GeneratedCopiable, Generat
let key = try container.decode(String.self, forKey: .key)
let value = try MetaDataValue(from: container.superDecoder(forKey: CodingKeys.value))

self.init(metadataID: metadataID, key: key, value: value)
self.init(metadataID: metadataID, key: key, value: value)
}
}

Expand All @@ -47,28 +47,18 @@ private extension MetaData {
}
}

public enum MetaDataValue: Codable, Equatable, Sendable, GeneratedCopiable {
case string(_ value: String)
case json(_ json: String)
public struct MetaDataValue: Codable, Equatable, Sendable {
private let valueHolder: ValueHolder

public var stringValue: String {
switch self {
case .string(let value):
return value
case .json(let json):
return json
}
valueHolder.stringValue
}
public var rawValue: String {
switch self {
case .string(let value):
return "\"\(value)\""
case .json(let json):
return json
}
valueHolder.rawValue
}

public var isJson: Bool {
switch self {
switch valueHolder {
case .string:
return false
case .json:
Expand All @@ -77,30 +67,76 @@ public enum MetaDataValue: Codable, Equatable, Sendable, GeneratedCopiable {
}

public init(rawValue: String) {
if let data = rawValue.data(using: .utf8), (try? JSONSerialization.jsonObject(with: data)) != nil {
self = .json(rawValue)
let jsonDecoder = JSONDecoder()
if let data = rawValue.data(using: .utf8),
let jsonObject = try? jsonDecoder.decode(AnyCodable.self, from: data),
// Re-encode the JSON to ensure we use the same formatting when storing it
let encodedJson = try? jsonObject.encodeAsJSON() {
valueHolder = .json(encodedJson)
} else {
self = .string(rawValue.removingPrefix("\"").removingSuffix("\""))
valueHolder = .string(rawValue.removingPrefix("\"").removingSuffix("\""))
}
}

public init(from decoder: Decoder) throws {
let decodable = try decoder.singleValueContainer().decode(AnyDecodable.self)
let codable = try decoder.singleValueContainer().decode(AnyCodable.self)

switch decodable.value {
switch codable.value {
case let value as String:
self = .string(value)
valueHolder = .string(value)
case is Bool:
self = MetaDataValue.string(decodable.description)
valueHolder = .string(codable.description)
case is any Numeric:
self = MetaDataValue.string(decodable.description)
valueHolder = .string(codable.description)
default:
if JSONSerialization.isValidJSONObject(decodable.value) {
let data = (try? JSONSerialization.data(withJSONObject: decodable.value)) ?? Data()
self = MetaDataValue.json(String(data: data, encoding: .utf8) ?? "")
if JSONSerialization.isValidJSONObject(codable.value) {
let encodedJson = try codable.encodeAsJSON()
valueHolder = .json(encodedJson)
} else {
self = MetaDataValue.string("")
valueHolder = .string("")
}
}
}

public func encode(to encoder: Encoder) throws {
switch valueHolder {
case .string(let value):
try value.encode(to: encoder)
case .json(let json):
let encodable = try JSONDecoder().decode(AnyCodable.self, from: Data(json.utf8))
try encodable.encode(to: encoder)
}
}
}

private extension MetaDataValue {
private enum ValueHolder: Codable, Equatable {
case string(_ value: String)
case json(_ json: String)

public var stringValue: String {
switch self {
case .string(let value):
return value
case .json(let json):
return json
}
}

public var rawValue: String {
switch self {
case .string(let value):
return "\"\(value)\""
case .json(let json):
return json
}
}
}
}

private extension Encodable {
func encodeAsJSON() throws -> String {
let data = try JSONEncoder().encode(self)
return String(data: data, encoding: .utf8) ?? ""
}
}
48 changes: 48 additions & 0 deletions Networking/NetworkingTests/Encoder/MetaDataEncoderTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import Foundation
import XCTest
@testable import Networking

/// Check MetaData encoding to Dictionary
///
class MetaDataEncoderTests: XCTestCase {
func test_encode_metadataString_to_dictionary() throws {
// Given
let metadata = MetaData(metadataID: 1, key: "key", value: "value")

// When
let dictionary = try metadata.toDictionary()

// Then
XCTAssertEqual(dictionary["id"] as? Int, 1)
XCTAssertEqual(dictionary["key"] as? String, "key")
XCTAssertEqual(dictionary["value"] as? String, "value")
}

func test_encode_metadataJson_to_dictionary() throws {
// Given
let metadata = MetaData(metadataID: 1, key: "key", value: "{\"key\":\"value\"}")

// When
let dictionary = try metadata.toDictionary()

// Then
let expectedValue = ["key": "value"]
XCTAssertEqual(dictionary["id"] as? Int, 1)
XCTAssertEqual(dictionary["key"] as? String, "key")
XCTAssertEqual(dictionary["value"] as? [String: String], expectedValue)
}

func test_encode_metadataJsonArray_to_dictionary() throws {
// Given
let metadata = MetaData(metadataID: 1, key: "key", value: "[{\"key\":\"value\"}]")

// When
let dictionary = try metadata.toDictionary()

// Then
let expectedValue = [["key": "value"]]
XCTAssertEqual(dictionary["id"] as? Int, 1)
XCTAssertEqual(dictionary["key"] as? String, "key")
XCTAssertEqual(dictionary["value"] as? [[String: String]], expectedValue)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ class CustomFieldViewModelTests: XCTestCase {
}

func test_when_isJson_called_then_return_correct_value() {
let jsonField = CustomFieldViewModel(metadata: MetaData(metadataID: 0, key: "key", value: .json("{\"key\":\"value\"}")))
let jsonField = CustomFieldViewModel(metadata: MetaData(metadataID: 0, key: "key", value: "{\"key\":\"value\"}"))
XCTAssertTrue(jsonField.isJson)

let nonJsonField = CustomFieldViewModel(metadata: MetaData(metadataID: 0, key: "key", value: .string("value")))
let nonJsonField = CustomFieldViewModel(metadata: MetaData(metadataID: 0, key: "key", value: "value"))
XCTAssertFalse(nonJsonField.isJson)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ final class CustomFieldsListViewModelTests: XCTestCase {

func test_given_existingField_when_editFieldCalled_then_displayedItemsAndPendingChangesAreUpdated() {
// Given: A custom field UI to edit an existing field
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1", value: .string("EditedValue1")))
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1",
value: MetaDataValue(rawValue: "EditedValue1")))

// When: Editing the field
viewModel.saveField(key: editedField.key, value: editedField.value, fieldID: editedField.fieldID)
Expand All @@ -76,7 +77,8 @@ final class CustomFieldsListViewModelTests: XCTestCase {

func test_given_editedAndNewFields_when_updatingDisplayedItems_then_changesAreReflected() {
// Given: An edited field and a new field
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1", value: .string("EditedValue1")))
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1",
value: MetaDataValue(rawValue: "EditedValue1")))
let newField = CustomFieldViewModel(key: "NewKey", value: "NewValue")

// When: Editing and adding fields
Expand Down Expand Up @@ -123,7 +125,8 @@ final class CustomFieldsListViewModelTests: XCTestCase {
XCTAssertFalse(viewModel.hasChanges)

// When: Editing a field
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1", value: .string("EditedValue1")))
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1",
value: MetaDataValue(rawValue: "EditedValue1")))
viewModel.saveField(key: editedField.key, value: editedField.value, fieldID: editedField.fieldID)

// Then: hasChanges should be true
Expand Down Expand Up @@ -278,7 +281,8 @@ final class CustomFieldsListViewModelTests: XCTestCase {

func test_given_editedFields_then_disallowedKeysForCreationStillContainsOriginalKeys() {
// Given: Original fields and an edited field
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1", value: .string("EditedValue1")))
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1",
value: MetaDataValue(rawValue: "EditedValue1")))

// When: Editing a field (not yet saved remotely)
viewModel.saveField(key: editedField.key, value: editedField.value, fieldID: editedField.fieldID)
Expand All @@ -304,7 +308,8 @@ final class CustomFieldsListViewModelTests: XCTestCase {

func test_given_multipleChanges_then_disallowedKeysForCreationReflectsOriginalAndAddedKeys() {
// Given: Original fields
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1", value: .string("EditedValue1")))
let editedField = CustomFieldViewModel(metadata: originalMetadata[0].copy(key: "EditedKey1",
value: MetaDataValue(rawValue: "EditedValue1")))
let newField = CustomFieldViewModel(key: "NewKey", value: "NewValue")
let fieldToDelete = originalFields[1]

Expand Down

0 comments on commit 432aa36

Please sign in to comment.