[Model] Automatic CreatedAt and UpdatedAt timestamps

craicoverflow commented 4 years ago

Graphback is missing the ability to automatically create and/or handle timestamps for CreatedAt and UpdatedAt timestamps.

This could be as simple as adding a custom @CreatedAt @UpdatedAt directive to the model which will set these values in the CRUD runtime.

"""
@model
"""
type Comment {
  id: ID!
  text: String
  createdAt: String @CreatedAt
  updatedAt: String @UpdatedAt
}

ssd71 commented 4 years ago

I think for mongodb, the best place to put this stuff would be in the provider itself, I have mostly got it to work using a few monkey patches here and there, I cannot see any nuances with this approach, please point out if you do, or suggest if I should open a PR

wtrocki commented 4 years ago

This can be added by adding wrapper over the data or directly in offix provider. No difference at this point

ssd71 commented 4 years ago

Going by some investigation in #1301 , For MongoDB we could modify the offix provider to setup this by checking fields for directives in the constructor, as well as using the update and create functions to set the fields that have these directives. There are some gotchas here:

MongoDB stores the creation timestamp inside the ObjectID of a document[1]. We could use that by mapping a field in the output document that will store the timestamp from the ObjectID. With this the actual document in the database will only contain an updatedAt field but the provider will still return objects with both fields(createdAt and updatedAt). This approach may significantly increase complexity of the code that has to be written, including code for filtering[2].
The standard approach of updating those fields in provider.create and provider.update sounds good enough. If we go with this, it allows for more flexibility. Somebody proficient in mongo can probably write code to utilise the timestamp in the ObjectID (perhaps in client side code) as it is simple enough to do so[3]. So they won't have to use @createdAt for that. This approach is also what one would normally expect. For example, if I have a created field in schema, I would expect that field in the database as well.

I am siding with the second approach as it feels more apt for this situation, but I would love for someone to argue against it :smile: so some help needed!

Also would love some pointers on how to do with Postgres, right now, I am thinking the same approach, using different fields for both and updating in providers, but haven't fully explored yet!

I haven't thought about how to handle timestamps passed in queries though, especially for postgres, can we have a Date Scalar of some sort, I think that might be more helpful with this :-/

wtrocki commented 4 years ago

We have couple different topis here.

Explicit vs implicit fields

People can add metadata to their existing fields to mark them as updated at or they can annotate model and generate this out of the box in their schema (models will be missing those fields

Multi database support

If we are adding this on schema level we need to support both Mongo and Postgress or have a plan how both can be supported. This brings us to adapt more lenient approach to generating values (new Date() instead of db specific stuff like MongoDB

Flexibility

The standard approach of updating those fields in provider.create and provider.update sounds good enough. If we go with this, it allows for more flexibility.

Flexibility is ok it it is not bringing not needed complexity in code.

Difference between DataSync and generic

Generally I thought about having """ @versioned """ marker for offix that will add those fields automatically. Ideally for offix we would simply need updatedAt field or something. Bringing this to generic use cases brings complexity and still not implementing the offix case (where those fields actually matter a lot)

Since you have done generic implementation ready it is ok to push that forward but I would implement this offix first.

checking fields for directives in the constructor

You are probably thinking about metadata markers?

craicoverflow commented 4 years ago

The standard approach of updating those fields in provider.create and provider.update sounds good enough. If we go with this, it allows for more flexibility.

This is what came to mind as an implementation approach when I created this issue. The challenge would be managing user expectations. If they add a custom mutation resolver should they also expected these timestamps to be managed, even partially with a helper function to apply the timestamps.

Bringing this to generic use cases brings complexity and still not implementing the offix case (where those fields actually matter a lot)

Would it not make more sense to implement the generic UpdateAt/CreatedAt timestamps first, and allow Offix to simply use it.

If Graphback runtime manages the application of the timestamp values in the data providers the Offix plugin would be simplified as it would just add a createdAt field to the model.

Model:

"""
@model
@versioned
"""
type User {
  id: ID!
}

Offix affected schema output:

"""
@model
@versioned
"""
type User {
  id: ID!
  """
  @createdAt
  """
  updatedAt: DateTime
}

You are probably thinking about metadata markers?

This issue is about directives - at the time we were thinking directives for runtime and annotations for build/bootstrapping the server. We should make a call on what we are doing. I am leaning towards full on annotations for simplicity.

I tried searching and there is no easy way to add custom directives logic to GraphQL-JS. Apollo on the other hand has a way to wrapping directive resolver to apply field logic at schema level, but we can't use this. See https://www.apollographql.com/docs/apollo-server/schema/directives/

wtrocki commented 4 years ago

Would it not make more sense to implement the generic UpdateAt/CreatedAt timestamps first, and allow Offix to simply use it.

100% agree

craicoverflow commented 4 years ago

I believe this is complete for MongoDb, but not Postgres. We can keep this open until Postgres is complete.

@ssd71 will you be doing it for Postgres? If not just unassign yourself

craicoverflow commented 4 years ago

Closing for https://github.com/aerogear/graphback/issues/1355

aerogear / graphback