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

Rotacloud #394

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion GRDB/Core/DatabaseWriter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public protocol DatabaseWriter : DatabaseReader {
///
/// For example:
///
/// try writer.write { db in
/// try writer.writeWithoutTransaction { db in
/// try db.execute("DELETE FROM player")
/// try writer.readFromCurrentState { db in
/// // Guaranteed to be zero
Expand Down
7 changes: 6 additions & 1 deletion GRDB/Record/FetchableRecord+Decodable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,12 @@ private struct RowKeyedDecodingContainer<Key: CodingKey>: KeyedDecodingContainer
} else if dbValue.isNull {
return nil
} else {
return try T(from: RowDecoder(row: row, codingPath: codingPath + [key]))
if let data = row.dataNoCopy(named: key.stringValue), let dataString = String(data: data, encoding: .utf8), JSONSerialization.isValidJSONObject([dataString]) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On top of my request for further information above, I have several questions here:

  • Why do you read data here, and save a string below?
  • Why isn't this code also present in decode<T>(_: forKey:)? It looks like an oversight. Caring about your use case (decoding an optional Array) is legit, but I'd like that the library remains as consistent and welcoming as it is today. This means thinking about other people as well, including those who, for example, decode a non-optional Array.
  • What about the JSONSerialization.isValidJSONObject performance hit? Is it really useful?
  • I'm not sure JSONSerialization.isValidJSONObject([dataString]) really checks what you expect it to check. To me it looks like it always return true.
  • That if prevents types from providing customized decoding of JSON data. I'd like to read an exploration of the potential harm, if any.

// If data is valid JSON then decode it into model
return try JSONDecoder().decode(type.self, from: data)
} else {
return try T(from: RowDecoder(row: row, codingPath: codingPath + [key]))
}
}
}

Expand Down
12 changes: 12 additions & 0 deletions GRDB/Record/PersistableRecord+Encodable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ private struct PersistableRecordKeyedEncodingContainer<Key: CodingKey> : KeyedEn
mutating func encode(_ value: Float, forKey key: Key) throws { encode(value, key.stringValue) }
mutating func encode(_ value: Double, forKey key: Key) throws { encode(value, key.stringValue) }
mutating func encode(_ value: String, forKey key: Key) throws { encode(value, key.stringValue) }
mutating func encode(_ value: Encodable, forKey key: Key) throws { encode(value.encodeToString(), key.stringValue) }

/// Encodes the given value for the given key.
///
Expand Down Expand Up @@ -194,6 +195,17 @@ private struct PersistableRecordEncoder : Encoder {
}
}

public extension Encodable {
/// Encode model (self) into JSON string
func encodeToString() -> String? {
let json = try! JSONEncoder().encode(self)
if let content = String(data: json, encoding: .utf8) {
return content
}
return nil
}
}

extension MutablePersistableRecord where Self: Encodable {
public func encode(to container: inout PersistenceContainer) {
// The inout container parameter won't enter an escaping closure since
Expand Down
1 change: 1 addition & 0 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- [ ] Avoid code duplication: https://forums.swift.org/t/c-interoperability-combinations-of-library-and-os-versions/14029/4
- [ ] Expose FTS5 in regular GRDB: https://github.com/groue/GRDB.swift/issues/373
- [ ] Allow joining methods on DerivableRequest
- [ ] Make *all* conversion errors provide sql + arguments + column name

Swift 4.2

Expand Down