fix: validate compound ids order in the path

sdk/java-sdk-spring/src/main/scala/kalix/javasdk/impl/Validations.scala

@@ -27,6 +27,7 @@ import kalix.javasdk.annotations.Publish
 import kalix.javasdk.annotations.Query
 import kalix.javasdk.annotations.Subscribe
 import kalix.javasdk.annotations.Table
+import kalix.javasdk.eventsourcedentity.EventSourcedEntity
 import kalix.javasdk.impl.ComponentDescriptorFactory.eventSourcedEntitySubscription
 import kalix.javasdk.impl.ComponentDescriptorFactory.findEventSourcedEntityClass
 import kalix.javasdk.impl.ComponentDescriptorFactory.findEventSourcedEntityType
@@ -49,9 +50,13 @@ import kalix.javasdk.impl.ComponentDescriptorFactory.hasValueEntitySubscription
 import kalix.javasdk.impl.ComponentDescriptorFactory.streamSubscription
 import kalix.javasdk.impl.ComponentDescriptorFactory.topicSubscription
 import kalix.javasdk.impl.Reflect.Syntax._
+import kalix.javasdk.impl.reflection.IdExtractor
 import kalix.javasdk.impl.reflection.ReflectionUtils
+import kalix.javasdk.impl.reflection.RestServiceIntrospector
 import kalix.javasdk.impl.reflection.ServiceMethod
+import kalix.javasdk.valueentity.ValueEntity
 import kalix.javasdk.view.View
+import kalix.javasdk.workflow.Workflow
 import kalix.spring.impl.KalixSpringApplication
 
 // TODO: abstract away spring and reactor dependencies
@@ -138,7 +143,59 @@ object Validations {
 
   def validate(component: Class[_]): Validation =
     validateAction(component) ++
-    validateView(component)
+    validateView(component) ++
+    validateValueEntity(component) ++
+    validateEventSourcedEntity(component) ++
+    validateWorkflow(component)
+
+  private def validateCompoundIdsOrder(component: Class[_]): Validation = {
+    val restService = RestServiceIntrospector.inspectService(component)
+    component.getMethods.toIndexedSeq
+      .filter(hasRestAnnotation)
+      .map { method =>
+        val ids = IdExtractor.extractIds(component, method)
+        if (ids.size == 1) {
+          Valid
+        } else {
+          restService.methods.find(_.javaMethod.getName == method.getName) match {
+            case Some(requestServiceMethod) =>
+              val idsFromPath = requestServiceMethod.parsedPath.fields.filter(ids.contains)
+              val diff = ids.diff(idsFromPath)
+              if (diff.nonEmpty) {
+                Validation(
+                  s"All ids [${ids.mkString(", ")}] should be used in the path '${requestServiceMethod.parsedPath.path}'. Missing ids [${diff
+                    .mkString(", ")}].")
+              } else if (ids != idsFromPath) { //check if the order is the same
+                Validation(
+                  s"Ids in the path '${requestServiceMethod.parsedPath.path}' are in a different order than specified in the @Id annotation [${ids
+                    .mkString(", ")}]. This could lead to unexpected bugs when calling the component.")
+              } else {
+                Valid
+              }
+            case None => throw new IllegalStateException(s"Method [${method.getName}] not found in the RestService")
+          }
+        }
+      }
+      .foldLeft(Valid: Validation)(_ ++ _)
+  }
+
+  private def validateValueEntity(component: Class[_]): Validation = {
+    when[ValueEntity[_]](component) {
+      validateCompoundIdsOrder(component)
+    }
+  }
+
+  private def validateEventSourcedEntity(component: Class[_]): Validation = {
+    when[EventSourcedEntity[_, _]](component) {
+      validateCompoundIdsOrder(component)
+    }
+  }
+
+  private def validateWorkflow(component: Class[_]): Validation = {
+    when[Workflow[_]](component) {
+      validateCompoundIdsOrder(component)
+    }
+  }
 
   private def validateAction(component: Class[_]): Validation = {
     when[Action](component) {

sdk/java-sdk-spring/src/test/java/kalix/spring/testmodels/eventsourcedentity/EventSourcedEntitiesTestModels.java

@@ -108,6 +108,15 @@ public Integer getInteger(@PathVariable Integer number) {
     }
   }
 
+  @TypeId("counter")
+  public static class ESEntityCompoundIdIncorrectOrder extends EventSourcedEntity<Integer, Object> {
+    @Id({"id", "id2"})
+    @GetMapping("/{id2}/eventsourced/{id}/int/{number}")
+    public Integer getInteger(@PathVariable Integer number) {
+      return number;
+    }
+  }
+
   @Id("id")
   @TypeId("counter")
   public static class CounterEventSourcedEntityWithIdMethodOverride extends EventSourcedEntity<Integer, Object> {

sdk/java-sdk-spring/src/test/java/kalix/spring/testmodels/valueentity/ValueEntitiesTestModels.java

@@ -41,6 +41,36 @@ public ValueEntity.Effect<Done> createEntity(@RequestBody CreateUser createUser)
     }
   }
 
+  @Id({"userId", "cartId"})
+  @TypeId("user")
+  @RequestMapping("/user/{cartId}/{userId}")
+  public static class PostWithIdsIncorrectOrder extends ValueEntity<User> {
+    @Override
+    public User emptyState() {
+      return null;
+    }
+
+    @PostMapping("/create")
+    public ValueEntity.Effect<Done> createEntity(@RequestBody CreateUser createUser) {
+      return effects().reply(Done.instance);
+    }
+  }
+
+  @Id({"userId", "cartId"})
+  @TypeId("user")
+  @RequestMapping("/user/{cartId}")
+  public static class PostWithIdsMissingParams extends ValueEntity<User> {
+    @Override
+    public User emptyState() {
+      return null;
+    }
+
+    @PostMapping("/create")
+    public ValueEntity.Effect<Done> createEntity(@RequestBody CreateUser createUser) {
+      return effects().reply(Done.instance);
+    }
+  }
+
   @Id({"userId", "cartId"})
   @TypeId("user")
   @RequestMapping()
@@ -55,8 +85,8 @@ public ValueEntity.Effect<Done> createEntity(@RequestBody CreateUser createUser)
       return effects().reply(Done.instance);
     }
 
-    @PostMapping("/user/{cartId}/create/{otherParam}")
-    public ValueEntity.Effect<Done> createEntity2(@RequestParam String someParam, @RequestParam String userId,
+    @PostMapping("/user/{userId}/{cartId}/create/{otherParam}")
+    public ValueEntity.Effect<Done> createEntity2(@RequestParam String someParam,
                                                   @PathVariable Integer otherParam, @PathVariable String cartId, @RequestBody CreateUser createUser) {
       return effects().reply(Done.instance);
     }
@@ -76,7 +106,7 @@ public static class ValueEntityWithServiceLevelAcl extends ValueEntity<User> {
   @Id( {"userId", "cartId"})
   @TypeId("user")
   public static class ValueEntityWithMethodLevelAcl extends ValueEntity<User> {
-    @PostMapping("/create")
+    @PostMapping("/{userId}/{cartId}/create")
     @Acl(allow = @Acl.Matcher(service = "test"))
     public ValueEntity.Effect<Done> createEntity(@RequestBody CreateUser createUser) {
       return effects().reply(Done.instance);
@@ -89,7 +119,7 @@ public ValueEntity.Effect<Done> createEntity(@RequestBody CreateUser createUser)
     validate = JWT.JwtMethodMode.BEARER_TOKEN,
     bearerTokenIssuer = {"a", "b"})
   public static class ValueEntityWithServiceLevelJwt extends ValueEntity<User> {
-    @PostMapping("/create")
+    @PostMapping("/{userId}/{cartId}/create")
     public ValueEntity.Effect<Done> createEntity(@RequestBody CreateUser createUser) {
       return effects().reply(Done.instance);
     }
@@ -98,7 +128,7 @@ public ValueEntity.Effect<Done> createEntity(@RequestBody CreateUser createUser)
   @Id( {"userId", "cartId"})
   @TypeId("user")
   public static class ValueEntityWithMethodLevelJwt extends ValueEntity<User> {
-    @PostMapping("/create")
+    @PostMapping("/{userId}/{cartId}/create")
     @JWT(
       validate = JWT.JwtMethodMode.BEARER_TOKEN,
       bearerTokenIssuer = {"a", "b"})

sdk/java-sdk-spring/src/test/scala/kalix/javasdk/impl/EventSourcedEntityDescriptorFactorySpec.scala

@@ -27,6 +27,7 @@ import kalix.spring.testmodels.eventsourcedentity.EventSourcedEntitiesTestModels
 import kalix.spring.testmodels.eventsourcedentity.EventSourcedEntitiesTestModels.CounterEventSourcedEntityWithIdOnMethod
 import kalix.spring.testmodels.eventsourcedentity.EventSourcedEntitiesTestModels.CounterEventSourcedEntityWithMethodLevelJWT
 import kalix.spring.testmodels.eventsourcedentity.EventSourcedEntitiesTestModels.CounterEventSourcedEntityWithServiceLevelJWT
+import kalix.spring.testmodels.eventsourcedentity.EventSourcedEntitiesTestModels.ESEntityCompoundIdIncorrectOrder
 import kalix.spring.testmodels.eventsourcedentity.EventSourcedEntitiesTestModels.EventSourcedEntityWithMethodLevelAcl
 import kalix.spring.testmodels.eventsourcedentity.EventSourcedEntitiesTestModels.EventSourcedEntityWithServiceLevelAcl
 import kalix.spring.testmodels.eventsourcedentity.EventSourcedEntitiesTestModels.IllDefinedEntityWithIdGeneratorAndId
@@ -139,6 +140,14 @@ class EventSourcedEntityDescriptorFactorySpec extends AnyWordSpec with Component
         service shouldBe "test"
       }
     }
+
+    "not allow different order of entity ids in the path" in {
+      // it should be annotated either on type or on method level
+      intercept[InvalidComponentException] {
+        Validations.validate(classOf[ESEntityCompoundIdIncorrectOrder]).failIfInvalid
+      }.getMessage should include(
+        "Ids in the path '/{id2}/eventsourced/{id}/int/{number}' are in a different order than specified in the @Id annotation [id, id2]. This could lead to unexpected bugs when calling the component.")
+    }
   }
 
 }

sdk/java-sdk-spring/src/test/scala/kalix/javasdk/impl/ValueEntityDescriptorFactorySpec.scala

@@ -24,6 +24,8 @@ import kalix.KeyGeneratorMethodOptions
 import kalix.spring.testmodels.valueentity.Counter
 import kalix.spring.testmodels.valueentity.ValueEntitiesTestModels.GetWithQueryParams
 import kalix.spring.testmodels.valueentity.ValueEntitiesTestModels.PostWithIds
+import kalix.spring.testmodels.valueentity.ValueEntitiesTestModels.PostWithIdsIncorrectOrder
+import kalix.spring.testmodels.valueentity.ValueEntitiesTestModels.PostWithIdsMissingParams
 import kalix.spring.testmodels.valueentity.ValueEntitiesTestModels.ValueEntityWithMethodLevelAcl
 import kalix.spring.testmodels.valueentity.ValueEntitiesTestModels.ValueEntityWithMethodLevelJwt
 import kalix.spring.testmodels.valueentity.ValueEntitiesTestModels.ValueEntityWithServiceLevelAcl
@@ -69,10 +71,10 @@ class ValueEntityDescriptorFactorySpec extends AnyWordSpec with ComponentDescrip
         val createMethod = desc.commandHandlers("CreateEntity2")
 
         assertRequestFieldNumberAndJavaType(createMethod, "json_body", 1, JavaType.MESSAGE)
-        assertRequestFieldNumberAndJavaType(createMethod, "cartId", 2, JavaType.STRING)
-        assertRequestFieldNumberAndJavaType(createMethod, "otherParam", 3, JavaType.INT)
-        assertRequestFieldNumberAndJavaType(createMethod, "someParam", 4, JavaType.STRING)
-        assertRequestFieldNumberAndJavaType(createMethod, "userId", 5, JavaType.STRING)
+        assertRequestFieldNumberAndJavaType(createMethod, "userId", 2, JavaType.STRING)
+        assertRequestFieldNumberAndJavaType(createMethod, "cartId", 3, JavaType.STRING)
+        assertRequestFieldNumberAndJavaType(createMethod, "otherParam", 4, JavaType.INT)
+        assertRequestFieldNumberAndJavaType(createMethod, "someParam", 5, JavaType.STRING)
       }
     }
 
@@ -110,6 +112,22 @@ class ValueEntityDescriptorFactorySpec extends AnyWordSpec with ComponentDescrip
         jwtOption.getValidate(0) shouldBe JwtMethodMode.BEARER_TOKEN
       }
     }
+
+    "not allow different order of entity ids in the path" in {
+      // it should be annotated either on type or on method level
+      intercept[InvalidComponentException] {
+        Validations.validate(classOf[PostWithIdsIncorrectOrder]).failIfInvalid
+      }.getMessage should include(
+        "Ids in the path '/user/{cartId}/{userId}/create' are in a different order than specified in the @Id annotation [userId, cartId]. This could lead to unexpected bugs when calling the component.")
+    }
+
+    "not allow missing ids in the path" in {
+      // it should be annotated either on type or on method level
+      intercept[InvalidComponentException] {
+        Validations.validate(classOf[PostWithIdsMissingParams]).failIfInvalid
+      }.getMessage should include(
+        "All ids [userId, cartId] should be used in the path '/user/{cartId}/create'. Missing ids [userId].")
+    }
   }
 
 }