From 40d7c37142b99385670101466acdf211d8ae03e2 Mon Sep 17 00:00:00 2001 From: GnsP Date: Wed, 8 Nov 2023 10:17:19 +0530 Subject: [PATCH] Fix the null pointer exception caused by unsafe unboxing of Boolean to boolean --- .../java/io/cdap/cdap/app/store/Store.java | 16 +++- .../ApplicationRegistrationStage.java | 4 +- .../internal/app/store/AppMetadataStore.java | 46 ++++++--- .../internal/app/store/ApplicationMeta.java | 17 +--- .../cdap/internal/app/store/DefaultStore.java | 11 ++- .../OperationsDashboardHttpHandlerTest.java | 4 +- .../PreferencesHttpHandlerInternalTest.java | 2 +- .../handlers/PreferencesHttpHandlerTest.java | 2 +- .../app/store/AppMetadataStoreTest.java | 57 ++++++++++- .../internal/app/store/DefaultStoreTest.java | 94 ++++++++++--------- .../app/store/NoSqlAppMetadataStoreTest.java | 10 ++ .../internal/profile/ProfileServiceTest.java | 2 +- .../cdap/cdap/metadata/LineageAdminTest.java | 6 +- .../MetadataSubscriberServiceTest.java | 4 +- 14 files changed, 179 insertions(+), 96 deletions(-) diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/app/store/Store.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/app/store/Store.java index d8a00979076b..a979c3ac1098 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/app/store/Store.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/app/store/Store.java @@ -359,6 +359,7 @@ List getRuns(Collection 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 @@ -366,7 +367,20 @@ List getRuns(Collection programs, ProgramRunSt * @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. diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/deploy/pipeline/ApplicationRegistrationStage.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/deploy/pipeline/ApplicationRegistrationStage.java index c58b32f7b3f2..2b4f8021d70d 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/deploy/pipeline/ApplicationRegistrationStage.java +++ b/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] diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/AppMetadataStore.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/AppMetadataStore.java index a84d54c838ea..5b93e1fb0c74 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/AppMetadataStore.java +++ b/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> getApplicationPrimaryKeys(String namespaceId, String appId, + String versionId) { + List> 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> getNamespaceApplicationKeys(ApplicationReference appRef) return fields; } - private List> getApplicationPrimaryKeys(String namespaceId, String appId, - String versionId) { - List> 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> getApplicationNamespaceAppCreationKeys(ApplicationId appId) throws IOException { List> 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( diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/ApplicationMeta.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/ApplicationMeta.java index 56e8a0cac078..0586c2df2179 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/ApplicationMeta.java +++ b/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,10 +67,6 @@ public SourceControlMeta getSourceControlMeta() { return sourceControlMeta; } - public boolean getIsLatest() { - return isLatest; - } - @Override public String toString() { return Objects.toStringHelper(this) @@ -88,7 +74,6 @@ public String toString() { .add("spec", ADAPTER.toJson(spec)) .add("change", change) .add("sourceControlMeta", sourceControlMeta) - .add("isLatest", isLatest) .toString(); } } diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java index 3b059a1c97e5..a35340f26445 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/DefaultStore.java @@ -594,9 +594,16 @@ public void markApplicationsLatest(Collection 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); } diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/gateway/handlers/OperationsDashboardHttpHandlerTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/gateway/handlers/OperationsDashboardHttpHandlerTest.java index 5f3105da0951..0544abe726d8 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/gateway/handlers/OperationsDashboardHttpHandlerTest.java +++ b/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); } /** diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/http/handlers/PreferencesHttpHandlerInternalTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/http/handlers/PreferencesHttpHandlerInternalTest.java index 18c92143d9c0..13cc80c823fa 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/http/handlers/PreferencesHttpHandlerInternalTest.java +++ b/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 diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/http/handlers/PreferencesHttpHandlerTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/http/handlers/PreferencesHttpHandlerTest.java index 0da55d73ffcc..13d4e68254a1 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/http/handlers/PreferencesHttpHandlerTest.java +++ b/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 diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/AppMetadataStoreTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/AppMetadataStoreTest.java index 6854dea6e9c5..7fe5232ee566 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/AppMetadataStoreTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/AppMetadataStoreTest.java @@ -19,6 +19,8 @@ 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; @@ -26,6 +28,7 @@ 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> 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. diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/DefaultStoreTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/DefaultStoreTest.java index de72260447c1..ec8f8574bd64 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/DefaultStoreTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/DefaultStoreTest.java @@ -159,7 +159,7 @@ public void testLoadingProgram() throws Exception { ApplicationMeta appMeta = new ApplicationMeta(appSpec.getName(), appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); ProgramDescriptor descriptor = store.loadProgram(appId.mr("mrJob1")); Assert.assertNotNull(descriptor); @@ -494,7 +494,7 @@ public void testAddApplication() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta("application1", spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); spec = store.getApplication(appId); Assert.assertNotNull(spec); @@ -507,12 +507,12 @@ public void testUpdateChangedApplication() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta("application1", Specifications.from(new FooApp()), new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(id, appMeta); + store.addLatestApplication(id, appMeta); // update ApplicationMeta appMetaUpdate = new ApplicationMeta("application1", Specifications.from(new ChangedFooApp()), new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(id, appMetaUpdate); + store.addLatestApplication(id, appMetaUpdate); ApplicationSpecification spec = store.getApplication(id); Assert.assertNotNull(spec); @@ -523,15 +523,18 @@ public void testUpdateChangedApplication() throws ConflictException { public void testAddApplicationWithoutMarkingLatest() throws ConflictException { long creationTime = System.currentTimeMillis(); - ApplicationId appId = new ApplicationId("account1", "app1"); - ApplicationMeta appMeta = new ApplicationMeta("app1", Specifications.from(new FooApp()), - new ChangeDetail(null, null, null, creationTime), null, false); - store.addApplication(appId, appMeta); + String appName = "notLatestApp1"; + ApplicationId appId = new ApplicationId("account1", appName, "v1"); + ApplicationMeta appMeta = new ApplicationMeta(appName, Specifications.from(new FooApp()), + new ChangeDetail(null, null, null, creationTime), null); + store.addApplication(appId, appMeta, false); ApplicationMeta storedMeta = store.getApplicationMetadata(appId); - Assert.assertEquals("app1", storedMeta.getId()); + Assert.assertEquals(appName, storedMeta.getId()); Assert.assertEquals(creationTime, storedMeta.getChange().getCreationTimeMillis()); - Assert.assertFalse(storedMeta.getIsLatest()); + + ApplicationMeta latestMeta = store.getLatest(appId.getAppReference()); + Assert.assertNull(latestMeta); } @Test @@ -541,26 +544,24 @@ public void testMarkApplicationsLatestWithNewApps() // Add 2 new applications without marking them latest ApplicationId appId1 = new ApplicationId("account1", "newApp1"); ApplicationMeta appMeta1 = new ApplicationMeta("newApp1", Specifications.from(new FooApp()), - new ChangeDetail(null, null, null, creationTime), null, false); - store.addApplication(appId1, appMeta1); + new ChangeDetail(null, null, null, creationTime), null); + store.addApplication(appId1, appMeta1, false); ApplicationId appId2 = new ApplicationId("account1", "newApp2"); ApplicationMeta appMeta2 = new ApplicationMeta("newApp2", Specifications.from(new FooApp()), - new ChangeDetail(null, null, null, creationTime), null, false); - store.addApplication(appId2, appMeta2); + new ChangeDetail(null, null, null, creationTime), null); + store.addApplication(appId2, appMeta2, false); // Now mark them as latest in bulk store.markApplicationsLatest(Arrays.asList(appId1, appId2)); - ApplicationMeta storedMeta1 = store.getApplicationMetadata(appId1); + ApplicationMeta storedMeta1 = store.getLatest(appId1.getAppReference()); Assert.assertEquals("newApp1", storedMeta1.getId()); Assert.assertEquals(creationTime, storedMeta1.getChange().getCreationTimeMillis()); - Assert.assertTrue(storedMeta1.getIsLatest()); - ApplicationMeta storedMeta2 = store.getApplicationMetadata(appId2); + ApplicationMeta storedMeta2 = store.getLatest(appId2.getAppReference()); Assert.assertEquals("newApp2", storedMeta2.getId()); Assert.assertEquals(creationTime, storedMeta2.getChange().getCreationTimeMillis()); - Assert.assertTrue(storedMeta2.getIsLatest()); } @Test @@ -576,28 +577,29 @@ public void testMarkApplicationsLatestWithExistingLatest() ApplicationId appIdV1 = new ApplicationId("account1", appName, oldVersion); ApplicationMeta appMetaV1 = new ApplicationMeta(appName, Specifications.from(new FooApp(), appName, oldVersion), new ChangeDetail(null, null, null, creationTime)); - store.addApplication(appIdV1, appMetaV1); + store.addLatestApplication(appIdV1, appMetaV1); // Add a new version of the application without marking latest ApplicationId appIdV2 = new ApplicationId("account1", appName, newVersion); ApplicationMeta appMetaV2 = new ApplicationMeta(appName, Specifications.from(new FooApp(), appName, newVersion), - new ChangeDetail(null, null, null, v2CreationTime), null, false); - store.addApplication(appIdV2, appMetaV2); + new ChangeDetail(null, null, null, v2CreationTime), null); + store.addApplication(appIdV2, appMetaV2, false); // Now mark the new version as latest store.markApplicationsLatest(Collections.singletonList(appIdV2)); + ApplicationMeta latestMeta = store.getLatest(appIdV1.getAppReference()); ApplicationMeta storedMetaV1 = store.getApplicationMetadata(appIdV1); Assert.assertEquals(appName, storedMetaV1.getId()); Assert.assertEquals(oldVersion, storedMetaV1.getSpec().getAppVersion()); Assert.assertEquals(creationTime, storedMetaV1.getChange().getCreationTimeMillis()); - Assert.assertFalse(storedMetaV1.getIsLatest()); + Assert.assertNotEquals(latestMeta.getSpec().getAppVersion(), storedMetaV1.getSpec().getAppVersion()); ApplicationMeta storedMetaV2 = store.getApplicationMetadata(appIdV2); Assert.assertEquals(appName, storedMetaV2.getId()); Assert.assertEquals(newVersion, storedMetaV2.getSpec().getAppVersion()); Assert.assertEquals(v2CreationTime, storedMetaV2.getChange().getCreationTimeMillis()); - Assert.assertTrue(storedMetaV2.getIsLatest()); + Assert.assertEquals(latestMeta.getSpec().getAppVersion(), storedMetaV2.getSpec().getAppVersion()); } @Test(expected = ApplicationNotFoundException.class) @@ -620,7 +622,7 @@ public void testUpdateApplicationScmMeta() new ChangeDetail(null, null, null, System.currentTimeMillis()), null); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); Map updateRequests = new HashMap<>(); updateRequests.put(appId, new SourceControlMeta("updated-file-hash")); store.updateApplicationSourceControlMeta(updateRequests); @@ -640,7 +642,7 @@ public void testUpdateApplicationScmMetaWithNonExistingAppIds() new ChangeDetail(null, null, null, System.currentTimeMillis()), new SourceControlMeta("initial-file-hash")); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); // The following appId is not added to the store ApplicationId appId2 = new ApplicationId("account1", "application2"); @@ -708,7 +710,7 @@ public void testServiceDeletion() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(appSpec.getName(), appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); AbstractApplication newApp = new AppWithNoServices(); @@ -728,7 +730,7 @@ public void testServiceInstances() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(appSpec.getName(), appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); // Test setting of service instances ProgramId programId = appId.program(ProgramType.SERVICE, "NoOpService"); @@ -755,7 +757,7 @@ public void testWorkerInstances() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); ProgramId programId = appId.worker(AppWithWorker.WORKER); int instancesFromSpec = spec.getWorkers().get(AppWithWorker.WORKER).getInstances(); @@ -776,7 +778,7 @@ public void testRemoveAll() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); Assert.assertNotNull(store.getApplication(appId)); @@ -794,7 +796,7 @@ public void testRemoveApplication() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); Assert.assertNotNull(store.getApplication(appId)); @@ -818,7 +820,7 @@ public void testProgramRunCount() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); // add some run records to workflow and service for (int i = 0; i < 5; i++) { @@ -864,7 +866,7 @@ public void testRuntimeArgsDeletion() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); Assert.assertNotNull(store.getApplication(appId)); @@ -916,14 +918,14 @@ public void testHistoryDeletion() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId1, appMeta); + store.addLatestApplication(appId1, appMeta); spec = Specifications.from(new AppWithServices()); ApplicationId appId2 = namespaceId.app(spec.getName()); appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId2, appMeta); + store.addLatestApplication(appId2, appMeta); ArtifactId artifactId = appId1.getNamespaceId().artifact("testArtifact", "1.0").toApiArtifactId(); @@ -986,7 +988,7 @@ public void testRunsLimit() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); ProgramId mapreduceProgramId = new ApplicationId("testRunsLimit", spec.getName()) .mr(AllProgramsApp.NoOpMR.class.getSimpleName()); @@ -1017,7 +1019,7 @@ public void testCheckDeletedProgramSpecs() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); Set specsToBeVerified = Sets.newHashSet(); specsToBeVerified.addAll(spec.getMapReduce().keySet()); @@ -1063,7 +1065,7 @@ protected void testScanApplications(Store store) throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(appName, appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(new ApplicationId(NamespaceId.DEFAULT.getNamespace(), appName), appMeta); + store.addLatestApplication(new ApplicationId(NamespaceId.DEFAULT.getNamespace(), appName), appMeta); } // Mimicking editing the first count / 2 apps @@ -1073,7 +1075,7 @@ protected void testScanApplications(Store store) throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(appName, appSpec, new ChangeDetail("edited" + i, null, null, System.currentTimeMillis())); - store.addApplication(new ApplicationId(NamespaceId.DEFAULT.getNamespace(), appName, version), appMeta); + store.addLatestApplication(new ApplicationId(NamespaceId.DEFAULT.getNamespace(), appName, version), appMeta); } List allAppsVersion = new ArrayList<>(); @@ -1118,9 +1120,9 @@ public void testScanApplicationsWithNamespace(Store store) throws ConflictExcept ApplicationMeta appMeta = new ApplicationMeta(appName, appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(new ApplicationId(NamespaceId.DEFAULT.getNamespace(), appName), appMeta); + store.addLatestApplication(new ApplicationId(NamespaceId.DEFAULT.getNamespace(), appName), appMeta); appName = "test" + (2 * i + 1); - store.addApplication(new ApplicationId(NamespaceId.CDAP.getNamespace(), appName), appMeta); + store.addLatestApplication(new ApplicationId(NamespaceId.CDAP.getNamespace(), appName), appMeta); } List apps = new ArrayList(); @@ -1185,7 +1187,7 @@ public void testCheckDeletedWorkflow() throws ConflictException { ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); Set specsToBeDeleted = Sets.newHashSet(); specsToBeDeleted.addAll(spec.getWorkflows().keySet()); @@ -1330,7 +1332,7 @@ public void testStateRemovedOnRemoveApplication() throws ApplicationNotFoundExce ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); store.saveState(new AppStateKeyValue(namespaceId, spec.getName(), stateKey, stateValue)); Assert.assertNotNull(store.getApplication(appId)); @@ -1357,7 +1359,7 @@ public void testStateRemovedOnRemoveAll() throws ApplicationNotFoundException, C ApplicationMeta appMeta = new ApplicationMeta(spec.getName(), spec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); store.saveState(new AppStateKeyValue(namespaceId, appName, stateKey, stateValue)); Assert.assertNotNull(store.getApplication(appId)); @@ -1388,7 +1390,7 @@ public void testListRunsWithLegacyRows() throws ConflictException { List expectedApps = new ArrayList<>(); // Insert a row that is null for changeDetail - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); expectedApps.add(appId); ApplicationId newVersionAppId = appId.getAppReference().app("new_version"); @@ -1399,7 +1401,7 @@ public void testListRunsWithLegacyRows() throws ConflictException { new ChangeDetail(null, null, null, currentTime)); // Insert a second version - store.addApplication(newVersionAppId, newAppMeta); + store.addLatestApplication(newVersionAppId, newAppMeta); expectedApps.add(newVersionAppId); // Insert a third version @@ -1408,7 +1410,7 @@ public void testListRunsWithLegacyRows() throws ConflictException { ApplicationMeta anotherAppMeta = new ApplicationMeta(anotherVersionAppId.getApplication(), spec, new ChangeDetail(null, null, null, currentTime + 1000)); - store.addApplication(anotherVersionAppId, anotherAppMeta); + store.addLatestApplication(anotherVersionAppId, anotherAppMeta); expectedApps.add(anotherVersionAppId); // Reverse it because we want DESC order diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/NoSqlAppMetadataStoreTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/NoSqlAppMetadataStoreTest.java index c278f5ccff6e..a0c0d6b4571a 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/NoSqlAppMetadataStoreTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/NoSqlAppMetadataStoreTest.java @@ -50,4 +50,14 @@ public void testScanApplicationsReverse() { public void testScanApplicationsWithNamespaceReverse() { super.testScanApplicationsWithNamespaceReverse(); } + + /** + * This testcase tries to create a row in the app spec table, with the latest column set + * to null (To test the getLatest method for applications deployed before v6.8.0). Setting + * a Boolean column to null in the NoSqlStructuredTable is expected to throw a NullPointerException. + */ + @Override @Test(expected = NullPointerException.class) + public void testGetLatestOnLegacyRows() throws Exception { + super.testGetLatestOnLegacyRows(); + } } diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/profile/ProfileServiceTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/profile/ProfileServiceTest.java index a9b62aabbef1..31853154e8fe 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/profile/ProfileServiceTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/profile/ProfileServiceTest.java @@ -354,7 +354,7 @@ public void testProfileDeletion() throws Exception { ApplicationMeta appMeta = new ApplicationMeta(appSpec.getName(), appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, appMeta); + store.addLatestApplication(appId, appMeta); ProgramId programId = NamespaceId.DEFAULT.app(appSpec.getName()).workflow(SampleWorkflow.NAME); ArtifactId artifactId = NamespaceId.DEFAULT.artifact("testArtifact", "1.0").toApiArtifactId(); diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/metadata/LineageAdminTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/metadata/LineageAdminTest.java index dcb9f1c8e7a6..3c08eb81beca 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/metadata/LineageAdminTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/metadata/LineageAdminTest.java @@ -495,7 +495,7 @@ public void testWorkflowLineage() throws ConflictException { ApplicationMeta meta = new ApplicationMeta(appSpec.getName(), appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(testApp, meta); + store.addLatestApplication(testApp, meta); LineageAdmin lineageAdmin = new LineageAdmin(lineageReader, store); // Add accesses for D3 -> P2 -> D2 -> P1 -> D1 <-> P3 @@ -602,7 +602,7 @@ public void testWorkflowLineage() throws ConflictException { new Relation(dataset1, program3, AccessType.UNKNOWN, twillRunId(run3)) ), oneLevelLineage.getRelations()); - + // Assert that in a different namespace both lineage and metadata should be empty NamespaceId customNamespace = new NamespaceId("custom_namespace"); DatasetId customDataset1 = customNamespace.dataset(dataset1.getEntityName()); @@ -671,7 +671,7 @@ public void testLocalDatasetsInWorkflow() throws Exception { ApplicationMeta meta = new ApplicationMeta(appSpec.getName(), appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(testApp, meta); + store.addLatestApplication(testApp, meta); LineageAdmin lineageAdmin = new LineageAdmin(lineageReader, store); // Add accesses for D1 -| diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/metadata/MetadataSubscriberServiceTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/metadata/MetadataSubscriberServiceTest.java index 63e80d37d372..a61d28a4f9f8 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/metadata/MetadataSubscriberServiceTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/metadata/MetadataSubscriberServiceTest.java @@ -447,7 +447,7 @@ public void testProfileMetadata() throws Exception { ApplicationMeta meta = new ApplicationMeta(appSpec.getName(), appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, meta); + store.addLatestApplication(appId, meta); // set default namespace to use the profile, since now MetadataSubscriberService is not started, // it should not affect the mds @@ -591,7 +591,7 @@ public void testProfileMetadataWithNoProfilePreferences() throws Exception { ApplicationMeta meta = new ApplicationMeta(appSpec.getName(), appSpec, new ChangeDetail(null, null, null, System.currentTimeMillis())); - store.addApplication(appId, meta); + store.addLatestApplication(appId, meta); // set default namespace to use the profile, since now MetadataSubscriberService is not started, // it should not affect the mds