Is 'request(for:...)' only available on types that adopt MutablePersistableRecord?

[james-rant]

What did you do?

I'm trying to design my model structs with associations using hasMany() and belongsTo() etc. The documentation has plenty of examples how to do this, but they make use of request(for: MyType.myAssociation), which appears to be only defined for types that adopt MutablePersistableRecord. My types are not mutable, so I am only adopting TableRecord and FetchableRecord. Am I not able to use associations if my types are immutable?

What did you expect to happen?

Expect to be able to use associations with read-only types.

What happened instead?

The request(for: association) function is unavailable for types that do not adopt MutablePersistableRecord.

Environment

GRDB flavor(s): GRDBCipher GRDB version: v3.3.0 Installation method: Carthage (I understand it's not supported, but it works for me at the moment) Xcode version: 10.0 (10A255) Swift version: 4.2 Platform(s) running GRDB: iOS macOS version running Xcode: 10.14

Example:

struct MyType: Decodable, TableRecord, FetchableRecord {
    let id: Int
    let name: String

    static let otherTypes = hasMany(MyOtherType.self)
    var otherTypes: QueryInterfaceRequest<MyOtherType> {
        return request(for: MyType.otherTypes) // <-- Use of unresolved identifier 'request'; did you mean 'Request'?
    }
}

struct MyOtherType: Decodable, TableRecord, FetchableRecord {
    let id: Int
    let name: String

    static let myType = belongsTo(MyType.self)
    var myTypes: QueryInterfaceRequest<MyType> {
        return request(for: MyOtherType.myType) // <-- Use of unresolved identifier 'request'; did you mean 'Request'?
    }
}

[groue]

Hello @james-rantmedia,

(Related GRDB documentation)

I'm glad you ask this question, because I understand that you are puzzled. Like you, I prefer to avoid granting write access to parts of the program that are not supposed to write.

Why does fetching associated records require a protocol which grants write access???

Let's look at this sample code:

let author = Author(id: 42, ...)
let books = try author.books.fetchAll(db)

It needs to execute the following SQL query:

SELECT * FROM books WHERE authorId = 42

Where does this 42, in the SQL query, come from? It comes from author.encode(to:), where encode(to:) is a PersistableRecord protocol method.

And that's why you currently have to give write access to the Author type, when you just want to read their books. A feature that was initially designed for writing in the database has eventually left its initial purpose: it now also fuels read-only features.

I totally agree that there is room for improvement here.

All suggestions are welcome!

[james-rant]

Hey @groue, thanks for the quick response. What you say makes sense now that I understand where the functionality comes from.

Would adopting the Codable (or Encodable) protocol provide the encode(to:) functionality without having to bring in the whole PersistableRecord protocol? I would think that sometimes it makes sense to need to encode a type without having to write it to the database. A contrived example might be an app that uses a read-only database to pre-fill requests to an external API?

[groue]

Would adopting the Codable (or Encodable) protocol provide the encode(to:) functionality without having to bring in the whole PersistableRecord protocol? I would think that sometimes it makes sense to need to encode a type without having to write it to the database. A contrived example might be an app that uses a read-only database to pre-fill requests to an external API?

Yes, this is a good idea. We don't even have to look for a contrived example, since we have a real one right is this issue.

Now, the standard Encodable protocol a convenience protocol for GRDB. The "real" job currently happens as below:

       +-------------+
       | TableRecord |       SQL generation
       +-------------+
              ^
              |
+-------------+------------+
| MutablePersistableRecord | Mutating Persistence
+--------------------------+
              ^
              |
    +---------+---------+
    | PersistableRecord |    Non-mutating Persistence
    +-------------------+

/// SQL generation
/// Comes with methods that build requests
protocol TableRecord {
    static var databaseTableName: String { get }
}

/// Mutating Persistence (for structs with auto-incremented ids)
/// Comes with:
/// - mutating methods that write in the database
/// - record comparison methods
/// - ability to feed some association requests
protocol MutablePersistableRecord: TableRecord {
    func encode(to container: PersistenceContainer)
    mutating func didInsert(...)
}

/// Non-mutating Persistence (for class types, and structs
/// without auto-incremented ids)
/// Comes with non-mutating methods that write in the database
protocol PersistableRecord: MutablePersistableRecord {
    func didInsert(...)
}

We could indeed split MutablePersistableRecord into two distinct protocols:

+-------------+   +-----------------+
| TableRecord |   | EncodableRecord |
+-------------+   +-----------------+
         ^             ^
         |             |
   +-----+-------------+------+
   | MutablePersistableRecord |
   +--------------------------+
               ^
               |
     +---------+---------+
     | PersistableRecord |
     +-------------------+

/// SQL generation
/// Comes with methods that build requests
protocol TableRecord {
    static var databaseTableName: String { get }
}

/// NEW: Database Representation
/// Comes with:
/// - record comparison methods
/// - ability to feed some association requests
protocol EncodableRecord {
    func encode(to container: PersistenceContainer)
}

/// Mutating Persistence
/// Comes with mutating methods that write in the database
protocol MutablePersistableRecord: TableRecord, EncodableRecord {
    mutating func didInsert(...)
}

/// Non-mutating Persistence
/// Comes with non-mutating methods that write in the database
protocol PersistableRecord: MutablePersistableRecord {
    func didInsert(...)
}

That's a quick draft. I guess we need more exploration until we come up with a conclusion.

[james-rant]

I think this is a good start, separating the record comparison and association stuff from the database writing stuff 👍

[groue]

Yes, I think it could work. That would complexity the API, though. I'm already dissatisfied by the MutablePersistableRecord/PersistableRecord couple...

Well, so be it.

The challenge is not really in implementation: we just have to replace MutablePersistableRecord with EncodableRecord where possible. We don't have to update any test, because the change is not API-breaking. A few new tests, for the core EncodableRecord APIs, should be enough.

The real challenge is the necessary documentation updates. The record protocol hierarchy is already complex. It is complex because it has many purposes. Records have many facets. We can't expect the reader to know them all, to know them well, and to gently swallow everything we tell him. We have to be as progressive as possible in the documentation.

We only have one use case, so far, where the library user wants EncodableRecord but not PersistableRecord (this issue). And it is a pretty advanced use case on top of that (associations are not trivial).

That's my current analysis of the feature request.

Now, what do you think, James? Would you be interested in submitting a pull request with those changes (feature, tests, documentation)? I'd support you along the way, of course, if you have any question. Meanwhile, I'll be able to keep on adding a few missing association features (#423, and other items in the "Known Issues" and "Future Directions" chapters of the Associations Guide). Tell me if you're interested. If you're not, that's not a problem: I'll add this issue to the list of Suggested Contributions, so that it is not forgotten.

[james-rant]

Unfortunately I won't have any time to be able to work on a PR for this 🙁. I think it's a great candidate for the suggested contributions list as you say. I don't need this feature to be added immediately, there are ways to work around what I wanted to do.

[groue]

OK James, that's fine. CONTRIBUTING.md has been updated.

I'm closing this issue, since nobody is currently working on it. Happy GRDB!

[james-rant]

Thanks for the help @groue 👍

[groue]

This issue will be fixed in GRDB 4. Relevant PR: #499.