fix(java-sdk): enforce entity id presence on http calls (#1974)

lightbend · Jan 17, 2024 · eec0a18 · eec0a18
1 parent 687c716
commit eec0a18
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 7 deletions.
diff --git a/sdk/java-sdk-spring/src/it/java/com/example/wiring/SpringSdkIntegrationTest.java b/sdk/java-sdk-spring/src/it/java/com/example/wiring/SpringSdkIntegrationTest.java
@@ -107,6 +107,38 @@ public void failRequestWithRequiredQueryParam() {
     assertThat(response.getBody()).isEqualTo("Required request parameter is missing: longValue");
   }
 
+  @Test
+  public void acceptRequestWithMissingPathParamIfNotEntityId() {
+
+    ResponseEntity<String> response =
+        webClient
+            .get()
+            .uri("/echo/message/") // missing param
+            .retrieve()
+            .toEntity(String.class)
+            .onErrorResume(WebClientResponseException.class, error -> Mono.just(ResponseEntity.status(error.getStatusCode()).body(error.getResponseBodyAsString())))
+            .block(timeout);
+
+    assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK);
+  }
+
+  @Test
+  public void failRequestWithMissingRequiredIntPathParam() {
+
+    ResponseEntity<String> response =
+        webClient
+            .get()
+            .uri("/echo/int/") // missing param
+            .retrieve()
+            .toEntity(String.class)
+            .onErrorResume(WebClientResponseException.class, error -> Mono.just(ResponseEntity.status(error.getStatusCode()).body(error.getResponseBodyAsString())))
+            .block(timeout);
+
+    assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
+    assertThat(response.getBody()).isEqualTo("Path contains value of wrong type! Expected field of type INT32.");
+  }
+
+
   @Test
   public void verifyRequestWithOptionalQueryParams() {
 
@@ -401,6 +433,22 @@ public void verifyCounterViewMultipleSubscriptions() throws InterruptedException
         new IsEqual<>(2));
   }
 
+  @Test
+  public void failRequestWithMissingEntityId() {
+
+    ResponseEntity<String> response =
+        webClient
+            .get()
+            .uri("/user/") // missing id path param
+            .retrieve()
+            .toEntity(String.class)
+            .onErrorResume(WebClientResponseException.class, error -> Mono.just(ResponseEntity.status(error.getStatusCode()).body(error.getResponseBodyAsString())))
+            .block(timeout);
+
+    assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
+    assertThat(response.getBody()).isEqualTo("INVALID_ARGUMENT: Entity key field(s) List(id) must not be empty.");
+  }
+
   @Test
   public void verifyTransformedUserViewWiring() throws InterruptedException {
 

diff --git a/sdk/java-sdk-spring/src/it/java/com/example/wiring/actions/echo/EchoAction.java b/sdk/java-sdk-spring/src/it/java/com/example/wiring/actions/echo/EchoAction.java
@@ -56,6 +56,13 @@ public Effect<Message> stringMessage(@PathVariable("msg_value") String msg) {
     return effects().reply(new Message(response));
   }
 
+  @GetMapping("/echo/int/{int_value}")
+  public Effect<Message> intMessage(@PathVariable("int_value") Integer i) {
+    String response = this.parrot.repeat(i+"");
+    return effects().reply(new Message(response));
+  }
+
+
   @GetMapping("/echo/message")
   public Effect<Message> stringMessageFromParam(@RequestParam String msg) {
     return stringMessage(msg);

diff --git a/sdk/java-sdk-spring/src/main/scala/kalix/javasdk/impl/ComponentDescriptor.scala b/sdk/java-sdk-spring/src/main/scala/kalix/javasdk/impl/ComponentDescriptor.scala
@@ -442,19 +442,24 @@ private[kalix] object ComponentDescriptor {
       fieldNumber: Int,
       paramType: Type,
       fieldNumberOffset: Int): FieldDescriptorProto = {
-    FieldDescriptorProto
+    val builder = FieldDescriptorProto
       .newBuilder()
       .setName(name)
       .setNumber(fieldNumber)
       .setType(mapJavaTypeToProtobuf(paramType))
       .setLabel(mapJavaWrapperToLabel(paramType))
-      .setProto3Optional(true)
-      //setting optional flag is not enough to have the knowledge if the field was set or
-      //indexing starts from 0, so we must subtract the offset
-      //there won't be any gaps, since we are marking all path and query params as optional
-      .setOneofIndex(fieldNumber - fieldNumberOffset)
       .setOptions(addEntityKeyIfNeeded(entityKeys, name))
-      .build()
+
+    if (!entityKeys.contains(name)) {
+      builder
+        .setProto3Optional(true)
+        //setting optional flag is not enough to have the knowledge if the field was set or
+        //indexing starts from 0, so we must subtract the offset
+        //there won't be any gaps, since we are marking all path and query params as optional
+        .setOneofIndex(fieldNumber - fieldNumberOffset - entityKeys.size)
+    }
+
+    builder.build()
   }
 
   private def addEntityKeyIfNeeded(entityKeys: Seq[String], paramName: String): DescriptorProtos.FieldOptions =