Merge pull request #17 from Worklytics/s174-more-cleanup

S174 : more cleanup

java/pom.xml

@@ -8,7 +8,7 @@
   <url>https://github.com/Worklytics/appengine-pipelines/</url>
   <!-- follow semver, with suggested guidance for versioning fork of OSS -->
   <!-- see https://gofore.com/en/best-practices-for-forking-a-git-repo/ -->
-  <version>0.3+worklytics.3</version>
+  <version>0.3+worklytics.4</version>
   <packaging>jar</packaging>
   <licenses>
     <license>
@@ -111,12 +111,6 @@
 
   <dependencies>
     <!-- Compile/Runtime Dependencies -->
-    <dependency>
-      <groupId>org.json</groupId>
-      <artifactId>json</artifactId>
-      <version>20231013</version>
-    </dependency>
-
     <dependency>
       <groupId>com.google.appengine</groupId>
       <artifactId>appengine-api-1.0-sdk</artifactId>
@@ -191,17 +185,19 @@
       <artifactId>jakarta.inject-api</artifactId>
       <version>2.0.1</version>
     </dependency>
-    <!-- Dagger - Dependency Inject FW - http://square.github.io/dagger/ -->
+    <!-- Dagger - Dependency Inject FW - https://github.com/google/dagger -->
+    <!-- TBC: if micronaut, that provides compile-time DI too ... -->
+    <!-- also, is this very extensible? believe this will in effect require users to provide Dagger 2 module FQN to @Injectable to leverage this -->
     <dependency>
-      <groupId>com.squareup.dagger</groupId>
+      <groupId>com.google.dagger</groupId>
       <artifactId>dagger</artifactId>
-      <version>1.2.5</version>
+      <version>2.51.1</version>
       <scope>compile</scope>
     </dependency>
     <dependency>
-      <groupId>com.squareup.dagger</groupId>
+      <groupId>com.google.dagger</groupId>
       <artifactId>dagger-compiler</artifactId>
-      <version>1.2.5</version>
+      <version>2.51.1</version>
       <scope>compile</scope>
     </dependency>
 
@@ -229,11 +225,17 @@
       <artifactId>junit-jupiter-engine</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-params</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.mockito</groupId>
        <artifactId>mockito-core</artifactId>
        <version>4.11.0</version>
        <scope>test</scope>
     </dependency>
+
   </dependencies>
 </project>

java/src/main/java/com/google/appengine/tools/pipeline/DIContainer.java

@@ -0,0 +1,6 @@
+package com.google.appengine.tools.pipeline;
+
+public interface DIContainer<T> {
+  void inject(T instance);
+
+}

java/src/main/java/com/google/appengine/tools/pipeline/DefaultContainer.java

@@ -0,0 +1,18 @@
+package com.google.appengine.tools.pipeline;
+
+import com.google.appengine.tools.pipeline.impl.PipelineManager;
+import dagger.Component;
+
+import javax.inject.Singleton;
+
+@Singleton
+@Component(
+  modules = {
+    DefaultDIModule.class,
+  }
+)
+public interface DefaultContainer {
+
+  void injects(PipelineManager pipelineManager);
+
+}

java/src/main/java/com/google/appengine/tools/pipeline/DefaultDIModule.java

@@ -4,7 +4,6 @@
 import com.google.appengine.api.utils.SystemProperty;
 import com.google.appengine.tools.pipeline.impl.PipelineManager;
 import com.google.appengine.tools.pipeline.impl.backend.*;
-import com.google.appengine.tools.pipeline.impl.servlets.PipelineServlet;
 import com.google.auth.oauth2.GoogleCredentials;
 import com.google.cloud.datastore.DatastoreOptions;
 import dagger.Module;
@@ -14,13 +13,7 @@
 import javax.inject.Singleton;
 
 //TODO: split internals v stuff that can be re-used by others
-@Module(
-  injects = { //alphabetical order
-    PipelineServlet.class,
-  },
-  library = true,
-  complete = false
-)
+@Module
 public class DefaultDIModule {
 
   @Provides @Singleton
@@ -33,6 +26,11 @@ AppEngineBackEnd.Options appEngineBackEndOptions() {
       .build();
   }
 
+  @Provides @Singleton
+  PipelineBackEnd pipelineBackEnd(AppEngineBackEnd appEngineBackEnd) {
+    return appEngineBackEnd;
+  }
+
   @Provides @Singleton
   PipelineBackEnd.Options backendOptions(AppEngineBackEnd.Options options) {
     return options;

java/src/main/java/com/google/appengine/tools/pipeline/Injectable.java

@@ -6,7 +6,7 @@
 /**
  * equivalent of Spring component, in a sense
  *
- * FQN of Dagger 1 module class that should be used when filling this classes dependencies, if any.
+ * FQN of Dagger 2 module class that should be used when filling this classes dependencies, if any.
  *
  *
  * Usage:

java/src/main/java/com/google/appengine/tools/pipeline/impl/PipelineManager.java

@@ -775,10 +775,8 @@ private void inject(Job<?> job) {
       inject(((RootJobInstance) job).jobInstance);
     }
 
-    Optional<Injectable> injectable = Optional.ofNullable(job.getClass().getAnnotation(Injectable.class));
-
-    if (injectable.isPresent()) {
-      DIUtil.inject(injectable.get().value().getName(), job);
+    if (DIUtil.isInjectable(job)) {
+      DIUtil.inject(job);
     }
   }
 

java/src/main/java/com/google/appengine/tools/pipeline/impl/backend/AppEngineBackEnd.java

@@ -437,7 +437,7 @@ public List<Key> call() {
 
   @Override
   public Object deserializeValue(PipelineModelObject model, Object serializedVersion)
-      throws IOException {
+    throws IOException, ClassNotFoundException {
     if (serializedVersion instanceof Blob) {
       return SerializationUtils.deserialize(((Blob) serializedVersion).toByteArray());
     } else {

java/src/main/java/com/google/appengine/tools/pipeline/impl/backend/SerializationStrategy.java

@@ -30,6 +30,6 @@ public interface SerializationStrategy {
    * @throws IOException if any problem occurs
    */
   Object deserializeValue(PipelineModelObject model, Object serializedVersion)
-    throws IOException;
+    throws IOException, ClassNotFoundException;
 
 }

java/src/main/java/com/google/appengine/tools/pipeline/impl/model/ExceptionRecord.java

@@ -46,7 +46,7 @@ public ExceptionRecord(Entity entity) {
     byte[] serializedException = serializedExceptionBlob.toByteArray();
     try {
       exception = (Throwable) SerializationUtils.deserialize(serializedException);
-    } catch (IOException e) {
+    } catch (IOException | ClassNotFoundException e) {
       throw new RuntimeException("Failed to deserialize exception for " + getKey(), e);
     }
   }

java/src/main/java/com/google/appengine/tools/pipeline/impl/model/JobInstanceRecord.java

@@ -111,7 +111,7 @@ public synchronized Job<?> getJobInstanceDeserialized() {
     if (null == jobInstance) {
       try {
         jobInstance = (Job<?>) serializationStrategy.deserializeValue(this, value);
-      } catch (IOException e) {
+      } catch (IOException | ClassNotFoundException e) {
         throw new RuntimeException(
             "Exception while attempting to deserialize jobInstance for " + jobKey, e);
       }

java/src/main/java/com/google/appengine/tools/pipeline/impl/model/Slot.java

@@ -97,7 +97,7 @@ public Slot(Entity entity, SerializationStrategy serializationStrategy, boolean
   private Object deserializeValue(Object value) {
     try {
       return serializationStrategy.deserializeValue(this, value);
-    } catch (IOException e) {
+    } catch (IOException | ClassNotFoundException e) {
       throw new RuntimeException("Failed to deserialize slot value from " + this.getKey(), e);
     }
   }

java/src/main/java/com/google/appengine/tools/pipeline/impl/servlets/PipelineServlet.java

@@ -14,6 +14,7 @@
 
 package com.google.appengine.tools.pipeline.impl.servlets;
 
+import com.google.appengine.tools.pipeline.DaggerDefaultContainer;
 import com.google.appengine.tools.pipeline.DefaultDIModule;
 import com.google.appengine.tools.pipeline.PipelineOrchestrator;
 import com.google.appengine.tools.pipeline.PipelineRunner;
@@ -112,7 +113,7 @@ public void init() throws ServletException {
 
     // TODO: fix this? may have second copy IF user overrides; OK?
     if (pipelineManager == null) {
-      DIUtil.inject(DefaultDIModule.class.getName(), this);
+      DIUtil.inject(DaggerDefaultContainer.class, this);
     }
 
     //TODO: move these to DI?

java/src/main/java/com/google/appengine/tools/pipeline/impl/util/DIUtil.java

@@ -1,41 +1,71 @@
 package com.google.appengine.tools.pipeline.impl.util;
 
+/**
+ * Copyright Worklytics, Co. 2024.
+ */
+import com.google.appengine.tools.pipeline.Injectable;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-import dagger.ObjectGraph;
+
 import lombok.NonNull;
 import lombok.extern.java.Log;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 
 /**
- * Helper to get an ObjectGraph out of a Dagger Module.
+ * Helper to get a component instance out of Dagger, support runtime injection of classes.
  * <p>
- * This is mostly intended to be used for jobs, thus Default Module is linked somehow to AsyncModule.
  *
  */
-
 @Log
 public class DIUtil {
 
 
 	private static final Object lock = new Object();
 
-	private static final Map<String, ObjectGraph> objectGraphCache = new ConcurrentHashMap<>(1);
+	private static final Map<Class<?>, Object> componentCache = new ConcurrentHashMap<>(1);
 
-	private static final Map<String, ObjectGraph> overriddenObjectGraphCache = new ConcurrentHashMap<>(1);
+	private static final Map<Class<?>, Object> overriddenComponentCache = new ConcurrentHashMap<>(1);
 
 	// just for logging purposes
 	private static boolean overridden = false;
 
+  /**
+   * Injects an instance annotated with @Injectable, based on the module provided in the annotation
+   *
+   * @param instance
+   */
+  public static void inject(Object instance) {
+    Optional<Injectable> injectable = Optional.ofNullable(instance.getClass().getAnnotation(Injectable.class));
+
+    if (injectable.isPresent()) {
+      inject(injectable.get().value(), instance);
+    }
+  }
+
+  public static boolean isInjectable(Object instance) {
+    return instance.getClass().getAnnotation(Injectable.class) != null;
+  }
+
 	/**
 	 * Injects and Injectable instance through Reflection
-	 * @param injectable class to inject
+   * @param componentClass component class (Dagger-generated)
+	 * @param instance class instance to inject
 	 */
-	public static void inject(String moduleClassFQN, Object injectable) {
-		ObjectGraph objectGraph = getFromModuleClass(moduleClassFQN);
-    objectGraph.inject(injectable);
+	public static void inject(Class<?> componentClass, Object instance) {
+    Object objectGraph = getFromComponentClass(componentClass);
+
+    try {
+      Method injectMethod = objectGraph.getClass().getMethod("inject", instance.getClass());
+      injectMethod.setAccessible(true); //avoid 'volatile' thing
+      injectMethod.invoke(objectGraph, instance);
+    } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
+      throw new RuntimeException("Couldn't inject object " + e.getMessage(), e);
+    }
 	}
 
 	/**
@@ -47,47 +77,41 @@ public static void inject(String moduleClassFQN, Object injectable) {
 	 * - Module has a static instance of the graph it builds and provided by method of signature
 	 *   public static ObjectGraph getObjectGraph()
 	 *
-	 * @param moduleFQNClassName module fully qualified name
+	 * @param componentClass module fully qualified name
 	 * @return the object graph for that module
 	 * @throws RuntimeException if the class is not a Dagger Module or doesn't meet the requirements
 	 */
-	public static ObjectGraph getFromModuleClass(@NonNull String moduleFQNClassName) {
+	public static Object getFromComponentClass(@NonNull Class<?> componentClass) {
     synchronized (lock) {
       if (overridden) {
-        return overriddenObjectGraphCache.getOrDefault(moduleFQNClassName, objectGraphCache.get(moduleFQNClassName));
+        return overriddenComponentCache.getOrDefault(componentClass, componentCache.get(componentClass));
       }
       try {
-        Class<?> moduleClass = Class.forName(moduleFQNClassName);
-        Preconditions.checkArgument(moduleClass.isAnnotationPresent(dagger.Module.class), "Class is not a Dagger Module: " + moduleFQNClassName);
-
-        ObjectGraph objectGraph = ObjectGraph.create(moduleClass.newInstance());
-        objectGraphCache.put(moduleFQNClassName, objectGraph);
-        return objectGraph;
-      } catch (ClassNotFoundException | IllegalAccessException | InstantiationException e) {
+        Object component = componentClass.getMethod("create").invoke(null);
+        componentCache.put(componentClass, component);
+        return component;
+      } catch (IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
         throw new RuntimeException("Couldn't get an object graph " + e.getMessage(), e);
       }
     }
   }
 
-	/**
-	 * A test helper to directly override the default module with the test's graph
-	 * @param graph the object graph to override for default module
-	 */
-	@VisibleForTesting
-	public static void overrideGraphForTests(String key, ObjectGraph graph) {
-		synchronized (lock) {
-			overriddenObjectGraphCache.put(key, graph);
-			overridden = true;
-		}
-	}
+
+  @VisibleForTesting
+  public static void overrideComponentInstanceForTests(Class<?> clazz, Object componentInstance) {
+    synchronized (lock) {
+      overriddenComponentCache.put(clazz, componentInstance);
+      overridden = true;
+    }
+  }
 
 	/**
-	 * Resets the objectGraphCache to its original state (intended to be used on tests' teardown)
+	 * Resets the componentCache to its original state (intended to be used on tests' teardown)
 	 */
 	@VisibleForTesting
-	public static void resetGraph() {
+	public static void resetComponents() {
 		synchronized (lock) {
-			overriddenObjectGraphCache.clear();
+			overriddenComponentCache.clear();
 			overridden = false;
 			log.fine("Reset overridden DI Module");
 		}