separate read and write paths

[nelztruss]

Here is the PR that I "threatened", proposing an approach that approximates Command-Query Responsibility Separation... E.g. making sure the read paths and write paths are separate and distinct from each other.

This pattern also executes a little bit of the Don't Repeat Yourself (DRY) principle as well, if you squint. Pretend that the read/fetch operation somehow gets more complicated, and you update the FetchDog appropriately, but you forget to update how the CreateDog or UpdateDog operations build their response, and then the entity that you get back from FetchDog no longer matches what comes back from CreateDog/UpdateDog... That could lead to some subtle bugs in the code. By forcing everything through the FetchDog path, you've effectively DRY'd your code.

Another way that this helps is that it is ready to adjust to high read volume and scale later on down the road. Wanna put REDIS or Memcached caching on the read path? Cool, it's already separated out.

(I generally have felt the same thing should apply at the API layer, i.e. POSTs/PUTs should not return the body of the entity itself, but at most a 301 to the canonical entity location... However Kara convinced me that for purposes of reducing client round-trips to the server, returning those entities might be useful depending on the use-case. I am definitely willing to leave that in the "it depends" bucket.)

I definitely think read/write separation at the data layer is a MUST. I also built in the separation at the service layer in the PR just to show how it might affect higher layers.

[ahobson]

One of the things I like about GraphQL is it explicitly separates input data from data that is returned. In this instance, it's not clear to the consumer that the ID will be overwritten. Would it be worth it to create a models.DogCreateInput or some such to make that more explicit?

Similarly, for update, it's not explicit how the dog is found for update. Sure, it's probably the ID, but again, I wonder about making it clear about the key(s) used for finding the record and the data that is allowed to update.

This pattern is particularly useful when in different situations different fields are allowed to be updated.

Thoughts?

[nelztruss]

(Sorry it's taken me a while to get back to this!)

Yes, I believe I see where you are going with the models.DogCreateInput... (I believe we're ending up in similar challenges on EASi.)
The thing is that these types are grouping several different kinds of fields: some are directly editable by the API consumer; several are really "metadata" that managed by the business logic and aren't directly mutable by the API consumer; but in the end they all do fit together in one single representations as an artifact of a READ operation.
If we went down the path of having first a models.Dog object, and then a models.DogCreateInput object, how long until we have a models.DogUpdateName object? As an example, if we were going down this path on the EASi project, I can see that we'd be tempted to create an models.XXXUpdateYYY object that corresponded to each UI page of a multi-page form... And, to me, this feels like "over-fitting" the API to the UI.
I do think that the pattern of having more model types is probably a wise choice on larger projects, especially if you can delineate a read-side model from a creation/mutation model (probably using lots more pointers to communicate "this field has not been changed" vs "this field has been changed to the zero value"). (NB: I am now scheming on how I might be able to interject this into EASi... 🤔 )

Your point about the ID generation strikes another chord with me too. On EASi, we've just ended up with a business need to use something akin to the CreateXXX function, but we need the ID to NOT be overridden. (Use case: backfill of historical records.) I believe ID generation is another responsibility that could be decomposed out and called for explicitly, running the risk of this example code being less transferrable to projects where it is more appropriate for the DB to do ID generation.

Altogether, I love these discussions! I also believe we could talk ourselves blue in the face over many a beverage trying to find the most bestest design evar! However, when I was proposing this set of changes, I was striving for small iterative steps from what is already laid down, and that's why I didn't go too wild... I was mostly trying to DRY the read-path, and to remove the hidey-hole for the subtle bugs that could happen when you don't round-trip from the DB on the mutation-side functions.