loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.93k stars 1.06k forks source link

Support auto-updated model properties "createdAt" and "updateAt" #1857

Open vvdwivedi opened 5 years ago

vvdwivedi commented 5 years ago

Feature proposal

While creating a model/Entity from cli, I have an option to create a date field. A common example would be a user model with created_at and updated_at fields (or any variants of createdAt and updatedAt). Typically the behaviour is that on new resource creation, I would like to set the created_at and updated_at as the current time. on subsequent updates, I would like to update the updated_at timestamp with the time of operation.

Given that created_at needs to be populated at creation and updated_at needs to be updated with every modification, it can be a nice inbuilt feature to have with models where I can say that a particular field acts as created_at and another one as updated_at and loopback can take care of updating these fields, saving me some lines of code.

marioestradarosa commented 5 years ago
@property({
    type: 'date',
    default: () => new Date(),
  }) 

doesn't work ? , also by just placing Date() should work, however, you can use () => and format the date however you want, depending on how the db server would expect it.

dhmlau commented 5 years ago

The above snippet seems to be working for me. Extending the Todo application , I added the following in the model.ts.

  @property({
    type: 'date',
    default: () => new Date(),
  })
  createdAt?: Date;

The output is:

{
  "createdAt": "2018-10-16T02:41:31.198Z"
// with other attributes
}
marioestradarosa commented 5 years ago

@vvdwivedi did this answer your question?

vvdwivedi commented 5 years ago

So I was using new Date() and that works fine (as mentioned above). I was just wondering if there is a built in way to handle time stamps. Something similar to Eloquent (https://laravel.com/docs/4.2/eloquent#timestamps). If not, we can close this ticket. I got the answer for the date default.

dhmlau commented 5 years ago

@vvdwivedi , IIUC, there's no built-in timestamp. @raymondfeng can confirm.

marioestradarosa commented 5 years ago

I would like to set the created_at and updated_at as the current time. on subsequent updates, I would like to update the updated_at timestamp with the time of operation

Two options here: (for the updated_at_timestamp)

  1. Insert your code (hooks) in the controller to place a timestamp for CRUD operations
  2. Let the database handle it, for instance mysql can handle that, and I assume most of DB servers can handle this scenario too, which is the best scenario since no matter the client operation the DB remains consistent

it can be a nice inbuilt feature to have with models where I can say that a particular field acts as created_at and another one as updated_at

Yes that would be a nice feature, probably a feature that extends the property decorator like so:

@property({
    type: 'timestamp',
    onUpdate: true,    
  })
David-Mulder commented 5 years ago

Anyway, shouldn't there be on the model level the possibility to run a piece of code whenever the model is altered? I would expect something like

export class SomeEntity extends Entity implements OnUpdate {
    lb4OnUpdate() {
        this.last_updated = new Date();
    }
}

for example (styled per Angular style, not necessarily the best option).

marioestradarosa commented 5 years ago

@David-Mulder these are my personal suggestions only, currently I haven't tested it.

At the repository level you can (since the models won't have any business logic) :

David-Mulder commented 5 years ago

@marioestradarosa And that's the moment I officially realize I sincerely regret picking loopback for this project 😵😟 . Hacking such a thing together on the repository layer is perfectly fine when you are doing a small one-man-team project, but it's completely unrealistic if you have other developers who might use the model... 😕 . The lack of basic validation in GA was scaring the crap out of me, but I figured it was just a weird "lets rush GA release even if it's unfinished and bad"... but the list of "hack your own solution together"-problems is getting uncomfortably long 😕 .

marioestradarosa commented 5 years ago

@David-Mulder, I agree that we should be able to specify hooks easily. keep in mind that I am from the community just like you, and I also decided to choose LB4 for my own project too, which by the way is not small and here are my personal notes:

Releasing soon our first LB4 application :

Problems?. Yes, we found problems, and we had to create most of the artifacts by hand. But we decided to use a typescript based nodeJS framework and found LB4 and liked its architecture and for the records, I am new in the Type script world, but the framework architecture looked familiar to me, since I am coming from Java EE.

Now my Team is focused on Node JS and TypeScript for both UI/Backend, before TS, I'd never have started a javascript project on the backend. We are happy with LB4, it is growing and I am not expecting the framework to do all the job at this moment, however I am expecting to the framework to evolve so next projects will be more easy to develop.

Migrating from LB3 ?. It really depends on your project and your will to rollup your sleeves and do some coding in some cases. However, in my personal opinion for what I have seen so far, LB3 is stable and LB4 hasn't reached to a point of migrating automatically from LB3 yet. LB4 also will need to able to work in a declarative way and generate the code automatically.

And finally, the current issue exposed by @vvdwivedi is a Feature Proposal , so we have four options. (1) Use another framework, (2) Wait until others implement it, (3) Use a workaround or (4) roll up our sleeves and try to implement it and make this framework grow.

Disclaimer: This is my personal comments as a community member 😄. I agree with you as a user and believe me, I passed already thru frustrations and regrets also, especially if I am new to Node JS development. I more or less survived so far.

bajtos commented 5 years ago

Anyway, shouldn't there be on the model level the possibility to run a piece of code whenever the model is altered?

In LoopBack 3.x, we have a concept of Operation Hooks allowing models to register handlers for common events: access, before/after save, before/after delete, load/persist.

I think it makes sense to implement similar hooks in LoopBack 4, I opened a new issue to keep track of this feature - see https://github.com/strongloop/loopback-next/issues/1919

Hacking such a thing together on the repository layer is perfectly fine when you are doing a small one-man-team project, but it's completely unrealistic if you have other developers who might use the model...

I agree we need a solution that's easy to share together with the model. IMO, if a shared component is providing a model class then it should provide also the accompanying repository implementation.

@David-Mulder Have you considered that approach? Does it address your concerns?

The lack of basic validation in GA was scaring the crap out of me, but I figured it was just a weird "lets rush GA release even if it's unfinished and bad"... but the list of "hack your own solution together"-problems is getting uncomfortably long

I would like to change the way how we look at the current situation. Yes, the feature set of LoopBack 4 is much smaller than LoopBack 3 right now and we fully acknowledge that. However, we see this limitation as a great opportunity for the community to experiment with different approaches on how to implement missing features and get more involved with LoopBack project. It is one of our primary design goals to keep LoopBack 4 extensible to allow this kind of experimentation in the community.

@David-Mulder If you are looking for a mature framework providing everything "with batteries included" then I am afraid LoopBack 4 is not a good choice for you at this time. You may get more luck with LoopBack version 3 as it offer much more features out of the box, plus there are more 3rd party extensions available for v3.

bajtos commented 5 years ago

Thank you @marioestradarosa for sharing the details of your motivations for using LB4 👍

bajtos commented 5 years ago

Oh, I forgot to share few more thoughts on possible solutions (workarounds?).

First of all, while LoopBack 4 comes with a loopback-datasource-juggler as the recommended ORM framework to use, it should be very easy to replace it with any other ORM you like more.

For example, if you have a solution for modified_at property that works in LoopBack 3, then you can keep using LoopBack 3 models to access the database, just update your REST API Controllers to call those models instead of LB4 repositories.

Maybe you don't like juggler? You can use Objection.js, typeorm, Bookshelf or any other ORM out there.

We did a quick spike to see how to integrate with TypeORM from LoopBack4, see https://github.com/strongloop-archive/loopback4-extension-typeorm (just note the code is quite outdated by now).

bajtos commented 5 years ago

Back to the original topic of implementing createdAt and updatedAt properties.

IMO, a reliable implementation of such timestamp properties is tricky and requires support from database connectors. Let me explain why.

The problem boils down to a simple question: for a given CRUD operation, we need to know whether the operation is going to create a new record or update an existing one. Unfortunately, there are CRUD operations where it's not possible to answer that question in advance: patchOrCreate, replaceOrCreate and upsertWithWhere. The answer is inside the database. Because the entire "change-or-create" operation must be atomic to prevent race conditions and data corruption, the generic ORM layer in juggler cannot query the database to find out whether we are going to create a new record or update an existing one.

(As a result, a naive LB 3.x solution leveraging before save hook and itsctx.isNew flag is not robust and cannot be relied on, unless the models have disabled all three "change-or-create" APIs.)

My conclusion is that createdAt and modifiedAt properties are a feature that must be provided by the ORM framework itself, with the cooperation of connectors that must craft queries in such way that createdAt and updatedAt are correctly updated for "change-or-create" operations.

For example, the connector can pass a different set of properties to set for "create" and for "update" branch. Another alternative is to start a micro-transaction and run two queries - the first one to decide on "update" vs "create" and the second one to persist the data. Ideally, the constraint should be enforced by the database itself (as suggested by @marioestradarosa) and the connector should include commands to set up such constraint as part of database migration (see #1547 and #487).

marioestradarosa commented 5 years ago

for a given CRUD operation, we need to know whether the operation is going to create a new record or update an existing one. Unfortunately, there are CRUD operations where it's not possible to answer that question in advance: patchOrCreate, replaceOrCreate and upsertWithWhere

Exactly!. But that's because somehow these operations were introduced. Back in time we only had C,R,U or D . and thus the hooks could potentially monitor pretty well.

Also, on another note, we tend to forget that the database is a full fledge software system. Take mysql for instance, you can easily create the schema as follows and voilá, problem solved.

created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP 
                    ON UPDATE CURRENT_TIMESTAMP

I believe all database system will have an answer for this issue also. LB is not a framework that replaces your DB admin tools (table alters, backups, referential integrity etc.) , nor the juggler ORM or any other ORM will replace the full potential of the DB server software, these serve most as bridges.

vvdwivedi commented 5 years ago

@marioestradarosa fair point that DB can handle this, but generally I prefer my schema to be a documentation for someone picking up the code. Given that there can be multiple datasources, and we can't have generic solution for all, a hook on the behaviour would be a good thing to have. I can decide what to do on update or on create or ignore it all together. You can say this is not exactly a responsibility of LB, but mostly good to have kind of thing. I am a beginner in terms of open source contribution, but if you can provide some guidance, I will be happy to take up the implementation as decided.

bajtos commented 5 years ago

Given that there can be multiple datasources, and we can't have generic solution for all, a hook on the behaviour would be a good thing to have. I can decide what to do on update or on create or ignore it all together.

Until we have support for generic hooks, I believe it's possible to implement support for createdAt/updatedAt properties by writing a custom repository class.

Something along the following lines:

export class TimestampingRepository<T extends Entity, ID>
  extends DefaultCrudRepository <T, ID> {

  constructor(
    public entityClass: typeof Entity & {prototype: T},
    public dataSource: juggler.DataSource,
  ) {
    super(entityClass, dataSource);
    // TODO: check whether the model has updatedAt/createdAt properties?
  }

  async create(entity: DataObject<T>, options?: Options): Promise<T> {
    entity.createdAt = new Date();
    entity.updatedAt = new Date();
    return super.create(entity, options);
  }

  async updateAll(
    data: DataObject<T>,
    where?: Where<T>,
    options?: Options,
  ): Promise<Count> {
    data.updatedAt = new Date();
    return super.updateAll(data, where, options);
  }

  async replaceById(
    id: ID,
    data: DataObject<T>,
    options?: Options,
  ): Promise<void> {
    data.updatedAt = new Date();
    return super.replaceById(id, data, options);
  }
}

To make allow this functionality to compose with other possible extensions, it may be better to extract it into a mixin.

// TODO: add type information for the "repository" argument
export function TimestampMixin(repository) {
  return class extends repository {
    async create(entity: DataObject<T>, options?: Options): Promise<T> {
      entity.createdAt = new Date();
      entity.updatedAt = new Date();
      return super.create(entity, options);
    }

    // ...
  };
}

// usage
export class TodoRepository extends TimestampMixin(
  DefaultCrudRepository<Todo, typeof Todo.prototype.id>
) {
  // ...
}
ludohenin commented 5 years ago

~~@bajtos looks great, thanks. What would be the way to add createdBy though? I'm strungling with getting access to the CURRENT_USER, request context being not available in the repository class.~~

Edit: Found my way

ghtech commented 5 years ago

Given that there can be multiple datasources, and we can't have generic solution for all, a hook on the behaviour would be a good thing to have. I can decide what to do on update or on create or ignore it all together.

Until we have support for generic hooks, I believe it's possible to implement support for createdAt/updatedAt properties by writing a custom repository class.

Something along the following lines:

export class TimestampingRepository<T extends Entity, ID>
  extends DefaultCrudRepository <T, ID> {

  constructor(
    public entityClass: typeof Entity & {prototype: T},
    public dataSource: juggler.DataSource,
  ) {
    super(entityClass, dataSource);
    // TODO: check whether the model has updatedAt/createdAt properties?
  }

  async create(entity: DataObject<T>, options?: Options): Promise<T> {
    entity.createdAt = new Date();
    entity.updatedAt = new Date();
    return super.create(entity, options);
  }

  async updateAll(
    data: DataObject<T>,
    where?: Where<T>,
    options?: Options,
  ): Promise<Count> {
    data.updatedAt = new Date();
    return super.updateAll(data, where, options);
  }

  async replaceById(
    id: ID,
    data: DataObject<T>,
    options?: Options,
  ): Promise<void> {
    data.updatedAt = new Date();
    return super.replaceById(id, data, options);
  }
}

To make allow this functionality to compose with other possible extensions, it may be better to extract it into a mixin.

// TODO: add type information for the "repository" argument
export function TimestampMixin(repository) {
  return class extends repository {
    async create(entity: DataObject<T>, options?: Options): Promise<T> {
      entity.createdAt = new Date();
      entity.updatedAt = new Date();
      return super.create(entity, options);
    }

    // ...
  };
}

// usage
export class TodoRepository extends TimestampMixin(
  DefaultCrudRepository<Todo, typeof Todo.prototype.id>
) {
  // ...
}

Hi I am implementing proposed solution but getting error Property 'createdAt' does not exist on type 'DataObject'. how to define createdAt and updatedAt properties in TimestampingRepository thanks

bajtos commented 5 years ago

@ghtech I think you need to tell TypeScript that the target entity (T) must have createdAt and updatedAt properties. If you share the code you have written so far then I may be able to give you a more specific advice, perhaps including code snippets too.

rzubov-devpro commented 5 years ago

for a given CRUD operation, we need to know whether the operation is going to create a new record or update an existing one. Unfortunately, there are CRUD operations where it's not possible to answer that question in advance: patchOrCreate, replaceOrCreate and upsertWithWhere

Exactly!. But that's because somehow these operations were introduced. Back in time we only had C,R,U or D . and thus the hooks could potentially monitor pretty well.

Also, on another note, we tend to forget that the database is a full fledge software system. Take mysql for instance, you can easily create the schema as follows and voilá, problem solved.

created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP 
                    ON UPDATE CURRENT_TIMESTAMP

I believe all database system will have an answer for this issue also. LB is not a framework that replaces your DB admin tools (table alters, backups, referential integrity etc.) , nor the juggler ORM or any other ORM will replace the full potential of the DB server software, these serve most as bridges.

How can we use it at the same time as migrations?

bajtos commented 5 years ago

LB is not a framework that replaces your DB admin tools (table alters, backups, referential integrity etc.) , nor the juggler ORM or any other ORM will replace the full potential of the DB server software, these serve most as bridges.

Agreed 👍 IMO, we need to find a good balance between easy-to-use high-level abstractions that are exposed via Juggler/Repository APIs and work for every supported database, and extension points allowing advanced users to tap into specific features their database provides.

In the case of "createdAt" and "updatedAt" properties, I think it would be great to find a solution allowing the developer to tell LoopBack that these two properties are handled by the database, they are not allowed in incoming requests but present in outgoing responses. Pretty much the same logic as we want to use for id properties generated by the database and _rev property used by CouchDB/Cloudant.

I believe all database system will have an answer for this issue also.

CouchDB is an example of a database that makes many features difficult to implement, from patch updates to auto-updated properties.

How can we use it at the same time as migrations?

Good point about migrations. Adding to my list above, it would be great to find a declarative way allowing developers to include additional database-specific constraints for each property, and then modify our connectors to apply them during auto-migration.

For example:

@model()
class MyModel extends Entity {
  @property({
    mysql: {
      name: 'created_at',
      definition: 'NOT NULL DEFAULT CURRENT_TIMESTAMP'
    }
  })
  createdAt: Date;

  @property({
    mysql: {
      name: 'updated_at',
      definition: 'NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP'
    }
  })
  updatedAt: Date
}
haotangio commented 4 years ago

@bajtos Thanks for your help, I tried to make the mixin approach working, but was struggling with the repository type. Could you help to provide type information for the "repository" argument?

ivesdebruycker commented 4 years ago

@haotangio this worked for me:

export function TimeStampRepositoryMixin<
  E extends Entity & {createdAt?: Date; updatedAt?: Date},
  ID,
  R extends Constructor<EntityCrudRepository<E, ID>>
>(repository: R) {
  class MixedRepository extends repository {
    async create(entity: DataObject<E>, options?: Options): Promise<E> {
      entity.createdAt = new Date();
      entity.updatedAt = new Date();
      return super.create(entity, options);
    }

    async updateAll(
      data: DataObject<E>,
      where?: Where<E>,
      options?: Options,
    ): Promise<Count> {
      data.updatedAt = new Date();
      return super.updateAll(data, where, options);
    }

    async replaceById(
      id: ID,
      data: DataObject<E>,
      options?: Options,
    ): Promise<void> {
      data.updatedAt = new Date();
      return super.replaceById(id, data, options);
    }
  }
  return MixedRepository;
}

Based on https://loopback.io/doc/en/lb4/migration-models-mixins.html#defining-a-repository-mixin-class-factory-function

danysz commented 4 years ago

@haotangio this worked for me:

export function TimeStampRepositoryMixin<
  E extends Entity & {createdAt?: Date; updatedAt?: Date},
  ID,
  R extends Constructor<EntityCrudRepository<E, ID>>
>(repository: R) {
  class MixedRepository extends repository {
    async create(entity: DataObject<E>, options?: Options): Promise<E> {
      entity.createdAt = new Date();
      entity.updatedAt = new Date();
      return super.create(entity, options);
    }

    async updateAll(
      data: DataObject<E>,
      where?: Where<E>,
      options?: Options,
    ): Promise<Count> {
      data.updatedAt = new Date();
      return super.updateAll(data, where, options);
    }

    async replaceById(
      id: ID,
      data: DataObject<E>,
      options?: Options,
    ): Promise<void> {
      data.updatedAt = new Date();
      return super.replaceById(id, data, options);
    }
  }
  return MixedRepository;
}

Based on https://loopback.io/doc/en/lb4/migration-models-mixins.html#defining-a-repository-mixin-class-factory-function

I tried to implement your solution but the controllers are not working anymore.

faris-mobile89 commented 4 years ago

@haotangio this worked for me:

export function TimeStampRepositoryMixin<
  E extends Entity & {createdAt?: Date; updatedAt?: Date},
  ID,
  R extends Constructor<EntityCrudRepository<E, ID>>
>(repository: R) {
  class MixedRepository extends repository {
    async create(entity: DataObject<E>, options?: Options): Promise<E> {
      entity.createdAt = new Date();
      entity.updatedAt = new Date();
      return super.create(entity, options);
    }

    async updateAll(
      data: DataObject<E>,
      where?: Where<E>,
      options?: Options,
    ): Promise<Count> {
      data.updatedAt = new Date();
      return super.updateAll(data, where, options);
    }

    async replaceById(
      id: ID,
      data: DataObject<E>,
      options?: Options,
    ): Promise<void> {
      data.updatedAt = new Date();
      return super.replaceById(id, data, options);
    }
  }
  return MixedRepository;
}

Based on https://loopback.io/doc/en/lb4/migration-models-mixins.html#defining-a-repository-mixin-class-factory-function

I tried to implement your solution but the controllers are not working anymore.

@danysz
You need to implement the block into your repository file:

e.g.

import { DefaultCrudRepository } from '@loopback/repository';
import { Certificate, CertificateRelations } from '../models';
import { DbDataSource } from '../datasources';
import { inject, Constructor } from '@loopback/core';
import { TimeStampRepositoryMixin } from '../mixins/time-stamp.mixin';

export class CertificateRepository extends TimeStampRepositoryMixin<
  Certificate, typeof Certificate.prototype.id,
  Constructor<DefaultCrudRepository<Certificate, typeof Certificate.prototype.id, CertificateRelations>>
>(DefaultCrudRepository) {
  constructor(@inject('datasources.db') dataSource: DbDataSource) {
    super(Certificate, dataSource);
  }
}
pookdeveloper commented 4 years ago

I get an error:

Cannot start the application. Error: Cannot resolve injected arguments for PruebaRepository.[0]: The arguments[0] is not decorated for dependency injection, but a value i s not supplied

My repository:

import {Count, DataObject, DefaultCrudRepository, Entity, juggler, Options, Where} from '@loopback/repository';

export class PruebaRepository<T extends Entity, ID>
  extends DefaultCrudRepository<T, ID> {

  constructor(
    public entityClass: typeof Entity & {prototype: T},
    public dataSource: juggler.DataSource,
  ) {
    super(entityClass, dataSource);
    // TODO: check whether the model has updatedAt/createdAt properties?
  }

  async create(entity: DataObject<T>, options?: Options): Promise<T> {
    // entity.createdAt = new Date();
    // entity.updatedAt = new Date();
    return super.create(entity, options);
  }

  async updateAll(
    data: DataObject<T>,
    where?: Where<T>,
    options?: Options,
  ): Promise<Count> {
    // data.updatedAt = new Date();
    return super.updateAll(data, where, options);
  }

  async replaceById(
    id: ID,
    data: DataObject<T>,
    options?: Options,
  ): Promise<void> {
    // data.updatedAt = new Date();
    return super.replaceById(id, data, options);
  }
}

I use migration:

await this.migrateSchema();

Without this line it works.

Thanks!

pookdeveloper commented 4 years ago

Attach a simple project : (I use postgreslSql in the project) Archivo comprimido.zip Thanks

bajtos commented 4 years ago

@pookdeveloper The error message is saying that PruebaRepository cannot be instantiated from Inversion-of-Control context because the constructor arguments are not decorated with @inject and thus the context does not know what values to provide for those arguments.

Now the question is why is the context trying to create an instance of PruebaRepository? Take a look at how migrateSchema is implemented:

https://github.com/strongloop/loopback-next/blob/07c431bd62341cc04f7da18f9d7d17747153eba0/packages/repository/src/mixins/repository.mixin.ts#L194-L199

It searches for all repository bindings and then attempts to migrate each repository.

The question we need to ask now is why is PruebaRepository considered as a "regular" repository to be migrated, when it's not a regular model repository, but a base repository class instead.

The answer is in your project layout, where you put your PruebaRepository into src/repositories/prueba.repository.ts. When an application is booted, all files matching src/repositores/*.repository.ts are considered as model repositories and the exported repositories are automatically bound to the application context for dependency injection.

To solve the problem, just rename your repository file to something else. I recommend you to use prueba.repository.base.ts as the name, this will allow lb4 repository to recognize your custom repository base class when building a list of base classes to offer in its prompts.

pookdeveloper commented 4 years ago

@bajtos Thanks for all I will try it ! ;)

pookdeveloper commented 4 years ago

Works perfectly! https://github.com/strongloop/loopback-next/issues/1857#issuecomment-620589676

haotangio commented 4 years ago

@ivesdebruycker It worked! Thanks a lot man! :)

pookdeveloper commented 4 years ago

@bajtos about : https://github.com/strongloop/loopback-next/issues/1857#issuecomment-620589676 Its work but I need use transaction and i give this error:

image

image

I try that but i don´t know how resolve that: image

Thanks for advance!

bajtos commented 4 years ago

@pookdeveloper I am not very familiar with Transaction-related APIs myself. Can you try to use DefaultTransactionalRepository instead of DefaultCrudRepository?

In the future, please post code snippets in text, so that we can copy, paste and edit to show you a fixed version. See https://help.github.com/en/github/writing-on-github/creating-and-highlighting-code-blocks

pookdeveloper commented 4 years ago

@bajtos thanks I will try it!

Clauber commented 3 years ago

Are you guys planning on pushing this to loopback so we have it within the lib, or is this something we have to implement ourselves as we use loopback?