-
-
Notifications
You must be signed in to change notification settings - Fork 372
refactor: Add attributable protocol for typed attribute values #7077
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
base: main
Are you sure you want to change the base?
Changes from 7 commits
df295cf
7aecbe3
4a4dc20
afd1f46
b57d616
6a1093d
8b7c319
67494c1
ee2b24a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,7 +40,67 @@ public final class SentryAttribute: NSObject { | |
| super.init() | ||
| } | ||
|
|
||
| internal init(value: Any) { | ||
| public init(stringArray values: [String]) { | ||
| self.type = "string[]" | ||
| self.value = values | ||
| super.init() | ||
| } | ||
|
|
||
| public init(booleanArray values: [Bool]) { | ||
| self.type = "boolean[]" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
private enum AttributeType: String {
case stringType = "string",
booleanType = "boolean",
integerType = "integer",
integerArrayType = "integer[]",
doubleType = "double",
doubleArrayType = "double[]"
}SentryAttributeValue could also use it for the types. |
||
| self.value = values | ||
| super.init() | ||
| } | ||
|
|
||
| public init(integerArray values: [Int]) { | ||
| self.type = "integer[]" | ||
| self.value = values | ||
| super.init() | ||
| } | ||
|
|
||
| public init(doubleArray values: [Double]) { | ||
| self.type = "double[]" | ||
| self.value = values | ||
| super.init() | ||
| } | ||
|
|
||
| /// Creates a double attribute from a float value | ||
| public init(floatArray values: [Float]) { | ||
| self.type = "double[]" | ||
| self.value = values.map(Double.init) | ||
| super.init() | ||
| } | ||
|
|
||
| internal init(attributableValue: SentryAttributeValue) { | ||
| switch attributableValue { | ||
| case .boolean(let value): | ||
| self.type = "boolean" | ||
| self.value = value | ||
| case .string(let value): | ||
| self.type = "string" | ||
| self.value = value | ||
| case .integer(let value): | ||
| self.type = "integer" | ||
| self.value = value | ||
| case .double(let value): | ||
| self.type = "double" | ||
| self.value = value | ||
| case .booleanArray(let array): | ||
| self.type = "boolean[]" | ||
| self.value = array | ||
| case .stringArray(let array): | ||
| self.type = "string[]" | ||
| self.value = array | ||
| case .integerArray(let array): | ||
| self.type = "integer[]" | ||
| self.value = array | ||
| case .doubleArray(let array): | ||
| self.type = "double[]" | ||
| self.value = array | ||
| } | ||
| } | ||
|
|
||
| internal init(value: Any) { // swiftlint:disable:this cyclomatic_complexity | ||
| switch value { | ||
| case let stringValue as String: | ||
| self.type = "string" | ||
|
|
@@ -57,6 +117,31 @@ public final class SentryAttribute: NSObject { | |
| case let floatValue as Float: | ||
| self.type = "double" | ||
| self.value = Double(floatValue) | ||
| case let stringValues as [String]: | ||
| self.type = "string[]" | ||
| self.value = stringValues | ||
| case let boolValues as [Bool]: | ||
| self.type = "boolean[]" | ||
| self.value = boolValues | ||
| case let intValues as [Int]: | ||
| self.type = "integer[]" | ||
| self.value = intValues | ||
philprime marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| case let doubleValues as [Double]: | ||
| self.type = "double[]" | ||
| self.value = doubleValues | ||
| case let floatValues as [Float]: | ||
| self.type = "double[]" | ||
| self.value = floatValues.map(Double.init) | ||
| case let attributable as SentryAttributeValuable: | ||
| let value = attributable.asAttributeValue | ||
| self.type = value.type | ||
| self.value = value.anyValue | ||
| case let attribute as SentryAttributeValue: | ||
| self.type = attribute.type | ||
| self.value = attribute.anyValue | ||
| case let attribute as SentryAttribute: | ||
| self.type = attribute.type | ||
| self.value = attribute.value | ||
| default: | ||
| // For any other type, convert to string representation | ||
| self.type = "string" | ||
|
|
@@ -67,40 +152,51 @@ public final class SentryAttribute: NSObject { | |
| } | ||
|
|
||
| // MARK: - Internal Encodable Support | ||
| @_spi(Private) extension SentryAttribute: Encodable { | ||
| private enum CodingKeys: String, CodingKey { | ||
| case value | ||
| case type | ||
| } | ||
|
|
||
| @_spi(Private) extension SentryAttribute: Encodable { | ||
| @_spi(Private) public func encode(to encoder: any Encoder) throws { | ||
| var container = encoder.container(keyedBy: CodingKeys.self) | ||
|
|
||
| try container.encode(type, forKey: .type) | ||
| try self.asAttributeValue.encode(to: encoder) | ||
| } | ||
| } | ||
|
|
||
| switch type { | ||
| @_spi(Private) extension SentryAttribute: SentryAttributeValuable { | ||
| @_spi(Private) public var asAttributeValue: SentryAttributeValue { | ||
| switch self.type { | ||
| case "string": | ||
| guard let stringValue = value as? String else { | ||
| throw EncodingError.invalidValue(value, EncodingError.Context(codingPath: encoder.codingPath, debugDescription: "Expected String but got \(Swift.type(of: value))")) | ||
| if let val = self.value as? String { | ||
| return .string(val) | ||
| } | ||
| try container.encode(stringValue, forKey: .value) | ||
| case "boolean": | ||
| guard let boolValue = value as? Bool else { | ||
| throw EncodingError.invalidValue(value, EncodingError.Context(codingPath: encoder.codingPath, debugDescription: "Expected Bool but got \(Swift.type(of: value))")) | ||
| if let val = self.value as? Bool { | ||
| return .boolean(val) | ||
| } | ||
| try container.encode(boolValue, forKey: .value) | ||
| case "integer": | ||
| guard let intValue = value as? Int else { | ||
| throw EncodingError.invalidValue(value, EncodingError.Context(codingPath: encoder.codingPath, debugDescription: "Expected Int but got \(Swift.type(of: value))")) | ||
| if let val = self.value as? Int { | ||
| return .integer(val) | ||
| } | ||
| try container.encode(intValue, forKey: .value) | ||
| case "double": | ||
| guard let doubleValue = value as? Double else { | ||
| throw EncodingError.invalidValue(value, EncodingError.Context(codingPath: encoder.codingPath, debugDescription: "Expected Double but got \(Swift.type(of: value))")) | ||
| if let val = self.value as? Double { | ||
| return .double(val) | ||
| } | ||
| case "string[]": | ||
| if let val = self.value as? [String] { | ||
| return .stringArray(val) | ||
| } | ||
| case "boolean[]": | ||
| if let val = self.value as? [Bool] { | ||
| return .booleanArray(val) | ||
| } | ||
| case "integer[]": | ||
| if let val = self.value as? [Int] { | ||
| return .integerArray(val) | ||
| } | ||
| case "double[]": | ||
| if let val = self.value as? [Double] { | ||
| return .doubleArray(val) | ||
| } | ||
| try container.encode(doubleValue, forKey: .value) | ||
| default: | ||
| try container.encode(String(describing: value), forKey: .value) | ||
| break | ||
| } | ||
| return .string(String(describing: value)) | ||
philprime marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,145 @@ | ||||||
| public protocol SentryAttributeValuable { | ||||||
| var asAttributeValue: SentryAttributeValue { get } | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| extension String: SentryAttributeValuable { | ||||||
| public var asAttributeValue: SentryAttributeValue { | ||||||
| return .string(self) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| extension Bool: SentryAttributeValuable { | ||||||
| public var asAttributeValue: SentryAttributeValue { | ||||||
| return .boolean(self) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| extension Int: SentryAttributeValuable { | ||||||
| public var asAttributeValue: SentryAttributeValue { | ||||||
| return .integer(self) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| extension Double: SentryAttributeValuable { | ||||||
| public var asAttributeValue: SentryAttributeValue { | ||||||
| return .double(self) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| extension Float: SentryAttributeValuable { | ||||||
| public var asAttributeValue: SentryAttributeValue { | ||||||
| return .double(Double(self)) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| extension Array: SentryAttributeValuable { | ||||||
| public var asAttributeValue: SentryAttributeValue { | ||||||
| // Arrays can be heterogenous, therefore we must validate if all elements are of the same type. | ||||||
| // We must assert the element type too, because due to Objective-C bridging we noticed invalid conversions | ||||||
| // of empty String Arrays to Bool Arrays. | ||||||
| if Element.self == Bool.self, let values = self as? [Bool] { | ||||||
| return .booleanArray(values) | ||||||
| } | ||||||
| if Element.self == Double.self, let values = self as? [Double] { | ||||||
| return .doubleArray(values) | ||||||
| } | ||||||
| if Element.self == Float.self, let values = self as? [Float] { | ||||||
| return .doubleArray(values.map(Double.init)) | ||||||
| } | ||||||
| if Element.self == Int.self, let values = self as? [Int] { | ||||||
| return .integerArray(values) | ||||||
| } | ||||||
| if Element.self == String.self, let values = self as? [String] { | ||||||
| return .stringArray(values) | ||||||
| } | ||||||
| if let values = self as? [SentryAttributeValuable] { | ||||||
| return castArrayToAttributeValue(values: values) | ||||||
| } | ||||||
| return .stringArray(self.map { element in | ||||||
| String(describing: element) | ||||||
| }) | ||||||
| } | ||||||
|
|
||||||
| private func castArrayToAttributeValue(values: [SentryAttributeValuable]) -> SentryAttributeValue { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to do that but it's not possible in Swift to have multiple conformances like this: extension Array: SentryAttributeValuable where Element == Bool {
public var asAttributeValue: SentryAttributeValue {
return .booleanArray(self)
}
}
extension Array: SentryAttributeValuable where Element == Int {
public var asAttributeValue: SentryAttributeValue {
return .integerArray(self)
}
}
extension Array: SentryAttributeValuable where Element == String {
public var asAttributeValue: SentryAttributeValue {
return .stringArray(self)
}
}
extension Array: SentryAttributeValuable where Element == Double {
public var asAttributeValue: SentryAttributeValue {
return .doubleArray(self)
}
}...because Swift does not allow multiple conditional conformances of the same generic type to the same protocol — even if the constraints are mutually exclusive. This is a hard compiler rule, not a limitation of my code. If you, @itaybre or @noahsmartin know how we can make only these types of arrays can actually be casted to
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, maybe we need a completely different approach if it doesn't work with the current one. I can try to come up with something, but that will take some time, and that will have to wait after the holidays. |
||||||
| // Check if the values are homogeneous and can be casted to a specific array type | ||||||
| if let booleanArray = castValuesToBooleanArray(values) { | ||||||
| return booleanArray | ||||||
| } | ||||||
| if let doubleArray = castValuesToDoubleArray(values) { | ||||||
| return doubleArray | ||||||
| } | ||||||
| if let integerArray = castValuesToIntegerArray(values) { | ||||||
| return integerArray | ||||||
| } | ||||||
| if let stringArray = castValuesToStringArray(values) { | ||||||
| return stringArray | ||||||
| } | ||||||
| // If the values are not homogenous we resolve the individual valuables to strings | ||||||
| return .stringArray(values.map { value in | ||||||
| switch value.asAttributeValue { | ||||||
| case .boolean(let value): | ||||||
| return String(describing: value) | ||||||
| case .double(let value): | ||||||
| return String(describing: value) | ||||||
| case .integer(let value): | ||||||
| return String(describing: value) | ||||||
| case .string(let value): | ||||||
| return value | ||||||
| default: | ||||||
| return String(describing: value) | ||||||
| } | ||||||
| }) | ||||||
| } | ||||||
|
|
||||||
| func castValuesToBooleanArray(_ values: [SentryAttributeValuable]) -> SentryAttributeValue? { | ||||||
| let mappedBooleanValues = values.compactMap { element -> Bool? in | ||||||
| guard case .boolean(let value) = element.asAttributeValue else { | ||||||
| return nil | ||||||
| } | ||||||
| return value | ||||||
| } | ||||||
| guard mappedBooleanValues.count == values.count else { | ||||||
| return nil | ||||||
| } | ||||||
| return .booleanArray(mappedBooleanValues) | ||||||
| } | ||||||
|
|
||||||
| func castValuesToDoubleArray(_ values: [SentryAttributeValuable]) -> SentryAttributeValue? { | ||||||
| let mappedDoubleValues = values.compactMap { element -> Double? in | ||||||
| guard case .double(let value) = element.asAttributeValue else { | ||||||
| return nil | ||||||
| } | ||||||
| return value | ||||||
| } | ||||||
| guard mappedDoubleValues.count == values.count else { | ||||||
| return nil | ||||||
| } | ||||||
| return .doubleArray(mappedDoubleValues) | ||||||
| } | ||||||
|
|
||||||
| func castValuesToIntegerArray(_ values: [SentryAttributeValuable]) -> SentryAttributeValue? { | ||||||
| let mappedIntegerValues = values.compactMap { element -> Int? in | ||||||
| guard case .integer(let value) = element.asAttributeValue else { | ||||||
| return nil | ||||||
| } | ||||||
| return value | ||||||
| } | ||||||
| guard mappedIntegerValues.count == values.count else { | ||||||
| return nil | ||||||
| } | ||||||
| return .integerArray(mappedIntegerValues) | ||||||
| } | ||||||
|
|
||||||
| func castValuesToStringArray(_ values: [SentryAttributeValuable]) -> SentryAttributeValue? { | ||||||
| let mappedStringValues = values.compactMap { element -> String? in | ||||||
| guard case .string(let value) = element.asAttributeValue else { | ||||||
| return nil | ||||||
| } | ||||||
| return value | ||||||
| } | ||||||
| guard mappedStringValues.count == values.count else { | ||||||
| return nil | ||||||
| } | ||||||
| return .stringArray(mappedStringValues) | ||||||
| } | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
h: I would highly appreciate proper code docs for all these new init methods, as these are public API.