Merge pull request #15419 from cdapio/CDAP-20877-fix-unsafe-unboxing-…

…of-Boolean

[🍒][CDAP-20877] Fix the null pointer exception (from #15410)

cdap-app-fabric/src/main/java/io/cdap/cdap/app/store/Store.java

@@ -359,14 +359,28 @@ List<ProgramHistory> getRuns(Collection<ProgramReference> programs, ProgramRunSt
 
   /**
    * Creates new application if it doesn't exist. Updates existing one otherwise.
+   * Always marks the added application as latest.
    *
    * @param id            application id
    * @param meta          application metadata to store
    * @return              the number of edits to the application. A new application will return 0.
    * @throws ConflictException if the app cannot be deployed when the user provided parent-version doesn't match the
    *     current latest version
    */
-  int addApplication(ApplicationId id, ApplicationMeta meta) throws ConflictException;
+  int addLatestApplication(ApplicationId id, ApplicationMeta meta) throws ConflictException;
+
+  /**
+   * Creates new application if it doesn't exist. Updates existing one otherwise.
+   * Marks the application as latest based on the isLatest param.
+   *
+   * @param id            application id
+   * @param meta          application metadata to store
+   * @param isLatest      boolean, indicating if the application should be marked latest
+   * @return              the number of edits to the application. A new application will return 0.
+   * @throws ConflictException if the app cannot be deployed when the user provided parent-version doesn't match the
+   *     current latest version
+   */
+  int addApplication(ApplicationId id, ApplicationMeta meta, boolean isLatest) throws ConflictException;
 
   /**
    * Marks existing applications as latest.

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/deploy/pipeline/ApplicationRegistrationStage.java

@@ -67,9 +67,9 @@ public void process(ApplicationWithPrograms input) throws Exception {
     boolean ownerAdded = addOwnerIfRequired(input, allAppVersionsAppIds);
     ApplicationMeta appMeta = new ApplicationMeta(applicationSpecification.getName(),
         input.getSpecification(),
-        input.getChangeDetail(), input.getSourceControlMeta(), !input.isSkipMarkingLatest());
+        input.getChangeDetail(), input.getSourceControlMeta());
     try {
-      int editCount = store.addApplication(input.getApplicationId(), appMeta);
+      int editCount = store.addApplication(input.getApplicationId(), appMeta, !input.isSkipMarkingLatest());
 
       if (input.isSkipMarkingLatest()) {
         // TODO [CDAP-20848]

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/AppMetadataStore.java

@@ -59,7 +59,6 @@
 import io.cdap.cdap.proto.id.ProgramReference;
 import io.cdap.cdap.proto.id.ProgramRunId;
 import io.cdap.cdap.proto.sourcecontrol.SourceControlMeta;
-import io.cdap.cdap.spi.data.InvalidFieldException;
 import io.cdap.cdap.spi.data.SortOrder;
 import io.cdap.cdap.spi.data.StructuredRow;
 import io.cdap.cdap.spi.data.StructuredTable;
@@ -630,6 +629,7 @@ public void markAsLatest(ApplicationId id)
 
   /**
    * Persisting a new application version in the table.
+   * Marks the created application version as latest (always).
    *
    * @param id the application id
    * @param appMeta the application metadata to be written
@@ -638,12 +638,28 @@ public void markAsLatest(ApplicationId id)
    * @throws ConflictException if parent-version provided in the request doesn't match the
    *     latest version, do not allow app to be created
    */
-  public int createApplicationVersion(ApplicationId id, ApplicationMeta appMeta)
+  public int createLatestApplicationVersion(ApplicationId id, ApplicationMeta appMeta)
+      throws IOException, ConflictException {
+    return createApplicationVersion(id, appMeta, true);
+  }
+
+  /**
+   * Persisting a new application version in the table.
+   * Marks the created application version as latest based on the value of markAsLatest.
+   *
+   * @param id the application id
+   * @param appMeta the application metadata to be written
+   * @param markAsLatest boolean, indicating if the application should be marked as latest
+   * @return the number of edits to the application. A new application will return 0.
+   * @throws IOException if failed to write app
+   * @throws ConflictException if parent-version provided in the request doesn't match the
+   *     latest version, do not allow app to be created
+   */
+  public int createApplicationVersion(ApplicationId id, ApplicationMeta appMeta, boolean markAsLatest)
       throws IOException, ConflictException {
     String parentVersion = Optional.ofNullable(appMeta.getChange())
         .map(ChangeDetail::getParentVersion).orElse(null);
 
-    boolean markAsLatest = appMeta.getIsLatest();
     // Fetch the latest version
     ApplicationMeta latest = getLatest(id.getAppReference());
     String latestVersion = latest == null ? null : latest.getSpec().getAppVersion();
@@ -695,11 +711,21 @@ void writeApplication(String namespaceId, String appId, String versionId,
       @Nullable SourceControlMeta sourceControlMeta, boolean markAsLatest) throws IOException {
     writeApplicationSerialized(namespaceId, appId, versionId,
         GSON.toJson(
-            new ApplicationMeta(appId, spec, null, null, markAsLatest)),
+            new ApplicationMeta(appId, spec, null, null)),
         change, sourceControlMeta, markAsLatest);
     updateApplicationEdit(namespaceId, appId);
   }
 
+  @VisibleForTesting
+  List<Field<?>> getApplicationPrimaryKeys(String namespaceId, String appId,
+      String versionId) {
+    List<Field<?>> fields = new ArrayList<>();
+    fields.add(Fields.stringField(StoreDefinition.AppMetadataStore.NAMESPACE_FIELD, namespaceId));
+    fields.add(Fields.stringField(StoreDefinition.AppMetadataStore.APPLICATION_FIELD, appId));
+    fields.add(Fields.stringField(StoreDefinition.AppMetadataStore.VERSION_FIELD, versionId));
+    return fields;
+  }
+
   /**
    * Get the edit number of an application.
    *
@@ -2478,15 +2504,6 @@ private List<Field<?>> getNamespaceApplicationKeys(ApplicationReference appRef)
     return fields;
   }
 
-  private List<Field<?>> getApplicationPrimaryKeys(String namespaceId, String appId,
-      String versionId) {
-    List<Field<?>> fields = new ArrayList<>();
-    fields.add(Fields.stringField(StoreDefinition.AppMetadataStore.NAMESPACE_FIELD, namespaceId));
-    fields.add(Fields.stringField(StoreDefinition.AppMetadataStore.APPLICATION_FIELD, appId));
-    fields.add(Fields.stringField(StoreDefinition.AppMetadataStore.VERSION_FIELD, versionId));
-    return fields;
-  }
-
   private List<Field<?>> getApplicationNamespaceAppCreationKeys(ApplicationId appId)
       throws IOException {
     List<Field<?>> fields = new ArrayList<>();
@@ -2644,7 +2661,8 @@ private ApplicationMeta decodeRow(StructuredRow row) {
     } else {
       changeDetail = new ChangeDetail(changeSummary, null, author, creationTimeMillis, latest);
     }
-    return new ApplicationMeta(id, spec, changeDetail, sourceControl, latest);
+
+    return new ApplicationMeta(id, spec, changeDetail, sourceControl);
   }
 
   private void writeToStructuredTableWithPrimaryKeys(

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/ApplicationMeta.java

@@ -36,23 +36,13 @@ public class ApplicationMeta {
   private final ChangeDetail change;
   @Nullable
   private final SourceControlMeta sourceControlMeta;
-  // the isLatest field does not need to be serialized in the ApplicationMetadata object
-  // as it's stored as a separate column in the app spec table.
-  private final transient boolean isLatest;
 
   public ApplicationMeta(String id, ApplicationSpecification spec,
-      @Nullable ChangeDetail change, @Nullable SourceControlMeta sourceControlMeta,
-      boolean isLatest) {
+      @Nullable ChangeDetail change, @Nullable SourceControlMeta sourceControlMeta) {
     this.id = id;
     this.spec = spec;
     this.change = change;
     this.sourceControlMeta = sourceControlMeta;
-    this.isLatest = isLatest;
-  }
-
-  public ApplicationMeta(String id, ApplicationSpecification spec,
-      @Nullable ChangeDetail change, @Nullable SourceControlMeta sourceControlMeta) {
-    this(id, spec, change, sourceControlMeta, true);
   }
 
   public ApplicationMeta(String id, ApplicationSpecification spec, @Nullable ChangeDetail change) {
@@ -77,18 +67,13 @@ public SourceControlMeta getSourceControlMeta() {
     return sourceControlMeta;
   }
 
-  public boolean getIsLatest() {
-    return isLatest;
-  }
-
   @Override
   public String toString() {
     return Objects.toStringHelper(this)
         .add("id", id)
         .add("spec", ADAPTER.toJson(spec))
         .add("change", change)
         .add("sourceControlMeta", sourceControlMeta)
-        .add("isLatest", isLatest)
         .toString();
   }
 }

cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java

@@ -594,9 +594,16 @@ public void markApplicationsLatest(Collection<ApplicationId> appIds)
   }
 
   @Override
-  public int addApplication(ApplicationId id, ApplicationMeta meta) throws ConflictException {
+  public int addLatestApplication(ApplicationId id, ApplicationMeta meta) throws ConflictException {
     return TransactionRunners.run(transactionRunner, context -> {
-      return getAppMetadataStore(context).createApplicationVersion(id, meta);
+      return getAppMetadataStore(context).createLatestApplicationVersion(id, meta);
+    }, ConflictException.class);
+  }
+
+  @Override
+  public int addApplication(ApplicationId id, ApplicationMeta meta, boolean isLatest) throws ConflictException {
+    return TransactionRunners.run(transactionRunner, context -> {
+      return getAppMetadataStore(context).createApplicationVersion(id, meta, isLatest);
     }, ConflictException.class);
   }
 

cdap-app-fabric/src/test/java/io/cdap/cdap/gateway/handlers/OperationsDashboardHttpHandlerTest.java

@@ -301,7 +301,7 @@ private void addAppSpecs() throws ConflictException {
     ApplicationMeta meta = new ApplicationMeta(dummyAppSpec1.getName(), dummyAppSpec1,
                                                new ChangeDetail(null, null, null,
                                                                 System.currentTimeMillis()));
-    store.addApplication(APP1_ID, meta);
+    store.addLatestApplication(APP1_ID, meta);
     WorkflowSpecification scheduledWorfklow2 =
       new WorkflowSpecification("DummyClass", SCHEDULED_PROG2_ID.getProgram(), "scheduled workflow",
                                 Collections.emptyMap(), Collections.emptyList(), Collections.emptyMap(),
@@ -319,7 +319,7 @@ private void addAppSpecs() throws ConflictException {
     meta = new ApplicationMeta(dummyAppSpec2.getName(), dummyAppSpec2,
                                new ChangeDetail(null, null, null,
                                                 System.currentTimeMillis()));
-    store.addApplication(APP2_ID, meta);
+    store.addLatestApplication(APP2_ID, meta);
   }
 
   /**

cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/http/handlers/PreferencesHttpHandlerInternalTest.java

@@ -54,7 +54,7 @@ private void addApplication(String namespace, Application app) throws ConflictEx
     ApplicationMeta meta = new ApplicationMeta(appSpec.getName(), appSpec,
                                                new ChangeDetail(null, null, null,
                                                                 System.currentTimeMillis()));
-    store.addApplication(new ApplicationId(namespace, appSpec.getName()), meta);
+    store.addLatestApplication(new ApplicationId(namespace, appSpec.getName()), meta);
   }
 
   @Test

cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/http/handlers/PreferencesHttpHandlerTest.java

@@ -55,7 +55,7 @@ private void addApplication(String namespace, Application app) throws ConflictEx
     ApplicationMeta meta = new ApplicationMeta(appSpec.getName(), appSpec,
                                                new ChangeDetail(null, null, null,
                                                                 System.currentTimeMillis()));
-    store.addApplication(new ApplicationId(namespace, appSpec.getName()), meta);
+    store.addLatestApplication(new ApplicationId(namespace, appSpec.getName()), meta);
   }
 
   @Test

cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/AppMetadataStoreTest.java

@@ -19,13 +19,16 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
 import io.cdap.cdap.AllProgramsApp;
 import io.cdap.cdap.api.app.ApplicationSpecification;
 import io.cdap.cdap.api.artifact.ArtifactId;
 import io.cdap.cdap.app.store.ScanApplicationsRequest;
 import io.cdap.cdap.common.app.RunIds;
 import io.cdap.cdap.common.utils.ProjectInfo;
 import io.cdap.cdap.internal.AppFabricTestHelper;
+import io.cdap.cdap.internal.app.ApplicationSpecificationAdapter;
 import io.cdap.cdap.internal.app.DefaultApplicationSpecification;
 import io.cdap.cdap.internal.app.deploy.Specifications;
 import io.cdap.cdap.internal.app.runtime.SystemArguments;
@@ -40,8 +43,12 @@
 import io.cdap.cdap.proto.id.ProgramReference;
 import io.cdap.cdap.proto.id.ProgramRunId;
 import io.cdap.cdap.spi.data.SortOrder;
+import io.cdap.cdap.spi.data.StructuredTable;
+import io.cdap.cdap.spi.data.table.field.Field;
+import io.cdap.cdap.spi.data.table.field.Fields;
 import io.cdap.cdap.spi.data.transaction.TransactionRunner;
 import io.cdap.cdap.spi.data.transaction.TransactionRunners;
+import io.cdap.cdap.store.StoreDefinition;
 import java.io.IOException;
 import java.time.Instant;
 import java.util.ArrayList;
@@ -1365,7 +1372,7 @@ public void testConcurrentCreateAppFirstVersion() throws Exception {
     String appName = "application1";
     ArtifactId artifactId = NamespaceId.DEFAULT.artifact("testArtifact", "1.0").toApiArtifactId();
     ApplicationReference appRef = new ApplicationReference(NamespaceId.DEFAULT, appName);
-    
+
     // Concurrently deploy different fist version of the same application
     int numThreads = 10;
     AtomicInteger idGenerator = new AtomicInteger();
@@ -1378,7 +1385,7 @@ public void testConcurrentCreateAppFirstVersion() throws Exception {
         ApplicationMeta meta = new ApplicationMeta(spec.getName(), spec,
                                                    new ChangeDetail(null, null, null,
                                                                     creationTimeMillis + id));
-        metaStore.createApplicationVersion(appId, meta);
+        metaStore.createLatestApplicationVersion(appId, meta);
       })
     );
 
@@ -1409,7 +1416,7 @@ public void testConcurrentCreateAppFirstVersion() throws Exception {
       latestVersionCount.set(latestVersions.size());
       appEditNumber.set(metaStore.getApplicationEditNumber(appRef));
     });
-    
+
     // There can only be one latest version
     Assert.assertEquals(1, latestVersionCount.get());
     Assert.assertEquals(numThreads, allVersionsCount.get());
@@ -1432,7 +1439,7 @@ public void testConcurrentCreateAppAfterTheFirstVersion() throws Exception {
       ApplicationMeta meta = new ApplicationMeta(spec.getName(), spec,
                                                  new ChangeDetail(null, null, null,
                                                                   creationTimeMillis + id));
-      metaStore.createApplicationVersion(appId, meta);
+      metaStore.createLatestApplicationVersion(appId, meta);
     });
 
     // Concurrently deploy different versions of the same application
@@ -1446,7 +1453,7 @@ public void testConcurrentCreateAppAfterTheFirstVersion() throws Exception {
         ApplicationMeta meta = new ApplicationMeta(spec.getName(), spec,
                                                    new ChangeDetail(null, null, null,
                                                                     creationTimeMillis + id));
-        metaStore.createApplicationVersion(appId, meta);
+        metaStore.createLatestApplicationVersion(appId, meta);
       })
     );
 
@@ -1528,6 +1535,46 @@ public void testDeleteCompletedRunsStartedBefore() throws Exception {
         });
   }
 
+  /**
+   * Testcase for getting the latest application, where the application was deployed
+   * before 6.8.0 (where the latest column is not set).
+   * In this case, first insert a row in app spec table with the latest column set to null.
+   * This step is expected to fail in the NoSql implementation.
+   */
+  @Test
+  public void testGetLatestOnLegacyRows() throws Exception {
+    Gson GSON = ApplicationSpecificationAdapter.addTypeAdapters(new GsonBuilder()).create();
+    // insert a row in appspec table with latest column set to null
+    String appName = "legacy_app_without_latest";
+    String appVersion = ApplicationId.DEFAULT_VERSION;
+    ApplicationReference appRef = new ApplicationReference(NamespaceId.DEFAULT, appName);
+
+    ArtifactId artifactId = NamespaceId.DEFAULT.artifact("testArtifact", "1.0").toApiArtifactId();
+    ApplicationId appId = appRef.app(appVersion);
+    ApplicationSpecification spec = createDummyAppSpec(appId.getApplication(), appId.getVersion(), artifactId);
+    ApplicationMeta appMeta = new ApplicationMeta(appName, spec, null, null);
+
+    TransactionRunners.run(transactionRunner, context -> {
+      AppMetadataStore metaStore = AppMetadataStore.create(context);
+      metaStore.createLatestApplicationVersion(appId, appMeta);
+      StructuredTable appSpecTable = context.getTable(
+          StoreDefinition.AppMetadataStore.APPLICATION_SPECIFICATIONS);
+
+      List<Field<?>> fields = metaStore.getApplicationPrimaryKeys(
+          NamespaceId.DEFAULT.getNamespace(), appName, appVersion);
+      fields.add(Fields.booleanField(StoreDefinition.AppMetadataStore.LATEST_FIELD, null));
+      appSpecTable.upsert(fields);
+    });
+
+    ApplicationMeta latestAppMeta = TransactionRunners.run(transactionRunner, context -> {
+      AppMetadataStore metaStore = AppMetadataStore.create(context);
+      return metaStore.getLatest(appRef);
+    });
+
+    Assert.assertEquals(appName, latestAppMeta.getId());
+    Assert.assertEquals(appVersion, latestAppMeta.getSpec().getAppVersion());
+  }
+
   /**
    * Creates a new run of {@code programRunId} in the completed state with a starting time of {@code
    * startingTime} and returns its corresponding run id.