Interceptor Redo (helidon-io#6658)

* Resolves issue helidon-io#6629

Author: trentjeff

pico/api/src/main/java/io/helidon/pico/api/Interceptor.java

@@ -32,6 +32,7 @@ public interface Interceptor {
      * @param args  the arguments to the call
      * @param <V>   the return value type (or {@link Void} for void method elements)
      * @return the return value to the caller
+     * @throws InvocationException if there are errors during invocation chain processing
      */
     <V> V proceed(InvocationContext ctx, Chain<V> chain, Object... args);
 
@@ -43,13 +44,14 @@ public interface Interceptor {
      */
     interface Chain<V> {
         /**
-         * Call the next interceptor in line, or finishing with the call to the service provider.
+         * Call the next interceptor in line, or finishing with the call to the service provider being intercepted.
          * Note that that arguments are passed by reference to each interceptor ultimately leading up to the final
-         * call to the underlying intercepted target. Callers can mutable the arguments passed directly on the provided array
+         * call to the underlying intercepted target. Callers can mutate the arguments passed directly on the provided array
          * instance.
          *
          * @param args the arguments passed
          * @return the result of the call
+         * @throws InvocationException if there are errors during invocation chain processing
          */
         V proceed(Object[] args);
     }

pico/api/src/main/java/io/helidon/pico/api/InvocationException.java

@@ -21,15 +21,59 @@
  */
 public class InvocationException extends PicoServiceProviderException {
 
+    /**
+     * Tracks whether the target being intercepted was called once successfully - meaning that the target was called and it
+     * did not result in any exception being thrown.
+     */
+    private final boolean targetWasCalled;
+
+    /**
+     * Constructor.
+     *
+     * @param msg               the message
+     * @param targetWasCalled   set to true if the target of interception was ultimately called successfully
+     */
+    public InvocationException(String msg,
+                               boolean targetWasCalled) {
+        super(msg);
+        this.targetWasCalled = targetWasCalled;
+    }
+    /**
+     * Constructor.
+     *
+     * @param msg               the message
+     * @param cause             the root cause
+     * @param targetWasCalled   set to true if the target of interception was ultimately called successfully
+     */
+    public InvocationException(String msg,
+                               Throwable cause,
+                               boolean targetWasCalled) {
+        super(msg, cause);
+        this.targetWasCalled = targetWasCalled;
+    }
     /**
      * Constructor.
      *
      * @param msg               the message
      * @param cause             the root cause
      * @param serviceProvider   the service provider
+     * @param targetWasCalled   set to true if the target of interception was ultimately called successfully
      */
-    public InvocationException(String msg, Throwable cause, ServiceProvider<?> serviceProvider) {
+    public InvocationException(String msg,
+                               Throwable cause,
+                               ServiceProvider<?> serviceProvider,
+                               boolean targetWasCalled) {
         super(msg, cause, serviceProvider);
+        this.targetWasCalled = targetWasCalled;
+    }
+
+    /**
+     * Returns true if the final target of interception was ultimately called.
+     *
+     * @return if the target being intercepted was ultimately called successfully
+     */
+    public boolean targetWasCalled() {
+        return targetWasCalled;
     }
 
 }

pico/runtime/src/main/java/io/helidon/pico/runtime/AbstractServiceProvider.java

@@ -264,7 +264,7 @@ public void interceptor(ServiceProvider<?> interceptor) {
         }
         this.interceptor = interceptor;
         if (interceptor instanceof AbstractServiceProvider<?>) {
-            ((AbstractServiceProvider) interceptor).intercepted(this);
+            ((AbstractServiceProvider<?>) interceptor).intercepted(this);
         }
     }
 

pico/runtime/src/main/java/io/helidon/pico/runtime/InterceptedMethod.java

@@ -128,10 +128,13 @@ public InvocationContext ctx() {
     public V apply(Object... args) {
         try {
             return invoke(args);
-        } catch (RuntimeException e) {
-            throw e;
         } catch (Throwable t) {
-            throw new InvocationException(t.getMessage(), t, ctx.serviceProvider());
+            boolean targetWasCalledSuccessfully = false;
+            if (t instanceof InvocationException) {
+                targetWasCalledSuccessfully = ((InvocationException) t).targetWasCalled();
+            }
+
+            throw new InvocationException(t.getMessage(), t, ctx.serviceProvider(), targetWasCalledSuccessfully);
         }
     }
 

pico/runtime/src/main/java/io/helidon/pico/runtime/Invocation.java

@@ -19,32 +19,35 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import java.util.ListIterator;
 import java.util.Objects;
 import java.util.function.Function;
 
 import io.helidon.pico.api.Interceptor;
 import io.helidon.pico.api.InvocationContext;
+import io.helidon.pico.api.InvocationException;
 import io.helidon.pico.api.ServiceProvider;
 
 import jakarta.inject.Provider;
 
 /**
  * Handles the invocation of {@link Interceptor} methods.
+ * Note that upon a successful call to the {@link io.helidon.pico.api.Interceptor.Chain#proceed(Object[])} or to the ultimate
+ * target, the invocation will be prevented from being executed again.
  *
  * @see io.helidon.pico.api.InvocationContext
  * @param <V> the invocation type
  */
 public class Invocation<V> implements Interceptor.Chain<V> {
     private final InvocationContext ctx;
-    private final ListIterator<Provider<Interceptor>> interceptorIterator;
+    private final List<Provider<Interceptor>> interceptors;
+    private int interceptorPos;
     private Function<Object[], V> call;
 
     private Invocation(InvocationContext ctx,
                        Function<Object[], V> call) {
         this.ctx = ctx;
         this.call = Objects.requireNonNull(call);
-        this.interceptorIterator = ctx.interceptors().listIterator();
+        this.interceptors = List.copyOf(ctx.interceptors());
     }
 
     @Override
@@ -60,25 +63,43 @@ public String toString() {
      * @param args  the call arguments
      * @param <V>   the type returned from the method element
      * @return the invocation instance
+     * @throws InvocationException if there are errors during invocation chain processing
      */
     @SuppressWarnings({"unchecked", "rawtypes"})
     public static <V> V createInvokeAndSupply(InvocationContext ctx,
                                               Function<Object[], V> call,
                                               Object[] args) {
         if (ctx.interceptors().isEmpty()) {
-            return call.apply(args);
+            try {
+                return call.apply(args);
+            } catch (Throwable t) {
+                throw new InvocationException("Error in interceptor chain processing", t, true);
+            }
         } else {
             return (V) new Invocation(ctx, call).proceed(args);
         }
     }
 
+    /**
+     * The degenerate case for {@link #mergeAndCollapse(List[])}. This is here only to eliminate the unchecked varargs compiler
+     * warnings that would otherwise be issued in code that does not have any interceptors on a method.
+     *
+     * @param <T>   the type of the provider
+     * @return an empty list
+     * @deprecated this method should only be called by generated code
+     */
+    @Deprecated
+    public static <T> List<Provider<T>> mergeAndCollapse() {
+        return List.of();
+    }
+
     /**
      * Merges a variable number of lists together, where the net result is the merged set of non-null providers
      * ranked in proper weight order, or else empty list.
      *
      * @param lists the lists to merge
      * @param <T>   the type of the provider
-     * @return the merged result, or null instead of empty lists
+     * @return the merged result or empty list if there is o interceptor providers
      */
     @SuppressWarnings("unchecked")
     public static <T> List<Provider<T>> mergeAndCollapse(List<Provider<T>>... lists) {
@@ -118,19 +139,52 @@ public static <T> List<Provider<T>> mergeAndCollapse(List<Provider<T>>... lists)
 
     @Override
     public V proceed(Object... args) {
-        if (interceptorIterator.hasNext()) {
-            return interceptorIterator.next()
-                    .get()
-                    .proceed(ctx, this, args);
-        } else {
-            if (this.call != null) {
-                Function<Object[], V> call = this.call;
-                this.call = null;
-                return call.apply(args);
-            } else {
-                throw new IllegalStateException("Duplicate invocation, or unknown call type: " + this);
+        if (this.call == null) {
+            throw new InvocationException("Duplicate invocation, or unknown call type: " + this, true);
+        }
+
+        if (interceptorPos < interceptors.size()) {
+            Provider<Interceptor> interceptorProvider =  interceptors.get(interceptorPos);
+            Interceptor interceptor = interceptorProvider.get();
+            interceptorPos++;
+            try {
+                return interceptor.proceed(ctx, this, args);
+            } catch (Throwable t) {
+                interceptorPos--;
+
+                if (t instanceof InvocationException) {
+                    throw t;
+                }
+
+                throw (interceptorProvider instanceof ServiceProvider)
+                        ? new InvocationException("Error in interceptor chain processing",
+                                                  t,
+                                                  (ServiceProvider<?>) interceptorProvider,
+                                                  call == null)
+                        : new InvocationException("Error in interceptor chain processing",
+                                                  t,
+                                                  call == null);
             }
         }
+
+        Function<Object[], V> call = this.call;
+        this.call = null;
+
+        try {
+            return call.apply(args);
+        } catch (Throwable t) {
+            if (t instanceof InvocationException) {
+                if (!((InvocationException) t).targetWasCalled()) {
+                    // allow the call to happen again
+                    this.call = call;
+                }
+                throw t;
+            }
+
+            // allow the call to happen again
+            this.call = call;
+            throw new InvocationException("Error in interceptor chain processing", t, true);
+        }
     }
 
 }

pico/runtime/src/test/java/io/helidon/pico/runtime/Control.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2023 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.pico.runtime;
+
+import java.util.Optional;
+
+import io.helidon.builder.Builder;
+import io.helidon.config.metadata.ConfiguredOption;
+
+@Builder
+abstract class Control {
+    abstract Optional<RuntimeException> exceptionBeforeProceed();
+    abstract Optional<RuntimeException> exceptionAfterProceed();
+    abstract Optional<Object> shortCircuitValue();
+    abstract int timesToCatchException();
+    @ConfiguredOption("1") abstract int timesToCallProceed();
+}