Qute: type pollution fixes

- turn ResultNode in an abstract class - restrict the support of CompletionStage by default; i.e. only consider CompletableFuture and CompletedStage - in some cases intentionally use "instanceof" to test interfaces as the last resort in order to mitigate the type pollution - also make Qute's version of LazyValue Loom-friendly
mkouba · Sep 14, 2023 · fcc0050 · fcc0050
1 parent 1213006
commit fcc0050
Show file tree

Hide file tree

Showing 11 changed files with 97 additions and 67 deletions.
diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/CompletionStageSupport.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/CompletionStageSupport.java
@@ -0,0 +1,33 @@
+package io.quarkus.qute;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionStage;
+
+import io.smallrye.mutiny.operators.AbstractUni;
+
+class CompletionStageSupport {
+
+    static final String SYSTEM_PROPERTY = "quarkus.qute.unrestricted-completion-stage-support";
+
+    /**
+     * {@code true} if any {@link CompletionStage} implementation is supported. {@code false} if only {@link CompletableFuture}
+     * and {@link CompletedStage} are considered in the API.
+     */
+    static final boolean UNRESTRICTED = Boolean.getBoolean(SYSTEM_PROPERTY);
+
+    @SuppressWarnings("unchecked")
+    static CompletionStage<Object> toCompletionStage(Object result) {
+        // Note that we intentionally use "instanceof" to test interfaces as the last resort in order to mitigate the "type pollution"
+        // See https://github.com/RedHatPerf/type-pollution-agent for more information
+        if (result instanceof CompletableFuture) {
+            return (CompletableFuture<Object>) result;
+        } else if (result instanceof CompletedStage) {
+            return (CompletedStage<Object>) result;
+        } else if (result instanceof AbstractUni) {
+            return ((AbstractUni<Object>) result).subscribeAsCompletionStage();
+        } else if (UNRESTRICTED && result instanceof CompletionStage) {
+            return (CompletionStage<Object>) result;
+        }
+        return CompletedStage.of(result);
+    }
+}
diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/EvaluatorImpl.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/EvaluatorImpl.java
@@ -8,15 +8,13 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
 
 import org.jboss.logging.Logger;
 
 import io.quarkus.qute.Expression.Part;
 import io.quarkus.qute.ExpressionImpl.PartImpl;
 import io.quarkus.qute.Results.NotFound;
-import io.smallrye.mutiny.operators.AbstractUni;
 
 class EvaluatorImpl implements Evaluator {
 
@@ -78,7 +76,7 @@ public CompletionStage<Object> evaluate(Expression expression, ResolutionContext
                 // Very often a single matching resolver will be found
                 return matching[0].resolve(context).thenCompose(r -> (parts.size() > 1)
                         ? resolveReference(false, r, parts, resolutionContext, expression, 1)
-                        : toCompletionStage(r));
+                        : CompletionStageSupport.toCompletionStage(r));
             } else {
                 // Multiple namespace resolvers match
                 return resolveNamespace(context, resolutionContext, parts, matching, 0, expression);
@@ -113,7 +111,7 @@ private CompletionStage<Object> resolveNamespace(EvalContext context, Resolution
             } else if (parts.size() > 1) {
                 return resolveReference(false, r, parts, resolutionContext, expression, 1);
             } else {
-                return toCompletionStage(r);
+                return CompletionStageSupport.toCompletionStage(r);
             }
         });
     }
@@ -144,7 +142,7 @@ private CompletionStage<Object> resolve(EvalContextImpl evalContext, Iterator<Va
                     if (Results.isNotFound(r)) {
                         return resolve(evalContext, null, false, expression, isLastPart, partIndex);
                     } else {
-                        return toCompletionStage(r);
+                        return CompletionStageSupport.toCompletionStage(r);
                     }
                 });
             }
@@ -212,27 +210,11 @@ private CompletionStage<Object> resolve(EvalContextImpl evalContext, Iterator<Va
             } else {
                 // Cache the first resolver where a result is found
                 evalContext.setCachedResolver(foundResolver);
-                return toCompletionStage(r);
+                return CompletionStageSupport.toCompletionStage(r);
             }
         });
     }
 
-    @SuppressWarnings("unchecked")
-    private static CompletionStage<Object> toCompletionStage(Object result) {
-        // Note that we intentionally avoid "result instanceof Uni"; see https://github.com/RedHatPerf/type-pollution-agent
-        // Unfortunatelly, we can't get rid of "result instanceof CompletionStage" so we at least try to test CompletableFuture and CompletedStage first
-        if (result instanceof CompletableFuture) {
-            return (CompletableFuture<Object>) result;
-        } else if (result instanceof CompletedStage) {
-            return (CompletedStage<Object>) result;
-        } else if (result instanceof AbstractUni) {
-            return ((AbstractUni<Object>) result).subscribeAsCompletionStage();
-        } else if (result instanceof CompletionStage) {
-            return (CompletionStage<Object>) result;
-        }
-        return CompletedStage.of(result);
-    }
-
     private TemplateException propertyNotFound(Object result, Expression expression) {
         String propertyMessage;
         if (result instanceof NotFound) {

diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/ExpressionNode.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/ExpressionNode.java
@@ -2,47 +2,52 @@
 
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
-import java.util.function.Function;
 
 import org.jboss.logging.Logger;
 
 /**
  * This node holds a single expression such as {@code foo.bar}.
  */
-class ExpressionNode implements TemplateNode, Function<Object, CompletionStage<ResultNode>> {
+class ExpressionNode implements TemplateNode {
 
     private static final Logger LOG = Logger.getLogger("io.quarkus.qute.nodeResolve");
 
     final ExpressionImpl expression;
     private final Engine engine;
     private final boolean traceLevel;
     private final boolean hasEngineResultMappers;
+    private final boolean unrestrictedCompletionStages;
 
     ExpressionNode(ExpressionImpl expression, Engine engine) {
         this.expression = expression;
         this.engine = engine;
         this.traceLevel = LOG.isTraceEnabled();
         this.hasEngineResultMappers = !engine.getResultMappers().isEmpty();
+        this.unrestrictedCompletionStages = CompletionStageSupport.UNRESTRICTED;
     }
 
     @Override
     public CompletionStage<ResultNode> resolve(ResolutionContext context) {
         if (traceLevel) {
             LOG.tracef("Resolve {%s} started:%s", expression.toOriginalString(), expression.getOrigin());
         }
-        return context.evaluate(expression).thenCompose(this);
+        return context.evaluate(expression).thenCompose(this::toResultNode);
     }
 
-    @Override
-    public CompletionStage<ResultNode> apply(Object result) {
+    CompletionStage<ResultNode> toResultNode(Object result) {
         if (traceLevel) {
             LOG.tracef("Resolve {%s} completed:%s", expression.toOriginalString(), expression.getOrigin());
         }
         if (result instanceof ResultNode) {
             return CompletedStage.of((ResultNode) result);
-        } else if (result instanceof CompletionStage) {
-            return ((CompletionStage<?>) result).thenCompose(this);
+        } else if (result instanceof CompletableFuture) {
+            return (CompletableFuture<ResultNode>) ((CompletionStage<?>) result).thenCompose(this::toResultNode);
+        } else if (result instanceof CompletedStage) {
+            return (CompletableFuture<ResultNode>) ((CompletionStage<?>) result).thenCompose(this::toResultNode);
+        } else if (unrestrictedCompletionStages && result instanceof CompletionStage) {
+            return ((CompletionStage<?>) result).thenCompose(this::toResultNode);
         } else {
             return CompletedStage.of(new SingleResultNode(result, this));
         }

diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/FieldAccessor.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/FieldAccessor.java
@@ -15,16 +15,10 @@ class FieldAccessor implements ValueAccessor, AccessorCandidate {
         this.field = field;
     }
 
-    @SuppressWarnings("unchecked")
     @Override
     public CompletionStage<Object> getValue(Object instance) {
         try {
-            Object ret = field.get(instance);
-            if (ret instanceof CompletionStage) {
-                return (CompletionStage<Object>) ret;
-            } else {
-                return CompletedStage.of(ret);
-            }
+            return CompletionStageSupport.toCompletionStage(field.get(instance));
         } catch (Exception e) {
             throw new IllegalStateException("Reflection invocation error", e);
         }

diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/GetterAccessor.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/GetterAccessor.java
@@ -15,16 +15,10 @@ class GetterAccessor implements ValueAccessor, AccessorCandidate {
         this.method = method;
     }
 
-    @SuppressWarnings("unchecked")
     @Override
     public CompletionStage<Object> getValue(Object instance) {
         try {
-            Object ret = method.invoke(instance);
-            if (ret instanceof CompletionStage) {
-                return (CompletionStage<Object>) ret;
-            } else {
-                return CompletedStage.of(ret);
-            }
+            return CompletionStageSupport.toCompletionStage(method.invoke(instance));
         } catch (Exception e) {
             throw new IllegalStateException("Reflection invocation error", e);
         }

diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/LazyValue.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/LazyValue.java
@@ -1,11 +1,15 @@
 package io.quarkus.qute;
 
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Supplier;
 
-final class LazyValue<T> {
+class LazyValue<T> {
 
     private final Supplier<T> supplier;
 
+    private final Lock lock = new ReentrantLock();
+
     private transient volatile T value;
 
     public LazyValue(Supplier<T> supplier) {
@@ -17,11 +21,15 @@ public T get() {
         if (valueCopy != null) {
             return valueCopy;
         }
-        synchronized (this) {
+
+        lock.lock();
+        try {
             if (value == null) {
                 value = supplier.get();
             }
             return value;
+        } finally {
+            lock.unlock();
         }
     }
 
@@ -30,13 +38,13 @@ public T getIfPresent() {
     }
 
     public void clear() {
-        synchronized (this) {
-            value = null;
-        }
+        lock.lock();
+        value = null;
+        lock.unlock();
     }
 
     public boolean isSet() {
         return value != null;
     }
 
-}
+}
diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/LoopSectionHelper.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/LoopSectionHelper.java
@@ -3,6 +3,8 @@
 import static io.quarkus.qute.Parameter.EMPTY;
 
 import java.lang.reflect.Array;
+import java.util.AbstractCollection;
+import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -71,27 +73,31 @@ public CompletionStage<ResultNode> resolve(SectionResolutionContext context) {
     }
 
     private static int extractSize(Object it) {
-        if (it instanceof Collection) {
-            return ((Collection<?>) it).size();
-        } else if (it instanceof Map) {
-            return ((Map<?, ?>) it).size();
+        // Note that we intentionally use "instanceof" to test interfaces as the last resort in order to mitigate the "type pollution"
+        // See https://github.com/RedHatPerf/type-pollution-agent for more information
+        if (it instanceof AbstractCollection) {
+            return ((AbstractCollection<?>) it).size();
+        } else if (it instanceof AbstractMap) {
+            return ((AbstractMap<?, ?>) it).size();
         } else if (it.getClass().isArray()) {
             return Array.getLength(it);
         } else if (it instanceof Integer) {
             return ((Integer) it);
+        } else if (it instanceof Collection) {
+            return ((Collection<?>) it).size();
+        } else if (it instanceof Map) {
+            return ((Map<?, ?>) it).size();
         }
         return 10;
     }
 
     private Iterator<?> extractIterator(Object it) {
-        if (it instanceof Iterable) {
-            return ((Iterable<?>) it).iterator();
-        } else if (it instanceof Iterator) {
-            return (Iterator<?>) it;
-        } else if (it instanceof Map) {
-            return ((Map<?, ?>) it).entrySet().iterator();
-        } else if (it instanceof Stream) {
-            return ((Stream<?>) it).sequential().iterator();
+        // Note that we intentionally use "instanceof" to test interfaces as the last resort in order to mitigate the "type pollution"
+        // See https://github.com/RedHatPerf/type-pollution-agent for more information
+        if (it instanceof AbstractCollection) {
+            return ((AbstractCollection<?>) it).iterator();
+        } else if (it instanceof AbstractMap) {
+            return ((AbstractMap<?, ?>) it).entrySet().iterator();
         } else if (it instanceof Integer) {
             return IntStream.rangeClosed(1, (Integer) it).iterator();
         } else if (it.getClass().isArray()) {
@@ -102,6 +108,14 @@ private Iterator<?> extractIterator(Object it) {
                 elements.add(Array.get(it, i));
             }
             return elements.iterator();
+        } else if (it instanceof Iterable) {
+            return ((Iterable<?>) it).iterator();
+        } else if (it instanceof Iterator) {
+            return (Iterator<?>) it;
+        } else if (it instanceof Map) {
+            return ((Map<?, ?>) it).entrySet().iterator();
+        } else if (it instanceof Stream) {
+            return ((Stream<?>) it).sequential().iterator();
         } else {
             TemplateException.Builder builder;
             if (Results.isNotFound(it)) {

diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/MultiResultNode.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/MultiResultNode.java
@@ -6,7 +6,7 @@
 /**
  * A result node backed by an array of result nodes.
  */
-public final class MultiResultNode implements ResultNode {
+public final class MultiResultNode extends ResultNode {
 
     private final Supplier<ResultNode>[] results;
 

diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/ResultNode.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/ResultNode.java
@@ -6,9 +6,9 @@
 /**
  * Node of a result tree.
  */
-public interface ResultNode {
+public abstract class ResultNode {
 
-    static CompletionStage<ResultNode> NOOP = CompletedStage.of(new ResultNode() {
+    public static CompletionStage<ResultNode> NOOP = CompletedStage.of(new ResultNode() {
         @Override
         public void process(Consumer<String> resultConsumer) {
         }
@@ -18,6 +18,6 @@ public void process(Consumer<String> resultConsumer) {
      *
      * @param resultConsumer
      */
-    void process(Consumer<String> resultConsumer);
+    public abstract void process(Consumer<String> resultConsumer);
 
 }
diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/SingleResultNode.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/SingleResultNode.java
@@ -9,7 +9,7 @@
 /**
  * A result node backed by a single object value.
  */
-public final class SingleResultNode implements ResultNode {
+public final class SingleResultNode extends ResultNode {
 
     private final Object value;
     private final ExpressionNode node;

diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/TextNode.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/TextNode.java
@@ -6,7 +6,7 @@
 /**
  * Static text.
  */
-public class TextNode implements TemplateNode, ResultNode {
+public class TextNode extends ResultNode implements TemplateNode {
 
     private final CompletedStage<ResultNode> result;
     private final String value;