-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
How to migrate CRUD Operation Hooks #3952
Comments
How about decorators inside the repository to allow a particular method to become part of the event (before/after etc) @observe('before-save')
async myMethodInsideRepository(rec: MyModel) : Promise<MyModel> {
} |
@bajtos how about this alternate interface? // repo is a Repository instance
repo.observe('before save', async ctx => {
console.log(
'Going to write to an instance of model %s',
ctx.Model.modelName,
);
}); The hooks are not hard-coded in the repository class, you add them as required using an instance method. |
@hacksparrow can you please clarify how is your interface dealing with the following problem?
Also IMO, setup of operation hooks is a responsibility of per-model repository class (e.g. |
@bajtos I am yet to start working on a solution for preventing registration of an operation hook multiple times, but it would be the same for whatever interface we choose ultimately. I am wondering, since the repository instance is destroyed after a request is responded and a new one created for each request. Where is the possibility of installing the same hook multiple times? Unless it is done intentionally by the developer in the repository? Even that should be taken care of by Juggler, if it is not doing it already.
I was thinking, the code in the model's repo would be: this.observe('before save', async ctx => {
console.log(
'Going to write to an instance of model %s',
ctx.Model.modelName,
);
});
|
DefaultCrudRepository needs a backing At the moment, DefaultCrudRepository checks if the backing PersistedModel has been created before (by another instance created previously) and does not recreate the model in such case. See here: Now we need to find such design that will make it easy for people writing their custom Repository classes inheriting from DefaultCrudRepository to register operation hooks on the backing PersistedModel only once. The following naive solution will register a new hook every time a new instance of class MyRepo extends DefaultCrudRepository</*...*/> {
constructor(/*...*/) {
super(/*...*/);
this.modelClass.observe('before save', async ctx => {/*...*/});
}
}
I like that proposal 👍 as long as Please keep in mind that in this user story, we want to find a solution that will allow LB3 developers to migrate their operation hooks to LB4 with minimal extra implementation effort required on our side. The solution based on PersistedModel's |
Closed via #4730 |
This is a follow-up for #3718 and #3922.
Write content for
docs/site/migration/models/operation-hooks.md
, explain how to migrate operation hooks (see LB3 docs: Operation Hooks).Explain that we don't have first-class support for Operation Hooks in LB4 yet, refer to #1919
As a temporary solution, describe how to leverage
PersistedModel
used by our legacy juggler bridge. Example implementation:Caveats:
(1)
Repository constructor is executed once for each incoming request. We don't want to install another copy of the same operation hook on each request, we need to find a way how to install hooks only once. Ideally,
DefaultCrudRepository
should provide a protected method that subclasses can use to supply one-time model setup.For example:
(2)
Current TypeScript typings in loopback-datasource-juggler do not contain operation hooks and
.observe()
method. We need to fix this.Acceptance criteria
definePersistedModel
toDefaultCrudRepository
, this method should overriden by user's repository class to access the model class where the operation hooks can be applied.The text was updated successfully, but these errors were encountered: