[GitHub] [camel-quarkus] JiriOndrusek opened a new pull request #2254: Nitrite native support #1298

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek opened a new pull request #2254: Nitrite native support #1298

GitBox

JiriOndrusek opened a new pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254


   fixes https://github.com/apache/camel-quarkus/issues/1298
   
   requires quarkus 1.13 (requires change https://github.com/quarkusio/quarkus/pull/15043)
   
   [ ] An issue should be filed for the change unless this is a trivial change (fixing a typo or similar). One issue should ideally be fixed by not more than one commit and the other way round, each commit should fix just one issue, without pulling in other changes.
   [ ] Each commit in the pull request should have a meaningful and properly spelled subject line and body. Copying the title of the associated issue is typically enough. Please include the issue number in the commit message prefixed by #.
   [ ] The pull request description should explain what the pull request does, how, and why. If the info is available in the associated issue or some other external document, a link is enough.
   [ ] Phrases like Fix #<issueNumber> or Fixes #<issueNumber> will auto-close the named issue upon merging the pull request. Using them is typically a good idea.
   [ ] Please run mvn process-resources -Pformat (and amend the changes if necessary) before sending the pull request.
   [ ] Contributor guide is your good friend: https://camel.apache.org/camel-quarkus/latest/contributor-guide.html


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #2254: Nitrite native support #1298

GitBox

ppalaga commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576724717



##########
File path: extensions/nitrite/deployment/src/main/java/org/apache/camel/quarkus/component/nitrite/deployment/NitriteProcessor.java
##########
@@ -38,25 +33,15 @@ FeatureBuildItem feature() {
         return new FeatureBuildItem(FEATURE);
     }
 
-    /**
-     * Remove this once this extension starts supporting the native mode.
-     */
-    @BuildStep(onlyIf = NativeBuild.class)
-    @Record(value = ExecutionTime.RUNTIME_INIT)
-    void warnJvmInNative(JvmOnlyRecorder recorder) {
-        JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
-        recorder.warnJvmInNative(FEATURE); // warn at runtime
-    }
-
     @BuildStep
-    void runtimeInitializedClasses(BuildProducer<RuntimeInitializedClassBuildItem> runtimeInitializedClasses) {
+    RuntimeInitializedClassBuildItem runtimeInitBcryptUtil() {

Review comment:
       ```suggestion
       RuntimeInitializedClassBuildItem runtimeInitializedClasses() {
   ```
   (Is there any relationship to bcrypt?)

##########
File path: integration-tests/nitrite/src/main/java/org/apache/camel/quarkus/component/nitrite/it/NitriteResource.java
##########
@@ -51,12 +52,13 @@
     @Inject
     ConsumerTemplate consumerTemplate;
 
-    @Path("/repositoryClass")
-    @GET
+    @Path("/getRepositoryClass")
+    @POST

Review comment:
       I think this should better be `@GET` to better correspond to what it does

##########
File path: extensions/nitrite/deployment/src/main/java/org/apache/camel/quarkus/component/nitrite/deployment/NitriteProcessor.java
##########
@@ -38,25 +33,15 @@ FeatureBuildItem feature() {
         return new FeatureBuildItem(FEATURE);
     }
 
-    /**
-     * Remove this once this extension starts supporting the native mode.
-     */
-    @BuildStep(onlyIf = NativeBuild.class)
-    @Record(value = ExecutionTime.RUNTIME_INIT)
-    void warnJvmInNative(JvmOnlyRecorder recorder) {
-        JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
-        recorder.warnJvmInNative(FEATURE); // warn at runtime
-    }
-
     @BuildStep
-    void runtimeInitializedClasses(BuildProducer<RuntimeInitializedClassBuildItem> runtimeInitializedClasses) {
+    RuntimeInitializedClassBuildItem runtimeInitBcryptUtil() {
         // this class uses a SecureRandom which needs to be initialised at run time
-        runtimeInitializedClasses.produce(new RuntimeInitializedClassBuildItem("org.dizitart.no2.Security"));
+        return new RuntimeInitializedClassBuildItem("org.dizitart.no2.Security");
     }
 
     @BuildStep
-    void reflectiveClasses(BuildProducer<ReflectiveClassBuildItem> reflectiveClasses) {
-        reflectiveClasses.produce(new ReflectiveClassBuildItem(false, false, FilePathNio.class));
+    List<ReflectiveClassBuildItem> registerMetricsForReflection() {

Review comment:
       ```suggestion
       List<ReflectiveClassBuildItem> reflectiveClasses() {
   ```
   
   (Why metrics?)

##########
File path: integration-tests/nitrite/src/main/java/org/apache/camel/quarkus/component/nitrite/it/NitriteResource.java
##########
@@ -84,11 +92,12 @@ public Object postRepositoryClass(Employee object) {
     @POST
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
-    public Object postRepositoryClassOperation(Operation operation) {
+    public Object postRepositoryClassOperation(Operation operation, @QueryParam("mappable") boolean mappable) {
+        String className = mappable ? EmployeeMappable.class.getName() : EmployeeSerializable.class.getName();
         LOG.debugf("Sending to nitrite: {%s}", operation);
         return producerTemplate.toF("nitrite://%s?repositoryClass=%s",
-                dbFile, Employee.class.getName())
-                .withBody(operation.getEmployee())
+                dbFile, className)
+                .withBody(mappable ? operation.getEmployeeMappable() : operation.getEmployeeSerializable())

Review comment:
       I do not see `getCollection()` and `postCollection` being used anywhere. Maybe you wanted to add more tests?

##########
File path: integration-tests/nitrite/src/test/java/org/apache/camel/quarkus/component/nitrite/it/NitriteTest.java
##########
@@ -64,12 +98,15 @@ public void repositoryClass() throws CloneNotSupportedException {
         /* Insert Leonard */
         RestAssured.given()
                 .contentType(ContentType.JSON)
+                .queryParam("mappable", mappable)
                 .body(leonard)
                 .post("/nitrite/repositoryClass")
                 .then()
                 .statusCode(200)
                 .body("name", is("Leonard"));
-        RestAssured.get("/nitrite/repositoryClass")
+        RestAssured.given()
+                .queryParam("mappable", mappable)
+                .post("/nitrite/getRepositoryClass")

Review comment:
       This may fail because `getRepositoryClass` calls `consumerTemplate.receiveNoWait()` and there is some concurrency and i/o delays in game. A `consumerTemplate.receive*(...)` variant with timeout would perhaps be a better choice.

##########
File path: integration-tests/nitrite/src/test/java/org/apache/camel/quarkus/component/nitrite/it/NitriteTest.java
##########
@@ -78,28 +115,42 @@ public void repositoryClass() throws CloneNotSupportedException {
         /* Insert Irma */
         RestAssured.given()
                 .contentType(ContentType.JSON)
+                .queryParam("mappable", mappable)
                 .body(irma)
                 .post("/nitrite/repositoryClass")
                 .then()
                 .statusCode(200)
                 .body("name", is("Irma"));
-        RestAssured.get("/nitrite/repositoryClass")
+        RestAssured.given()
+                .queryParam("mappable", mappable)
+                .post("/nitrite/getRepositoryClass")
                 .then()
                 .statusCode(200)
                 .header(NitriteConstants.CHANGE_TYPE, "INSERT")
                 .body("name", is("Irma"));
 
-        Employee updatedSheldon = (Employee) sheldon.clone();
+        Employee updatedSheldon = null;
+        if (sheldon instanceof EmployeeSerializable) {
+            updatedSheldon = (EmployeeSerializable) sheldon.clone();
+        } else {
+            updatedSheldon = (EmployeeMappable) sheldon.clone();
+        }
         updatedSheldon.setAddress("Moon");
+
         RestAssured.given()
                 .contentType(ContentType.JSON)
-                .body(new Operation(Operation.Type.update, "name", "Sheldon", updatedSheldon))
+                .body(new Operation(Operation.Type.update, "name", "Sheldon",
+                        mappable ? null : (EmployeeSerializable) updatedSheldon,
+                        mappable ? (EmployeeMappable) updatedSheldon : null))
+                .queryParam("mappable", mappable)
                 .post("/nitrite/repositoryClassOperation")
                 .then()
                 .body("name", is("Sheldon"),
                         "address", is("Moon"));
 
-        RestAssured.get("/nitrite/repositoryClass")
+        RestAssured.given()

Review comment:
       This may fail too because of `receiveNoWait()`

##########
File path: extensions/nitrite/runtime/src/main/doc/usage.adoc
##########
@@ -0,0 +1,12 @@
+If your persistence objects implement `java.io.Serializable`, you have to add configuration of all serializable classes.
+There is a new serialization support in GraalVM 21.0. Developers can configure classes
+for serialization via the serialization configuration file
+`-H:SerializationConfigurationResources=/path/to-serialization-config.json` option. For more
+information see https://github.com/oracle/graal/pull/2730[PR with feature].
+
+If your persistence objects implement `org.dizitart.no2.mapper.Mappable`. All classes have to
+implement also `java.io.Serializable` and have to be registered for serialization (see previous option),
+even though the Java serialization won't be used.
+
+
+

Review comment:
       ```suggestion
   ```
   
   Redundant whitespace




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

aldettinger commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576761437



##########
File path: integration-tests/nitrite/src/main/java/org/apache/camel/quarkus/component/nitrite/it/NitriteResource.java
##########
@@ -71,11 +73,17 @@ public Response getRepositoryClass() throws Exception {
     @POST
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
-    public Object postRepositoryClass(Employee object) {
+    public Object postRepositoryClass(EmployeeSerializable object, @QueryParam("mappable") boolean mappable) {
+        String className = mappable ? EmployeeMappable.class.getName() : EmployeeSerializable.class.getName();
+        //if object, is mappable, construct it from serializable (it is conversion caused by tthe type in method parameter)

Review comment:
       small typo here "tthe"




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

aldettinger commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576764549



##########
File path: extensions/nitrite/runtime/src/main/resources/META-INF/native-image/serialization-config.json
##########
@@ -0,0 +1,38 @@
+[

Review comment:
       Maybe, we can have a follow up issue to move the config to the deployment processor when/if https://github.com/quarkusio/quarkus/issues/14530 is implemented ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576769146



##########
File path: extensions/nitrite/runtime/src/main/resources/META-INF/native-image/serialization-config.json
##########
@@ -0,0 +1,38 @@
+[

Review comment:
       Yes, I didn't mention it, but the issue already exists: https://github.com/apache/camel-quarkus/issues/2255




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576777340



##########
File path: integration-tests/nitrite/src/test/java/org/apache/camel/quarkus/component/nitrite/it/NitriteTest.java
##########
@@ -64,12 +98,15 @@ public void repositoryClass() throws CloneNotSupportedException {
         /* Insert Leonard */
         RestAssured.given()
                 .contentType(ContentType.JSON)
+                .queryParam("mappable", mappable)
                 .body(leonard)
                 .post("/nitrite/repositoryClass")
                 .then()
                 .statusCode(200)
                 .body("name", is("Leonard"));
-        RestAssured.get("/nitrite/repositoryClass")
+        RestAssured.given()
+                .queryParam("mappable", mappable)
+                .post("/nitrite/getRepositoryClass")

Review comment:
       @ppalaga  I think, that it is not necessary. Value is written into nitrite with previous post, and nitrite component waits until nitrite finishes: https://github.com/apache/camel/blob/master/components/camel-nitrite/src/main/java/org/apache/camel/component/nitrite/NitriteConsumer.java#L71
   Then reading should work even without wait. WDYT? (to be sure, I also tried to "wait" in nitrite during insert, control is not released to camel component)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576782244



##########
File path: extensions/nitrite/deployment/src/main/java/org/apache/camel/quarkus/component/nitrite/deployment/NitriteProcessor.java
##########
@@ -38,25 +33,15 @@ FeatureBuildItem feature() {
         return new FeatureBuildItem(FEATURE);
     }
 
-    /**
-     * Remove this once this extension starts supporting the native mode.
-     */
-    @BuildStep(onlyIf = NativeBuild.class)
-    @Record(value = ExecutionTime.RUNTIME_INIT)
-    void warnJvmInNative(JvmOnlyRecorder recorder) {
-        JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
-        recorder.warnJvmInNative(FEATURE); // warn at runtime
-    }
-
     @BuildStep
-    void runtimeInitializedClasses(BuildProducer<RuntimeInitializedClassBuildItem> runtimeInitializedClasses) {
+    RuntimeInitializedClassBuildItem runtimeInitBcryptUtil() {

Review comment:
       copy/paste error, fixed, thanks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576782354



##########
File path: extensions/nitrite/deployment/src/main/java/org/apache/camel/quarkus/component/nitrite/deployment/NitriteProcessor.java
##########
@@ -38,25 +33,15 @@ FeatureBuildItem feature() {
         return new FeatureBuildItem(FEATURE);
     }
 
-    /**
-     * Remove this once this extension starts supporting the native mode.
-     */
-    @BuildStep(onlyIf = NativeBuild.class)
-    @Record(value = ExecutionTime.RUNTIME_INIT)
-    void warnJvmInNative(JvmOnlyRecorder recorder) {
-        JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
-        recorder.warnJvmInNative(FEATURE); // warn at runtime
-    }
-
     @BuildStep
-    void runtimeInitializedClasses(BuildProducer<RuntimeInitializedClassBuildItem> runtimeInitializedClasses) {
+    RuntimeInitializedClassBuildItem runtimeInitBcryptUtil() {
         // this class uses a SecureRandom which needs to be initialised at run time
-        runtimeInitializedClasses.produce(new RuntimeInitializedClassBuildItem("org.dizitart.no2.Security"));
+        return new RuntimeInitializedClassBuildItem("org.dizitart.no2.Security");
     }
 
     @BuildStep
-    void reflectiveClasses(BuildProducer<ReflectiveClassBuildItem> reflectiveClasses) {
-        reflectiveClasses.produce(new ReflectiveClassBuildItem(false, false, FilePathNio.class));
+    List<ReflectiveClassBuildItem> registerMetricsForReflection() {

Review comment:
       also copy/paste error, fixed, thanks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576782462



##########
File path: integration-tests/nitrite/src/main/java/org/apache/camel/quarkus/component/nitrite/it/NitriteResource.java
##########
@@ -51,12 +52,13 @@
     @Inject
     ConsumerTemplate consumerTemplate;
 
-    @Path("/repositoryClass")
-    @GET
+    @Path("/getRepositoryClass")
+    @POST

Review comment:
        fixed, thanks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576782642



##########
File path: extensions/nitrite/runtime/src/main/doc/usage.adoc
##########
@@ -0,0 +1,12 @@
+If your persistence objects implement `java.io.Serializable`, you have to add configuration of all serializable classes.
+There is a new serialization support in GraalVM 21.0. Developers can configure classes
+for serialization via the serialization configuration file
+`-H:SerializationConfigurationResources=/path/to-serialization-config.json` option. For more
+information see https://github.com/oracle/graal/pull/2730[PR with feature].
+
+If your persistence objects implement `org.dizitart.no2.mapper.Mappable`. All classes have to
+implement also `java.io.Serializable` and have to be registered for serialization (see previous option),
+even though the Java serialization won't be used.
+
+
+

Review comment:
        fixed, thanks

##########
File path: integration-tests/nitrite/src/main/java/org/apache/camel/quarkus/component/nitrite/it/NitriteResource.java
##########
@@ -71,11 +73,17 @@ public Response getRepositoryClass() throws Exception {
     @POST
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
-    public Object postRepositoryClass(Employee object) {
+    public Object postRepositoryClass(EmployeeSerializable object, @QueryParam("mappable") boolean mappable) {
+        String className = mappable ? EmployeeMappable.class.getName() : EmployeeSerializable.class.getName();
+        //if object, is mappable, construct it from serializable (it is conversion caused by tthe type in method parameter)

Review comment:
        fixed, thanks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576782924



##########
File path: integration-tests/nitrite/src/test/java/org/apache/camel/quarkus/component/nitrite/it/NitriteTest.java
##########
@@ -78,28 +115,42 @@ public void repositoryClass() throws CloneNotSupportedException {
         /* Insert Irma */
         RestAssured.given()
                 .contentType(ContentType.JSON)
+                .queryParam("mappable", mappable)
                 .body(irma)
                 .post("/nitrite/repositoryClass")
                 .then()
                 .statusCode(200)
                 .body("name", is("Irma"));
-        RestAssured.get("/nitrite/repositoryClass")
+        RestAssured.given()
+                .queryParam("mappable", mappable)
+                .post("/nitrite/getRepositoryClass")
                 .then()
                 .statusCode(200)
                 .header(NitriteConstants.CHANGE_TYPE, "INSERT")
                 .body("name", is("Irma"));
 
-        Employee updatedSheldon = (Employee) sheldon.clone();
+        Employee updatedSheldon = null;
+        if (sheldon instanceof EmployeeSerializable) {
+            updatedSheldon = (EmployeeSerializable) sheldon.clone();
+        } else {
+            updatedSheldon = (EmployeeMappable) sheldon.clone();
+        }
         updatedSheldon.setAddress("Moon");
+
         RestAssured.given()
                 .contentType(ContentType.JSON)
-                .body(new Operation(Operation.Type.update, "name", "Sheldon", updatedSheldon))
+                .body(new Operation(Operation.Type.update, "name", "Sheldon",
+                        mappable ? null : (EmployeeSerializable) updatedSheldon,
+                        mappable ? (EmployeeMappable) updatedSheldon : null))
+                .queryParam("mappable", mappable)
                 .post("/nitrite/repositoryClassOperation")
                 .then()
                 .body("name", is("Sheldon"),
                         "address", is("Moon"));
 
-        RestAssured.get("/nitrite/repositoryClass")
+        RestAssured.given()

Review comment:
       I think it is ok, see previous comment




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576784902



##########
File path: integration-tests/nitrite/src/main/java/org/apache/camel/quarkus/component/nitrite/it/NitriteResource.java
##########
@@ -84,11 +92,12 @@ public Object postRepositoryClass(Employee object) {
     @POST
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
-    public Object postRepositoryClassOperation(Operation operation) {
+    public Object postRepositoryClassOperation(Operation operation, @QueryParam("mappable") boolean mappable) {
+        String className = mappable ? EmployeeMappable.class.getName() : EmployeeSerializable.class.getName();
         LOG.debugf("Sending to nitrite: {%s}", operation);
         return producerTemplate.toF("nitrite://%s?repositoryClass=%s",
-                dbFile, Employee.class.getName())
-                .withBody(operation.getEmployee())
+                dbFile, className)
+                .withBody(mappable ? operation.getEmployeeMappable() : operation.getEmployeeSerializable())

Review comment:
       This is only difference in method names and mapped url. Both methods are used. Names are fixed, thanks.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

ppalaga commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576824552



##########
File path: integration-tests/nitrite/src/test/java/org/apache/camel/quarkus/component/nitrite/it/NitriteTest.java
##########
@@ -64,12 +98,15 @@ public void repositoryClass() throws CloneNotSupportedException {
         /* Insert Leonard */
         RestAssured.given()
                 .contentType(ContentType.JSON)
+                .queryParam("mappable", mappable)
                 .body(leonard)
                 .post("/nitrite/repositoryClass")
                 .then()
                 .statusCode(200)
                 .body("name", is("Leonard"));
-        RestAssured.get("/nitrite/repositoryClass")
+        RestAssured.given()
+                .queryParam("mappable", mappable)
+                .post("/nitrite/getRepositoryClass")

Review comment:
       You mean the consumer receives the data change event from Nitrite on the same thread where producer is writing the data? If so, yes, your setup may work well. But such an assumption sounds like an Nitrite's implementation detail that may change and it seems to be safer not to rely on that in our test.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

ppalaga commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576828834



##########
File path: integration-tests/nitrite/src/main/java/org/apache/camel/quarkus/component/nitrite/it/NitriteResource.java
##########
@@ -84,11 +92,12 @@ public Object postRepositoryClass(Employee object) {
     @POST
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
-    public Object postRepositoryClassOperation(Operation operation) {
+    public Object postRepositoryClassOperation(Operation operation, @QueryParam("mappable") boolean mappable) {
+        String className = mappable ? EmployeeMappable.class.getName() : EmployeeSerializable.class.getName();
         LOG.debugf("Sending to nitrite: {%s}", operation);
         return producerTemplate.toF("nitrite://%s?repositoryClass=%s",
-                dbFile, Employee.class.getName())
-                .withBody(operation.getEmployee())
+                dbFile, className)
+                .withBody(mappable ? operation.getEmployeeMappable() : operation.getEmployeeSerializable())

Review comment:
       Thanks, I can see the test now.
   
   I was CTRL-F-ing for `/collection` in browser and for some reason it was not finding anything.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

aldettinger commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r576866462



##########
File path: extensions/nitrite/runtime/src/main/resources/META-INF/native-image/serialization-config.json
##########
@@ -0,0 +1,38 @@
+[

Review comment:
       Great :+1:




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

ppalaga commented on pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#issuecomment-781902647


   @JiriOndrusek could you please check the test failures?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#issuecomment-781934978


   @ppalaga  I'll  look into it


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r579028288



##########
File path: integration-tests/nitrite/src/test/java/org/apache/camel/quarkus/component/nitrite/it/NitriteTest.java
##########
@@ -64,12 +98,15 @@ public void repositoryClass() throws CloneNotSupportedException {
         /* Insert Leonard */
         RestAssured.given()
                 .contentType(ContentType.JSON)
+                .queryParam("mappable", mappable)
                 .body(leonard)
                 .post("/nitrite/repositoryClass")
                 .then()
                 .statusCode(200)
                 .body("name", is("Leonard"));
-        RestAssured.get("/nitrite/repositoryClass")
+        RestAssured.given()
+                .queryParam("mappable", mappable)
+                .post("/nitrite/getRepositoryClass")

Review comment:
       I'll refactor it as you suggested. It would make more sense, I can not be 100% sure, that this behavior will stay...




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#issuecomment-781939717


   @ppalaga Failures will go away, once we switch quarkus to 1.13 (requires change quarkusio/quarkus#15043)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] JiriOndrusek commented on a change in pull request #2254: Nitrite native support #1298

GitBox
In reply to this post by GitBox

JiriOndrusek commented on a change in pull request #2254:
URL: https://github.com/apache/camel-quarkus/pull/2254#discussion_r579043850



##########
File path: integration-tests/nitrite/src/test/java/org/apache/camel/quarkus/component/nitrite/it/NitriteTest.java
##########
@@ -64,12 +98,15 @@ public void repositoryClass() throws CloneNotSupportedException {
         /* Insert Leonard */
         RestAssured.given()
                 .contentType(ContentType.JSON)
+                .queryParam("mappable", mappable)
                 .body(leonard)
                 .post("/nitrite/repositoryClass")
                 .then()
                 .statusCode(200)
                 .body("name", is("Leonard"));
-        RestAssured.get("/nitrite/repositoryClass")
+        RestAssured.given()
+                .queryParam("mappable", mappable)
+                .post("/nitrite/getRepositoryClass")

Review comment:
       fixed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


12