You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/02/16 11:11:56 UTC
[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #2254: Nitrite native support #1298
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:
users@infra.apache.org