fix(mapper): validate join machinery instead of silently mis-resolving

mapper/src/main/java/tw/com/softleader/data/jpa/spec/SpecJoinContext.java

@@ -23,11 +23,14 @@
 import static java.util.Collections.synchronizedMap;
 import static java.util.Optional.ofNullable;
 
+import jakarta.persistence.criteria.Fetch;
 import jakarta.persistence.criteria.Join;
 import jakarta.persistence.criteria.Root;
 import java.lang.annotation.Annotation;
+import java.lang.ref.WeakReference;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.WeakHashMap;
 import lombok.NonNull;
 import org.springframework.lang.Nullable;
 import tw.com.softleader.data.jpa.spec.domain.JoinContext;
@@ -38,8 +41,23 @@
 class SpecJoinContext implements JoinContext {
 
   private final Map<HandleKey, Object> handled = synchronizedMap(new HashMap<>());
-  private final Map<JoinKey, Join<?, ?>> joined = synchronizedMap(new HashMap<>());
-  private final Map<FetchKey, FetchRef> fetched = synchronizedMap(new HashMap<>());
+
+  /*
+   * Join and fetch bookkeeping belongs to the single query execution that created it: a reused
+   * Specification is executed against a brand new Root every time, so entries kept per Root would
+   * pile up for as long as that Specification lives.
+   *
+   * The Root is therefore a weak key, which lets an entry die together with the criteria tree it
+   * describes. The criteria nodes kept as values refer back to their Root, so they are held weakly
+   * as well - a strong value would keep its own key reachable and defeat the weak key entirely
+   * (see the WeakHashMap javadoc). That is safe because a Root owns every join and fetch built
+   * from it, which keeps the referents alive for as long as the execution can still reach them.
+   */
+  private final Map<Root<?>, Map<String, WeakReference<Join<?, ?>>>> joined =
+      synchronizedMap(new WeakHashMap<>());
+
+  private final Map<Root<?>, Map<String, FetchEntry>> fetched =
+      synchronizedMap(new WeakHashMap<>());
 
   @Override
   public boolean hasHandled(
@@ -54,22 +72,34 @@ public void markHandled(@NonNull Object target, @Nullable Object field, @NonNull
 
   @Override
   public void putIfAbsent(@NonNull Root<?> root, @NonNull String alias, @NonNull Join<?, ?> join) {
-    joined.putIfAbsent(new JoinKey(root, alias), join);
+    byAlias(joined, root).putIfAbsent(alias, new WeakReference<>(join));
   }
 
   @Override
   public void putIfAbsent(@NonNull Root<?> root, @NonNull String alias, @NonNull FetchRef ref) {
-    fetched.putIfAbsent(new FetchKey(root, alias), ref);
+    byAlias(fetched, root)
+        .putIfAbsent(alias, new FetchEntry(new WeakReference<>(ref.fetch()), ref.paths()));
   }
 
   @Override
   public Join<?, ?> getJoin(@NonNull Root<?> root, @NonNull String alias) {
-    return joined.get(new JoinKey(root, alias));
+    return ofNullable(joined.get(root))
+        .map(aliases -> aliases.get(alias))
+        .map(WeakReference::get)
+        .orElse(null);
   }
 
   @Override
   public FetchRef getFetch(@NonNull Root<?> root, @NonNull String alias) {
-    return fetched.get(new FetchKey(root, alias));
+    return ofNullable(fetched.get(root))
+        .map(aliases -> aliases.get(alias))
+        .map(FetchEntry::toRef)
+        .orElse(null);
+  }
+
+  private static <V> Map<String, V> byAlias(
+      @NonNull Map<Root<?>, Map<String, V>> byRoot, @NonNull Root<?> root) {
+    return byRoot.computeIfAbsent(root, key -> synchronizedMap(new HashMap<>()));
   }
 
   record HandleKey(@NonNull String target, @Nullable String field, @NonNull Annotation def) {
@@ -82,7 +112,11 @@ static String identityHex(@Nullable Object obj) {
     }
   }
 
-  record JoinKey(@NonNull Root<?> root, @NonNull String alias) {}
+  record FetchEntry(@NonNull WeakReference<Fetch<?, ?>> fetch, @NonNull String[] paths) {
 
-  record FetchKey(@NonNull Root<?> root, @NonNull String alias) {}
+    @Nullable
+    FetchRef toRef() {
+      return ofNullable(fetch.get()).map(f -> new FetchRef(f, paths)).orElse(null);
+    }
+  }
 }

mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/Join.java

@@ -25,6 +25,7 @@
 
 import jakarta.persistence.criteria.CriteriaBuilder;
 import jakarta.persistence.criteria.CriteriaQuery;
+import jakarta.persistence.criteria.From;
 import jakarta.persistence.criteria.JoinType;
 import jakarta.persistence.criteria.Predicate;
 import jakarta.persistence.criteria.Root;
@@ -81,25 +82,33 @@ public Join(
   public Predicate toPredicate(
       @NonNull Root<T> root, @Nullable CriteriaQuery<?> query, @NonNull CriteriaBuilder builder) {
     if (query != null) {
-      query.distinct(distinct);
+      // accumulate rather than overwrite: every join contributes to the query, so a join declared
+      // with distinct=false must not silently undo the distinct=true of an earlier one
+      query.distinct(query.isDistinct() || distinct);
     }
     join(root);
     return null;
   }
 
   private void join(Root<T> root) {
     var jc = context.getAs(CTX_JOIN, JoinContext.class);
-
-    // check if alias already exists, skip creating a new join
-    if (jc.getJoin(root, alias) != null) {
-      return;
-    }
+    var existing = jc.getJoin(root, alias);
 
     if (!pathToJoinOn.contains(".")) {
+      // alias already exists, reuse it as long as it stands for the very same join
+      if (existing != null) {
+        verifyNoConflict(existing, root, pathToJoinOn);
+        return;
+      }
       jc.putIfAbsent(root, alias, root.join(pathToJoinOn, joinType));
       return;
     }
     var byDot = pathToJoinOn.split("\\.");
+    if (byDot.length != 2) {
+      throw new IllegalArgumentException(
+          "Join path: '%s' (alias: '%s') consists of %d segments, but a join path is limited to 2 segments in the form of '<parent-alias>.<association>'! Define an intermediate join for each additional segment and refer to its alias here."
+              .formatted(pathToJoinOn, alias, byDot.length));
+    }
 
     var extractedAlias = byDot[0];
     var joined = jc.getJoin(root, extractedAlias);
@@ -110,6 +119,30 @@ private void join(Root<T> root) {
     }
 
     var extractedPathToJoin = byDot[1];
+    // alias already exists, reuse it as long as it stands for the very same join
+    if (existing != null) {
+      verifyNoConflict(existing, joined, extractedPathToJoin);
+      return;
+    }
     jc.putIfAbsent(root, alias, joined.join(extractedPathToJoin, joinType));
   }
+
+  private void verifyNoConflict(
+      @NonNull jakarta.persistence.criteria.Join<?, ?> existing,
+      @NonNull From<?, ?> parent,
+      @NonNull String attributeName) {
+    if (existing.getParent() == parent
+        && existing.getAttribute().getName().equals(attributeName)
+        && existing.getJoinType() == joinType) {
+      return;
+    }
+    throw new IllegalArgumentException(
+        "Conflicting join definitions share the alias: '%s'! It is already defined as a %s join on the attribute: '%s', so it can not be redefined as a %s join on the path: '%s'. Every join alias must be declared with the same path and joinType."
+            .formatted(
+                alias,
+                existing.getJoinType(),
+                existing.getAttribute().getName(),
+                joinType,
+                pathToJoinOn));
+  }
 }

mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/SimpleSpecification.java

@@ -27,6 +27,7 @@
 import jakarta.persistence.criteria.Path;
 import jakarta.persistence.criteria.Root;
 import java.lang.reflect.InvocationTargetException;
+import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.StringJoiner;
 import lombok.Builder;
@@ -98,28 +99,59 @@ protected <F> Path<F> getPath(@NonNull Root<T> root) {
   }
 
   private Path<?> getExpr(@NonNull Root<T> root, @NonNull String field) {
-    return getJoin(root, field).or(() -> getFetch(root, field)).orElseGet(() -> root.get(field));
+    return getJoin(root, field)
+        .or(() -> getFetch(root, field))
+        .orElseGet(() -> getAttribute(root, field));
   }
 
   @SuppressWarnings({"unchecked"})
   private Optional<Path<T>> getJoin(@NonNull Root<T> root, @NonNull String field) {
-    return ofNullable(context.getAs(CTX_JOIN, JoinContext.class).getJoin(root, field))
-        .map(joined -> (Path<T>) joined);
+    return ofNullable(joinContext().getJoin(root, field)).map(joined -> (Path<T>) joined);
   }
 
   private Optional<Path<T>> getFetch(@NonNull Root<T> root, @NonNull String field) {
-    return ofNullable(context.getAs(CTX_JOIN, JoinContext.class).getFetch(root, field))
-        .map(ref -> getFetchPath(root, ref));
+    return ofNullable(joinContext().getFetch(root, field)).map(ref -> getFetchPath(root, ref));
   }
 
+  @SuppressWarnings({"unchecked"})
   private Path<T> getFetchPath(@NonNull Root<T> root, @NonNull FetchRef ref) {
+    // Resolve through the fetch node itself, since it carries the joinType the fetch was declared
+    // with. Re-navigating the paths from the root would emit an implicit inner join instead, so a
+    // non-INNER @JoinFetch would restrict the content query differently from the count query -
+    // which resolves the very same alias as a real join - and page totals would disagree with the
+    // page content.
+    if (ref.fetch() instanceof Path<?> fetched) {
+      return (Path<T>) fetched;
+    }
     Path<T> current = root;
     for (var path : ref.paths()) {
       current = current.get(path);
     }
     return current;
   }
 
+  private Path<T> getAttribute(@NonNull Root<T> root, @NonNull String field) {
+    try {
+      return root.get(field);
+    } catch (RuntimeException e) {
+      throw new IllegalArgumentException(
+          "Unable to resolve: '%s' of the path: '%s'! It is neither a join alias registered on this query nor an attribute of %s. If '%s' is meant to be a join alias, make sure the @Join or @JoinFetch defining it is declared before this spec - declaration order matters, and a join declared on a null-valued field is never applied."
+              .formatted(field, path, root.getJavaType().getSimpleName(), field),
+          e);
+    }
+  }
+
+  private JoinContext joinContext() {
+    try {
+      return context.getAs(CTX_JOIN, JoinContext.class);
+    } catch (NoSuchElementException e) {
+      throw new IllegalStateException(
+          "No JoinContext registered under the context key: '%s'! Resolving the multi-segment path: '%s' needs the join registry that SpecMapper populates while it builds the specification, so build this spec through SpecMapper.toSpec(..) instead of constructing it directly."
+              .formatted(CTX_JOIN, path),
+          e);
+    }
+  }
+
   @Override
   public String toString() {
     return new StringJoiner(", ", getClass().getSimpleName() + "[", "]")

mapper/src/test/java/tw/com/softleader/data/jpa/spec/JoinFetchSpecificationResolverTest.java

@@ -25,6 +25,7 @@
 import static org.mockito.Mockito.spy;
 
 import jakarta.persistence.EntityManager;
+import jakarta.persistence.criteria.JoinType;
 import java.util.Collection;
 import lombok.AllArgsConstructor;
 import lombok.Builder;
@@ -34,12 +35,14 @@
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.data.domain.PageRequest;
 import tw.com.softleader.data.jpa.spec.annotation.JoinFetch;
 import tw.com.softleader.data.jpa.spec.annotation.JoinFetch.JoinFetches;
 import tw.com.softleader.data.jpa.spec.annotation.Spec;
 import tw.com.softleader.data.jpa.spec.domain.Conjunction;
 import tw.com.softleader.data.jpa.spec.domain.Equals;
 import tw.com.softleader.data.jpa.spec.domain.In;
+import tw.com.softleader.data.jpa.spec.domain.IsNull;
 import tw.com.softleader.data.jpa.spec.domain.Like;
 import tw.com.softleader.data.jpa.spec.usecase.*;
 
@@ -350,6 +353,28 @@ void duplicateAliasJoinFetchOnField() {
     assertThat(root.getFetches()).hasSize(1);
   }
 
+  @DisplayName("非 INNER 的 JoinFetch, count 與 content 應以相同的 join 語意解析 alias")
+  @Test
+  void nonInnerJoinFetchShouldKeepCountAndContentInSync() {
+    repository.save(
+        Customer.builder().name("matt").order(Order.builder().itemName("Pizza").build()).build());
+    // bob 沒有任何 order, 只有 LEFT JOIN 才找得到他
+    var bob = repository.save(Customer.builder().name("bob").build());
+
+    var criteria = LeftJoinFetchOnField.builder().withoutItem(true).build();
+
+    var spec = mapper.toSpec(criteria, Customer.class);
+
+    var content = repository.findAll(spec);
+    assertThat(content).containsExactly(bob);
+    assertThat(repository.count(spec)).isEqualTo(content.size());
+
+    // page size 1 讓 Spring Data 無法省略 count query
+    var page = repository.findAll(spec, PageRequest.of(0, 1));
+    assertThat(page.getTotalElements()).isEqualTo(content.size());
+    assertThat(page.getContent()).containsExactly(bob);
+  }
+
   @JoinFetch(path = "orders")
   @AllArgsConstructor
   @Data
@@ -418,4 +443,13 @@ public static class DuplicateAliasJoinFetchOnField {
     @Spec(path = "order.itemName", value = Like.class)
     String itemName;
   }
+
+  @Builder
+  @Data
+  public static class LeftJoinFetchOnField {
+
+    @JoinFetch(path = "orders", alias = "o", joinType = JoinType.LEFT)
+    @Spec(path = "o.itemName", value = IsNull.class)
+    Boolean withoutItem;
+  }
 }