first rough proposal for events #876
Conversation
An intriguing possibility here would be to generate event types into the static metamodel, so that you could write: void afterUpdate(@Observes _Book.PostUpdateEvent event) { ... } |
I take that back. Apparently they relaxed this restriction, and you can now fire a parameterized event as long as the type argument is somehow resolvable, for example: @Inject
Event<MyEvent<String>> event;Or: beanManager.getEvent()
.select(new TypeLiteral<MyEvent<String>>() {})
.fire(new MyEvent<>(title));So in fact we can use a parameterized event type, e.g. |
see jakartaee#373 Signed-off-by: Gavin King <[email protected]>
There was a problem hiding this comment.
This is nicely written and I like the change to use the parameterized entity type rather than Object.
I made some minor suggestions and proposed switching the Abstract supertype for the event into an interface.
I addition to the inline review comments, there are a few other things we will need to consider.
One important item is clarifying what the user is (or is not) allowed to do with the entity instance that they get within the event. Are they allowed to modify it? And if so, do those modifications get persisted to the datastore?
The entity value returned on the Post events is a difficult question to consider. I can certainly see why you wrote it to state the value passed in the lifecycle method, because that makes the most sense for PostDeleteEvent (to avoid extra fetch of values) and is consistent with the Delete lifecycle method which only supports a void return value.
For the PostInsertEvent and PostUpdateEvent, I wonder if some implementations based on a JPA provider might inadvertently end up with the entity value updated to include an automatically generated id and autoincremented version, which would no longer match the passed in entity value. And having that information might be useful to the user as well, so I could see a case for defining these differently, but then they would be inconsistent. Not really sure what is best here, but I thought I would at least bring it up.
| public abstract class LifecycleEvent<E> { | ||
| private final E entity; | ||
|
|
||
| public LifecycleEvent(E entity) { | ||
| this.entity = entity; | ||
| } | ||
|
|
||
| /** | ||
| * The entity instance which was passed as an argument to | ||
| * the lifecycle method. | ||
| */ | ||
| public E entity() { | ||
| return entity; | ||
| } |
There was a problem hiding this comment.
LifecycleEvent could be an interface rather than an abstract class,
| public abstract class LifecycleEvent<E> { | |
| private final E entity; | |
| public LifecycleEvent(E entity) { | |
| this.entity = entity; | |
| } | |
| /** | |
| * The entity instance which was passed as an argument to | |
| * the lifecycle method. | |
| */ | |
| public E entity() { | |
| return entity; | |
| } | |
| public interface LifecycleEvent<E> { | |
| /** | |
| * The entity instance which was passed as an argument to | |
| * the lifecycle method. | |
| */ | |
| E entity(); |
| public class PostDeleteEvent<E> extends LifecycleEvent<E> { | ||
| public PostDeleteEvent(E entity) { | ||
| super(entity); | ||
| } |
There was a problem hiding this comment.
If making LifecycleEvent into an interface (earlier comment), then it PostDeleteEvent might be implemented as:
| public class PostDeleteEvent<E> extends LifecycleEvent<E> { | |
| public PostDeleteEvent(E entity) { | |
| super(entity); | |
| } | |
| public record PostDeleteEvent<E>(E entity) implements LifecycleEvent<E> { |
| public class PreUpdateEvent<E> extends LifecycleEvent<E> { | ||
| public PreUpdateEvent(E entity) { | ||
| super(entity); | ||
| } |
There was a problem hiding this comment.
If switching to interface,
| public class PreUpdateEvent<E> extends LifecycleEvent<E> { | |
| public PreUpdateEvent(E entity) { | |
| super(entity); | |
| } | |
| public record PreUpdateEvent<E>(E entity) implements LifecycleEvent<E> { |
We can look at section 3.6.2 of the JPA spec for inspiration here.
Yeah good point. Hrrm. Not sure. |
Co-authored-by: Nathan Rauh <[email protected]>
Co-authored-by: Nathan Rauh <[email protected]>
Co-authored-by: Nathan Rauh <[email protected]>
Co-authored-by: Nathan Rauh <[email protected]>
Co-authored-by: Nathan Rauh <[email protected]>
Co-authored-by: Nathan Rauh <[email protected]>
Co-authored-by: Nathan Rauh <[email protected]>
Co-authored-by: Nathan Rauh <[email protected]>
Co-authored-by: Nathan Rauh <[email protected]>
Co-authored-by: Nathan Rauh <[email protected]>
| @@ -115,6 +117,11 @@ This fragment shows how the application might request injection of a `CarReposit | |||
|
|
|||
| This integration between CDI and Jakarta Data allows for seamless management of repository instances within Jakarta EE applications. | |||
|
|
|||
| ==== CDI Events | |||
There was a problem hiding this comment.
We should differentiate these lifecycle events from other events, such as Domain Event, see: #374
There was a problem hiding this comment.
Honestly I don't really see what the difference is. How is a "Domain Event" different from a lifecycle event for an entity that you're choosing call an "aggregate"?
There was a problem hiding this comment.
Right, that's what I thought.
Co-authored-by: Kyle Aure <[email protected]>
I tried this out on the EclipseLink JPA provider and observed it to be doing the following: After invoking After invoking Note that Jakarta Data's CrudRepository.insert has language like, "After invoking this method, do not continue to use the instance that is supplied as a parameter. This method makes no guarantees about the state of the instance that is supplied as a parameter." All of this makes me uncomfortable about supplying that same instance via the PostInsert/PostUpdate events. But I can also see why we don't want to require it to supply an updated entity instance. That might require some implementations to do extra fetching just for the sake of publishing events that the application might not even care about. Leaving out the entity from the event makes the Post event not useful because you cannot correlate it with the Pre event. I suppose one option would be to supply only the Id value and entity class (not instance) on the Post event which would at least identify it as matching the Pre event, but that is awkward. Not really sure what would be best. |
I mean with things like this, there's always the option of saying "portable applications should not rely on .... ". |
See #373. [Note that the first to propose this was @hantsy.]
Here's a rough and fairly minimal idea for what this could look like.
Note that here there is an
Eventclass for each kind of event, instead of a singleLifecycleEventwith@Before @Updateand@After @Insertqualifiers on the observer side. The reason for this is that we don't have a dependency on CDI, so I can't use the@Qualifierannotation to declare event qualifier types.An event observer would look like:
I would love to be able to filter by entity type, but I don't think CDI allows filtering events by type argument. (At least it didn't in 1.0, though that was a long time ago.)As a workaround for that, we could have@Before @Update @EntityType(Book.class)as qualifiers, but, again, that would pick up a dependency on CDI.