Enhance aggregate strategies with alias validation

nakamura-to · nakamura-to · commit a620ef0d3423 · 2025-02-08T09:04:42.000+09:00
diff --git a/doma-core/src/main/java/org/seasar/doma/jdbc/aggregate/AbstractAggregateStrategyType.java b/doma-core/src/main/java/org/seasar/doma/jdbc/aggregate/AbstractAggregateStrategyType.java
@@ -21,20 +21,27 @@
 
 public abstract class AbstractAggregateStrategyType implements AggregateStrategyType {
 
+  private final String name;
   private final EntityType<?> root;
   private final String tableAlias;
   private final List<AssociationLinkerType<?, ?>> associationLinkerTypes;
 
   protected AbstractAggregateStrategyType(
+      String name,
       EntityType<?> root,
       String tableAlias,
       List<AssociationLinkerType<?, ?>> associationLinkerTypes) {
-    Objects.requireNonNull(tableAlias);
+    this.name = Objects.requireNonNull(name);
     this.root = Objects.requireNonNull(root);
     this.tableAlias = Objects.requireNonNull(tableAlias);
     this.associationLinkerTypes = Objects.requireNonNull(associationLinkerTypes);
   }
 
+  @Override
+  public String getName() {
+    return name;
+  }
+
   @Override
   public EntityType<?> getRoot() {
     return root;
diff --git a/doma-core/src/main/java/org/seasar/doma/jdbc/aggregate/AggregateStrategyType.java b/doma-core/src/main/java/org/seasar/doma/jdbc/aggregate/AggregateStrategyType.java
@@ -15,6 +15,7 @@
  */
 package org.seasar.doma.jdbc.aggregate;
 
+import java.util.ArrayList;
 import java.util.List;
 import org.seasar.doma.jdbc.entity.EntityType;
 
@@ -23,9 +24,21 @@
  * its associated entities.
  */
 public interface AggregateStrategyType {
+
+  String getName();
+
   EntityType<?> getRoot();
 
   String getTableAlias();
 
   List<AssociationLinkerType<?, ?>> getAssociationLinkerTypes();
+
+  default List<String> getTableAliases() {
+    List<String> result = new ArrayList<>();
+    result.add(getTableAlias());
+    for (AssociationLinkerType<?, ?> associationLinkerType : getAssociationLinkerTypes()) {
+      result.add(associationLinkerType.getTableAlias());
+    }
+    return result;
+  }
 }
diff --git a/doma-core/src/main/java/org/seasar/doma/jdbc/query/AbstractSelectQuery.java b/doma-core/src/main/java/org/seasar/doma/jdbc/query/AbstractSelectQuery.java
@@ -19,13 +19,15 @@
 
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import org.seasar.doma.DomaException;
 import org.seasar.doma.FetchType;
 import org.seasar.doma.internal.expr.ExpressionEvaluator;
 import org.seasar.doma.internal.expr.Value;
@@ -45,7 +47,6 @@
 import org.seasar.doma.jdbc.aggregate.AggregateStrategyType;
 import org.seasar.doma.jdbc.command.SelectCommand;
 import org.seasar.doma.jdbc.dialect.Dialect;
-import org.seasar.doma.jdbc.entity.EntityPropertyType;
 import org.seasar.doma.jdbc.entity.EntityType;
 import org.seasar.doma.message.Message;
 import org.seasar.doma.wrapper.PrimitiveLongWrapper;
@@ -138,34 +139,38 @@ protected List<String> expandAggregateColumns(ExpandNode node, String aliasCsv)
     if (aggregateStrategyType == null) {
       return List.of();
     }
-    Set<String> aliases =
+
+    Set<String> definedAliases = new HashSet<>(aggregateStrategyType.getTableAliases());
+    Set<String> requiredAliases =
         Arrays.stream(aliasCsv.split(","))
             .map(String::trim)
             .filter(s -> !s.isEmpty())
+            .peek(
+                s -> {
+                  if (!definedAliases.contains(s)) {
+                    throw new DomaException(Message.DOMA2239, s, aggregateStrategyType.getName());
+                  }
+                })
             .collect(Collectors.toSet());
-    Stream<Pair<String, EntityPropertyType<?, ?>>> rootColumns =
-        entityType.getEntityPropertyTypes().stream()
-            .map(
-                p -> {
-                  String tableAlias = aggregateStrategyType.getTableAlias();
-                  return new Pair<>(tableAlias, p);
-                });
-    Stream<Pair<String, EntityPropertyType<?, ?>>> associationColumns =
-        aggregateStrategyType.getAssociationLinkerTypes().stream()
-            .flatMap(
-                linkerType -> {
-                  String tableAlias = linkerType.getTableAlias();
-                  return linkerType.getTarget().getEntityPropertyTypes().stream()
-                      .map(p -> new Pair<>(tableAlias, p));
-                });
+
     Naming naming = config.getNaming();
     Dialect dialect = config.getDialect();
-    return Stream.concat(rootColumns, associationColumns)
-        .filter(p -> aliases.isEmpty() || aliases.contains(p.fst))
+
+    Stream<Pair<String, EntityType<?>>> rootStream =
+        Stream.of(
+            new Pair<>(aggregateStrategyType.getTableAlias(), aggregateStrategyType.getRoot()));
+    Stream<Pair<String, EntityType<?>>> associationStream =
+        aggregateStrategyType.getAssociationLinkerTypes().stream()
+            .map(it -> new Pair<>(it.getTableAlias(), it.getTarget()));
+
+    return Stream.concat(rootStream, associationStream)
+        .filter(pair -> requiredAliases.isEmpty() || requiredAliases.contains(pair.fst))
+        .flatMap(
+            pair -> pair.snd.getEntityPropertyTypes().stream().map(it -> new Pair<>(pair.fst, it)))
         .map(
-            p -> {
-              String tableAlias = p.fst;
-              String columnName = p.snd.getColumnName(naming::apply, dialect::applyQuote);
+            pair -> {
+              String tableAlias = pair.fst;
+              String columnName = pair.snd.getColumnName(naming::apply, dialect::applyQuote);
               return String.format("%1$s.%2$s as %1$s_%2$s", tableAlias, columnName);
             })
         .toList();
diff --git a/doma-core/src/main/java/org/seasar/doma/message/Message.java b/doma-core/src/main/java/org/seasar/doma/message/Message.java
@@ -284,6 +284,8 @@ public enum Message implements MessageResource {
       {4}
       PATH=[{2}]
       SQL=[{3}]"""),
+  DOMA2239(
+      "The table alias \"{0}\" passed to the expansion directive is not defined in the aggregate strategy \"{1}\"."),
 
   // expression
   DOMA3001(
diff --git a/doma-processor/src/main/java/org/seasar/doma/internal/apt/generator/AggregateStrategyTypeGenerator.java b/doma-processor/src/main/java/org/seasar/doma/internal/apt/generator/AggregateStrategyTypeGenerator.java
@@ -78,8 +78,10 @@ private void printConstructor() {
     iprint("private %1$s() {%n", simpleName);
     indent();
     iprint(
-        "super(%1$s, \"%2$s\", java.util.List.of(%n",
-        strategyMeta.root().getTypeCode(), strategyMeta.tableAlias());
+        "super(\"%1$s\", %2$s, \"%3$s\", java.util.List.of(%n",
+        /* 1 */ strategyMeta.typeElement().getQualifiedName(),
+        /* 2 */ strategyMeta.root().getTypeCode(),
+        /* 3 */ strategyMeta.tableAlias());
     indent();
     Iterator<AssociationLinkerMeta> iter = strategyMeta.associationLinkerMetas().iterator();
     while (iter.hasNext()) {
diff --git a/doma-processor/src/main/java/org/seasar/doma/internal/apt/meta/aggregate/AggregateStrategyMeta.java b/doma-processor/src/main/java/org/seasar/doma/internal/apt/meta/aggregate/AggregateStrategyMeta.java
@@ -17,11 +17,15 @@
 
 import java.util.List;
 import java.util.Objects;
+import javax.lang.model.element.TypeElement;
 import org.seasar.doma.internal.apt.cttype.EntityCtType;
 import org.seasar.doma.internal.apt.meta.TypeElementMeta;
 
 public record AggregateStrategyMeta(
-    EntityCtType root, String tableAlias, List<AssociationLinkerMeta> associationLinkerMetas)
+    TypeElement typeElement,
+    EntityCtType root,
+    String tableAlias,
+    List<AssociationLinkerMeta> associationLinkerMetas)
     implements TypeElementMeta {
 
   public AggregateStrategyMeta {
diff --git a/doma-processor/src/main/java/org/seasar/doma/internal/apt/meta/aggregate/AggregateStrategyMetaFactory.java b/doma-processor/src/main/java/org/seasar/doma/internal/apt/meta/aggregate/AggregateStrategyMetaFactory.java
@@ -82,7 +82,7 @@ public AggregateStrategyMeta createTypeElementMeta(TypeElement typeElement) {
     validateAllTableAliases(aggregateStrategyAnnot.getTableAliasValue(), associationLinkerMetas);
     validateNavigation(root, associationLinkerMetas);
     return new AggregateStrategyMeta(
-        root, aggregateStrategyAnnot.getTableAliasValue(), associationLinkerMetas);
+        typeElement, root, aggregateStrategyAnnot.getTableAliasValue(), associationLinkerMetas);
   }
 
   private void validateAllPropertyPaths(List<AssociationLinkerMeta> associationLinkerMetas) {
diff --git a/doma-processor/src/test/resources/org/seasar/doma/internal/apt/processor/aggregate/AggregateStrategyProcessorTest_ChildEntityStrategy.txt b/doma-processor/src/test/resources/org/seasar/doma/internal/apt/processor/aggregate/AggregateStrategyProcessorTest_ChildEntityStrategy.txt
@@ -11,7 +11,7 @@ public final class _ChildEntityStrategy extends org.seasar.doma.jdbc.aggregate.A
     }
 
     private _ChildEntityStrategy() {
-        super(org.seasar.doma.internal.apt.processor.aggregate._ChildEntity.getSingletonInternal(), "c", java.util.List.of(
+        super("org.seasar.doma.internal.apt.processor.aggregate.ChildEntityStrategy", org.seasar.doma.internal.apt.processor.aggregate._ChildEntity.getSingletonInternal(), "c", java.util.List.of(
             org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "dept", 1, "d", org.seasar.doma.internal.apt.processor.aggregate._ChildEntity.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.ChildEntityStrategy.dept),
             org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "emp", 1, "e", org.seasar.doma.internal.apt.processor.aggregate._ChildEntity.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.ChildEntityStrategy.emp)
         ));
diff --git a/doma-processor/src/test/resources/org/seasar/doma/internal/apt/processor/aggregate/AggregateStrategyProcessorTest_DeptStrategy.txt b/doma-processor/src/test/resources/org/seasar/doma/internal/apt/processor/aggregate/AggregateStrategyProcessorTest_DeptStrategy.txt
@@ -11,7 +11,7 @@ public final class _DeptStrategy extends org.seasar.doma.jdbc.aggregate.Abstract
     }
 
     private _DeptStrategy() {
-        super(org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), "d", java.util.List.of(
+        super("org.seasar.doma.internal.apt.processor.aggregate.DeptStrategy", org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), "d", java.util.List.of(
             org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("employees", "employees.address", 2, "a", org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Address.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.DeptStrategy.address),
             org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "employees", 1, "e", org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.DeptStrategy.dept)
         ));
diff --git a/doma-processor/src/test/resources/org/seasar/doma/internal/apt/processor/aggregate/AggregateStrategyProcessorTest_EmpStrategy.txt b/doma-processor/src/test/resources/org/seasar/doma/internal/apt/processor/aggregate/AggregateStrategyProcessorTest_EmpStrategy.txt
@@ -11,7 +11,7 @@ public final class _EmpStrategy extends org.seasar.doma.jdbc.aggregate.AbstractA
     }
 
     private _EmpStrategy() {
-        super(org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), "e", java.util.List.of(
+        super("org.seasar.doma.internal.apt.processor.aggregate.EmpStrategy", org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), "e", java.util.List.of(
             org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "dept", 1, "d", org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.EmpStrategy.dept),
             org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "address", 1, "a", org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Address.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.EmpStrategy.address)
         ));
diff --git a/integration-test-java/src/main/java/org/seasar/doma/it/dao/DepartmentDao.java b/integration-test-java/src/main/java/org/seasar/doma/it/dao/DepartmentDao.java
@@ -116,6 +116,9 @@ int updateBySqlFile_nonEntity(
   @Select(aggregateStrategy = DepartmentStrategy.class)
   Department selectByIdAsAggregate(Integer departmentId);
 
+  @Select(aggregateStrategy = DepartmentStrategy.class)
+  Department illegalAlias(Integer departmentId);
+
   @Select(aggregateStrategy = DepartmentStrategy.class)
   List<Department> selectByIdsAsAggregate(List<Integer> departmentIds);
 
diff --git a/integration-test-java/src/main/resources/META-INF/org/seasar/doma/it/dao/DepartmentDao/illegalAlias.sql b/integration-test-java/src/main/resources/META-INF/org/seasar/doma/it/dao/DepartmentDao/illegalAlias.sql
@@ -0,0 +1,13 @@
+select
+    /*%expand "d, unknown" */*,
+    a.address_id as a_address_id,
+    a.street as a_street,
+    a.version as a_version
+from
+    DEPARTMENT d
+left outer join EMPLOYEE e
+    on d.department_id = e.department_id
+left outer join ADDRESS a
+    on e.address_id = a.address_id
+where
+    d.department_id = /*departmentId*/0
diff --git a/integration-test-java/src/test/java/org/seasar/doma/it/aggregate/AggregateTest.java b/integration-test-java/src/test/java/org/seasar/doma/it/aggregate/AggregateTest.java
@@ -20,6 +20,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.math.BigDecimal;
@@ -33,6 +34,7 @@
 import java.util.stream.Collectors;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.seasar.doma.DomaException;
 import org.seasar.doma.it.IntegrationTestEnvironment;
 import org.seasar.doma.it.criteria.Street;
 import org.seasar.doma.it.dao.DepartmentDao;
@@ -49,6 +51,7 @@
 import org.seasar.doma.it.entity.ImmutableDept;
 import org.seasar.doma.it.entity.ImmutableEmp;
 import org.seasar.doma.jdbc.Config;
+import org.seasar.doma.message.Message;
 
 @ExtendWith(IntegrationTestEnvironment.class)
 public class AggregateTest {
@@ -401,4 +404,17 @@ public void testImmutable_selfJoin(Config config) {
     assertNull(king.manager());
     assertEquals(king.employeeName(), blake.manager().employeeName());
   }
+
+  @Test
+  public void illegalAlias(Config config) {
+    DepartmentDao dao = new DepartmentDaoImpl(config);
+    DomaException ex =
+        assertThrows(
+            DomaException.class,
+            () -> {
+              dao.illegalAlias(1);
+            });
+    assertEquals(Message.DOMA2239, ex.getMessageResource());
+    System.out.println(ex.getMessage());
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ public AggregateStrategyMeta createTypeElementMeta(TypeElement typeElement) {`
`82`	`82`	`validateAllTableAliases(aggregateStrategyAnnot.getTableAliasValue(), associationLinkerMetas);`
`83`	`83`	`validateNavigation(root, associationLinkerMetas);`
`84`	`84`	`return new AggregateStrategyMeta(`
`85`		`- root, aggregateStrategyAnnot.getTableAliasValue(), associationLinkerMetas);`
	`85`	`+ typeElement, root, aggregateStrategyAnnot.getTableAliasValue(), associationLinkerMetas);`
`86`	`86`	`}`
`87`	`87`
`88`	`88`	`private void validateAllPropertyPaths(List<AssociationLinkerMeta> associationLinkerMetas) {`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ public final class _ChildEntityStrategy extends org.seasar.doma.jdbc.aggregate.A`
`11`	`11`	`}`
`12`	`12`
`13`	`13`	`private _ChildEntityStrategy() {`
`14`		`- super(org.seasar.doma.internal.apt.processor.aggregate._ChildEntity.getSingletonInternal(), "c", java.util.List.of(`
	`14`	`+ super("org.seasar.doma.internal.apt.processor.aggregate.ChildEntityStrategy", org.seasar.doma.internal.apt.processor.aggregate._ChildEntity.getSingletonInternal(), "c", java.util.List.of(`
`15`	`15`	`org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "dept", 1, "d", org.seasar.doma.internal.apt.processor.aggregate._ChildEntity.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.ChildEntityStrategy.dept),`
`16`	`16`	`org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "emp", 1, "e", org.seasar.doma.internal.apt.processor.aggregate._ChildEntity.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.ChildEntityStrategy.emp)`
`17`	`17`	`));`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ public final class _DeptStrategy extends org.seasar.doma.jdbc.aggregate.Abstract`
`11`	`11`	`}`
`12`	`12`
`13`	`13`	`private _DeptStrategy() {`
`14`		`- super(org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), "d", java.util.List.of(`
	`14`	`+ super("org.seasar.doma.internal.apt.processor.aggregate.DeptStrategy", org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), "d", java.util.List.of(`
`15`	`15`	`org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("employees", "employees.address", 2, "a", org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Address.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.DeptStrategy.address),`
`16`	`16`	`org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "employees", 1, "e", org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.DeptStrategy.dept)`
`17`	`17`	`));`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ public final class _EmpStrategy extends org.seasar.doma.jdbc.aggregate.AbstractA`
`11`	`11`	`}`
`12`	`12`
`13`	`13`	`private _EmpStrategy() {`
`14`		`- super(org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), "e", java.util.List.of(`
	`14`	`+ super("org.seasar.doma.internal.apt.processor.aggregate.EmpStrategy", org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), "e", java.util.List.of(`
`15`	`15`	`org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "dept", 1, "d", org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Dept.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.EmpStrategy.dept),`
`16`	`16`	`org.seasar.doma.jdbc.aggregate.AssociationLinkerType.of("", "address", 1, "a", org.seasar.doma.internal.apt.processor.aggregate._Emp.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate._Address.getSingletonInternal(), org.seasar.doma.internal.apt.processor.aggregate.EmpStrategy.address)`
`17`	`17`	`));`