nosql

nosql copied to clipboard

Hi there, first of all I want to thank you for all this wonderfull work. Nosql was the kind of specification/abstraction Jakarta EE needed and I think that all of this (beginning with the 4 different kind of nosql interfaces) is a stroke of genius.

Let's get into business:

1. There is a typo : jakarta.nosql.mapping.EntityPostPersit should be jakarta.nosql.mapping.EntityPostPersist

2. I believe using interfaces to manage PrePersist & PostPersist events is a mistake. JPA uses annotations : javax.persistence.PrePersist javax.persistence.PostPersist

Why annotations are better ? Imagine you have a hierarchy of MappedSuperclass, if you override EntityPrePersist methods you have to pay attention to call super implementations. This is prone error, you may just want to "do things" in every MappedSuperclass without knowing such interfaces were already implemented.

Please consider using annotations like jpa and scanning every method (in the whole hierarchy) and call each of them to handle prepersist and post persist events :

package jakarta.nosql.mapping;

import java.lang.annotation.Target;
import java.lang.annotation.Retention;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

/**
 * Specifies a callback method for the corresponding 
 * lifecycle event. This annotation may be applied to methods 
 * of an entity class or a mapped superclass.
 *
 * @since 1.0
 */
@Target({METHOD}) 
@Retention(RUNTIME)

public @interface PrePersist {}

and

package jakarta.nosql.mapping;

import java.lang.annotation.Target;
import java.lang.annotation.Retention;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

/**
 * Specifies a callback method for the corresponding 
 * lifecycle event. This annotation may be applied to methods 
 * of an entity class or a mapped superclass.
 *
 * @since 1.0
 */
@Target({METHOD}) 
@Retention(RUNTIME)

public @interface PostPersist {}

Even more, you may have entities with an attribute "value" and the getValue() could be misleading and could get in the way.

If you agree with me (to change the spec) I can make a pull request here (the spec) and a pull request for the reference implementation to help you.

Hello @amoscatelli Thank you for the suggestion. That sounds like an amazing idea. I'll put it as an enhancement to release after the 1.0.0 release. Right now, we're closing the changes in the API to release this version and then push new improvements in the next versions.

Hi @otaviojava
So you plan to keep both of them (interfaces and annotations) ? Otherwise you'll need to deprecate and remove interfaces later ...

Yeap, or maybe use the Jakarta commons annotation instead:

• https://jakarta.ee/specifications/platform/9/apidocs/jakarta/annotation/postconstruct
• https://jakarta.ee/specifications/platform/9/apidocs/jakarta/annotation/predestroy

Probably keeping them both would be better. I can't see a clear mapping/semantic with those annotations.

Hey, @amoscatelli, we reduced the spec to this first version. I'll close this one; we can start on the JNoSQL side instead.