graphqlcrud / spec

Specification for GraphQLCRUD
https://graphqlcrud.org
Apache License 2.0
22 stars 4 forks source link

Identifying objects to delete #62

Open breyed opened 2 years ago

breyed commented 2 years ago

The use of MutateNoteInput is surprising to me as the parameter type for deleteNote. It makes sense for updateNote, but for delete, I would have expected inputs patterned after what the Find and Get operations take, since a delete is essentially a Find/Get, except with the object removed from storage in addition to a copy of it being returned.

wtrocki commented 2 years ago

Very good point. I do not think we explain this properly.

Spec essentially see every operation that makes alteration of the data as "single object" operation. Reasons why this was designed this way was:

GraphQL CRUD is discouraging users to use it's model as only way of interacting with data. It is just mapping common operations and limiting boilerplate. That is why spec considers all multi object operations as batch operation that need to be manually implemented by user (meaning that filter is not exposed/configured on the client). That approach fully matches GraphQL spec ethos (Do not force application developer to 100% rely on Anemic CRUD Model).

wtrocki commented 2 years ago

In short form - Deletions on multiple objects should be designed as separate mutation outside spec without including any user input

Real life production example that follows this pattern

archiveAllMessagesForMarketplacePost(advertId: "176bdb1856d95f3d990c337c80c1c516")

We could imagine if we pull that into spec we will end up with anti-pattern and really bad design where client does business logic:

deleteMessages(filter: {owner: "wojtekteer", marketplaceAdvertId: "176bdb1856d95f3d990c337c80c1c516})
## For each deleted
deleteNotifications(filter: {channelId: "..."})
breyed commented 2 years ago

For any mutation, whether insert, update, or delete, there are simple CRUD operations, and there are custom operations based on business logic. GraphQLCRUD addresses only the simple CRUD operations. Any special cases are out of scope. For some object types, there will be no CRUD delete operations, or even no delete operations at all, in which case the GraphQLCRUD convention for delete won't apply. But for other use cases, it will apply.

It is worth noting that change tracking is independent of GraphQLCRUD. For example, a back end my keep track of the states of the data store both before and after an object was inserted, updated, or deleted. This may be done with custom code, or through a generic algorithm such as SQL Server's temporal tables. The GraphQLCRUD API is the same regardless of whether a history of past states is maintained. Deletes are not a special case relative to inserts and updates, but rather just another kind of mutation.

The GraphQLCRUD API is also independent of the security model. An API that accepts inputs formatted such that any object could be inserted, updated, or deleted can still reject mutations that exceed a caller's permissions. This can be implemented in the GraphQL layer or, as is common for PostGraphile for example, in the database.

wtrocki commented 2 years ago

Any special cases are out of scope. For some object types, there will be no CRUD delete operations, or even no delete operations at all, in which case the GraphQLCRUD convention for delete won't apply. But for other use cases, it will apply.

Exactly. For real life example see: https://graphback.dev/docs/crud/overview#per-type

The GraphQLCRUD API is also independent of the security model. An API that accepts inputs formatted such that any object could be inserted, updated, or deleted can still reject mutations that exceed a caller's permissions. This can be implemented in the GraphQL layer or, as is common for PostGraphile for example

I'm worried that this is quite hard to done right because it would need to evaluate specific user filters build on client or even map database permissions to client user permissions.

If we check graphback docs that provides authorization policies: https://graphback.dev/docs/authentication/keycloak#field-updates-authorization and https://graphback.dev/docs/authentication/keycloak#dynamic-filtering

Those are specifically subject to get out of sync with the schema evolution and are hard to maintain.