fluent-kit icon indicating copy to clipboard operation
fluent-kit copied to clipboard

Improve @OptionalParent decoding

Open tuchangwei opened this issue 5 years ago • 2 comments

Steps to reproduce

I have two database models named Article and Category. Category is Article's Parent, so in the Article class, I define:

    @OptionalParent(key: categoryIDKey)
    var category: ArticleCategory?

I allow the category property nil, but when I send the json, it returns an error. The json I send:

{
  "body": "Hello, my first blog.",
  "name": "my first blog"
}

The error:

{
    "error": true,
    "reason": "Value required for key 'category'."
}

After debugging, I find I should send the json like:

{
  "body": "Hello, my first blog.",
  "category": {},
  "name": "my first blog"
}

On the front-end developer view, it is weird, they don't know why they need to pass the category like that.

Expected behavior

The four formats should be allowed:

  1. no category key
  2. category: null
  3. category: {}
  4. category: { id: null }

Actual behavior

Environment

  • Vapor Framework version: 4.0.0-rc.3.12
  • Vapor Toolbox version: vapor-beta
  • OS version: Macos 10.5.4

tuchangwei avatar Apr 01 '20 02:04 tuchangwei

(The quick way around this is to create a CreateArticleData type that contains the bits that makes it logical to send and then construct a real Article in your code. This helps if the parent field is non-optional and you have to pass "category": { "id": 1} etc. Here's an example)

0xTim avatar Apr 01 '20 10:04 0xTim

TL;DR. Getting this to work well is pretty complicated. We should probably just stick to DTOs as Tim mentioned since those are explicit and predictable.


I looked into supporting this on the gm branch and it's harder than I thought. I ran into a couple of problems:

The four formats should be allowed:

  1. no category key

This is not possible given the current design of Fluent's property wrappers and their Codable support. Each property wrapper gets handed a container for its specific key, so the property is not able to control whether or not that key is decoded.

Using the example from the issue, the model will attempt to decode a sub-container for "category" to pass to @OptionalParent when the model is decoding. This will trigger an error if there's no "category" key before @OptionalParent is ever invoked.

This could be possible if we reworked how Codable + property wrappers work together but that requires more thought.

  1. category: null

Codable makes supporting different container types different. The code for this looks something like:

do {
    let single = try decoder.singleValueContainer()
    if single.decodeNil() { ... }
} catch {
    // what happens to error?
    let keyed = try decoder.keyedContainer(...)
}

There's no way to check if a decoder can yield a single or keyed container before you actually attempt it and have an error thrown. It's not clear what to do with this error once you get it since there are no promises it is not an error for something completely unrelated.

tanner0101 avatar May 08 '20 17:05 tanner0101