improve javadoc

gobblin-api/src/main/java/org/apache/gobblin/initializer/Initializer.java

@@ -32,21 +32,28 @@ public interface Initializer extends Closeable {
 
   /**
    * Marker interface to convey an opaque snapshot of the internal state of any concrete {@link Initializer}, thus affording state serialization for
-   * eventual "revival" as a new `Initializer` holding equivalent internal state.  {@link #commemorate()} the memento after {@link #initialize()}
-   * and subsequently {@link #recall(AfterInitializeMemento)} before {@link #close()}ing it.
+   * eventual "revival" as a new `Initializer` holding equivalent internal state.  {@link #commemorate()} (i.e. create) the memento after
+   * {@link #initialize()} and subsequently {@link #recall(AfterInitializeMemento)} the state it preserved before performing {@link #close()}.
    *
-   * Whereas the "Initializer Lifecycle", when synchronous and with the same instance, is:
-   *   [concrete `? implements Initializer` instance A]  `.initialize()` -> DO PROCESSING -> `.close()`
-   * When using `AfterInitializer` across instances and even memory-space boundaries it becomes:
-   *   [concrete `T0 implements Initializer` instance A] `.initialize()` -> `.commemorate()` -> PERSIST/TRANSMIT MEMENTO
-   *       -> DO PROCESSING ->
-   *   [concrete `T0 implements Initializer` instance B] RECEIVE MEMENTO -> `.recall()` -> `.close()`
+   * When synchronous and the same instance throughout, the "Initializer Lifecycle" is:
+   *   [concrete `My_T implements Initializer`, instance A] -
+   *       `.initialize()`; ==PROCESSING RUNS==; `.close()`;
    *
-   * For both backwards compatibility and because not every concrete `Initializer` has internal state worth capturing, not every `Initializer`
-   * impl will implement an `AfterInitializeMemento`.  Those that do will supply a unique impl cultivating self-aware impl details of their
-   * `Initializer`.  An `AfterInitializeMemento` impl needs simply be (de)serializable by {@link ObjectMapper}.  An `Initializer` impl with an
-   * `AfterInitializeMemento` impl MUST NOT (re-)process any {@link org.apache.gobblin.source.workunit.WorkUnit}s during its {@link #close()}
-   * method: `WorkUnit` processing MUST proceed entirely within {@link #initialize()}.
+   * When trading `AfterInitializeMemento` between instances (even memory-space boundaries) it becomes:
+   *   [concrete `My_T implements Initializer`, instance A] -
+   *       `.initialize()`; `.commemorate()`; ==PERSIST/TRANSMIT MEMENTO==
+   *   ==PROCESSING RUNS==;
+   *   [concrete `My_T implements Initializer`, instance B] -
+   *       ==RECEIVE MEMENTO==; `.recall()`; `.close()`
+   *
+   * Both for backwards compatibility and because not every concrete `Initializer` has internal state worth capturing, not every `Initializer`
+   * impl will implement an `AfterInitializeMemento`.  Those that do will supply a unique impl capturing self-aware impl details of their
+   * `Initializer`.
+   *
+   * An `AfterInitializeMemento` impl needs simply be (de)serializable by {@link ObjectMapper}.
+   *
+   * An `Initializer` impl with an `AfterInitializeMemento` impl MUST NOT (re-)process any {@link org.apache.gobblin.source.workunit.WorkUnit}s
+   * during its {@link #close()} method: `WorkUnit` processing MUST occur entirely within {@link #initialize()}.
    */
   @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY, property = "@class") // to handle variety of concrete impls
   public interface AfterInitializeMemento {
@@ -89,7 +96,7 @@ static AfterInitializeMemento deserialize(String serialized) {
       }
     }
 
-    /** cast `this` concrete `AfterInitializeMemento` to `castClass`, else {@link MismatchedMementoException} */
+    /** cast `this` (concrete `AfterInitializeMemento`) to `castClass`, else {@link MismatchedMementoException} */
     default <T extends AfterInitializeMemento> T castAsOrThrow(Class<T> castClass, Initializer forInitializer)
         throws MismatchedMementoException {
       if (castClass.isAssignableFrom(this.getClass())) {
@@ -111,18 +118,18 @@ default <T extends AfterInitializeMemento> T castAsOrThrow(Class<T> castClass, I
    * @see java.io.Closeable#close()
    *
    * NOTE: An `Initializer` impl with an `AfterInitializeMemento` impl MUST NOT (re-)process any {@link org.apache.gobblin.source.workunit.WorkUnit}s
-   * during its {@link #close()} method: `WorkUnit` processing MUST proceed entirely within {@link #initialize()}.
+   * during its {@link #close()} method: `WorkUnit` processing MUST occur entirely within {@link #initialize()}.
    */
   @Override
   public void close();
 
-  /** @return any `Initializer`-specific companion memento, as required to convey internal state after {@link #initialize()}, as needed to {@link #close()} */
+  /** @return the `Initializer`-specific companion memento, to convey internal state after {@link #initialize()}, and as needed to {@link #close()} */
   default Optional<AfterInitializeMemento> commemorate() {
     return Optional.empty();
   }
 
   /**
-   * "reinitialize" a fresh instance with (equiv.) post {@link #initialize()} internal state, per `Initializer`-specific companion `memento`
+   * "reinitialize" a fresh instance, per `Initializer`-specific companion `memento`, with (equivalent) post {@link #initialize()} internal state needed
    * to {@link #close()}
    */
   default void recall(AfterInitializeMemento memento) {

gobblin-core/src/test/java/org/apache/gobblin/initializer/MultiInitializerTest.java

@@ -121,14 +121,15 @@ public void recall(AfterInitializeMemento memento) {
     }
   }
 
+
   @Test
   public void testMementoCommemorateToSerializeAndDeserializeForRecall() {
     // create "first generation" of concrete initializers
     InitializerImplA initializerA_1 = new InitializerImplA();
     InitializerImplBNoMemento initializerB_1 = new InitializerImplBNoMemento();
     InitializerImplC initializerC_1 = new InitializerImplC();
 
-    // create the 1st-gen `MultiInitializer` and `initialize` all wrapped initializers
+    // create the 1st-gen `MultiInitializer`; `initialize` all wrapped initializers
     MultiInitializer multiInitializer1G = new MultiInitializer(Arrays.asList(initializerA_1, initializerB_1, initializerC_1));
     multiInitializer1G.initialize();
 
@@ -137,7 +138,7 @@ public void testMementoCommemorateToSerializeAndDeserializeForRecall() {
     Assert.assertTrue(optMemento1G.isPresent());
     String serializedMemento = Initializer.AfterInitializeMemento.serialize(optMemento1G.get());
 
-    // create a new 2nd-gen `MultiInitializer` using a "second generation" of concrete initializers
+    // create a new 2nd-gen `MultiInitializer` with a "second generation" of concrete initializers... but DO NOT `initialize` them!
     InitializerImplA initializerA_2 = new InitializerImplA();
     InitializerImplBNoMemento initializerB_2 = new InitializerImplBNoMemento();
     InitializerImplC initializerC_2 = new InitializerImplC();
@@ -151,12 +152,12 @@ public void testMementoCommemorateToSerializeAndDeserializeForRecall() {
     try {
       // verify not possible to `commemorate` prior to `recall()`
       multiInitializer2G.commemorate();
-      Assert.fail("`commemorate()` somehow possible even before `Initializer.initialize()` or `recall()`, despite `@NotNull` annotation on `state`");
+      Assert.fail("`commemorate()` somehow possible even before `Initializer.initialize()` or `recall()` - despite `@NotNull` annotation on `state`");
     } catch (NullPointerException npe) {
       // expected
     }
 
-    // now `deserialize` 1st-gen state and `recall` it to the 2nd-gen `MultiInitializer`
+    // next, `deserialize` 1st-gen state and `recall` it to the (un-`initialize`d) 2nd-gen `MultiInitializer`
     Initializer.AfterInitializeMemento deserializedMemento = Initializer.AfterInitializeMemento.deserialize(serializedMemento);
     multiInitializer2G.recall(deserializedMemento);
     Optional<Initializer.AfterInitializeMemento> optMemento2G = multiInitializer2G.commemorate();
@@ -169,7 +170,7 @@ public void testMementoCommemorateToSerializeAndDeserializeForRecall() {
     multiInitializer2G.initialize();
     Optional<Initializer.AfterInitializeMemento> optMemento2G_alt = multiInitializer2G.commemorate();
     Assert.assertTrue(optMemento2G_alt.isPresent());
-    // verify not simply that mementos always equal
+    // verify not simply the case that mementos always equal
     Assert.assertNotEquals(optMemento2G.get(), optMemento2G_alt.get());
   }
 }

gobblin-metastore/build.gradle

@@ -45,7 +45,7 @@ dependencies {
     testCompile externalDependency.slf4jToLog4j
 }
 
-// Begin HACK to get around POM being depenendent on the (empty) gobblin-rest-api instead of gobblin-rest-api-rest-client
+// Begin HACK to get around POM being dependent on the (empty) gobblin-rest-api instead of gobblin-rest-api-rest-client
 def installer = install.repositories.mavenInstaller
 [installer]*.pom*.whenConfigured {pom ->
     pom.dependencies.find {dep -> dep.groupId == project.group && dep.artifactId == 'gobblin-rest-api' }.artifactId = 'gobblin-rest-api-rest-client'

gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/writer/initializer/JdbcWriterInitializer.java

@@ -69,7 +69,7 @@ public class JdbcWriterInitializer implements WriterInitializer {
   @NoArgsConstructor // IMPORTANT: for jackson (de)serialization
   @AllArgsConstructor
   private static class Memento implements AfterInitializeMemento {
-    // NOTE: as this clearly MAY be `null` (below), DO NOT mark `@NonNull`, to avoid:
+    // NOTE: as this clearly MAY be `null` (see below), DO NOT mark `@NonNull`, to avoid:
     //   userCreatedStagingTable is marked non-null but is null
     private String userCreatedStagingTable;
     @NonNull private Set<String> createdStagingTables;
@@ -244,7 +244,7 @@ public void initialize() {
         i++;
 
         if (isSkipStaging) {
-          LOG.info("User chose to skip staing table on branch " + this.branchId + " workunit " + i);
+          LOG.info("User chose to skip staging table on branch " + this.branchId + " workunit " + i);
           wu.setProp(stagingTableKey, publishTable);
 
           if (i == 0) {