Skip to content

Commit

Permalink
Fix the null pointer exception caused by unsafe unboxing of Boolean t…
Browse files Browse the repository at this point in the history 
…o boolean
  • Loading branch information
@GnsP
GnsP committed Nov 8, 2023
1 parent 6bb2566 commit 3872346
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 16 deletions.
35 changes: 21 additions & 14 deletions cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/store/AppMetadataStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -414,7 +413,8 @@ public ApplicationMeta getLatest(ApplicationReference appReference) throws IOExc
// the -SNAPSHOT version as the latest.
List<Field<?>> fields = getApplicationPrimaryKeys(
appReference.app(ApplicationId.DEFAULT_VERSION));
ApplicationMeta appMeta = appSpecTable.read(fields).map(this::decodeRow).orElse(null);
ApplicationMeta appMeta = appSpecTable.read(fields).map(row -> decodeRow(row, true))
.orElse(null);
if (appMeta != null) {
return appMeta;
}
Expand All @@ -425,7 +425,7 @@ public ApplicationMeta getLatest(ApplicationReference appReference) throws IOExc
StoreDefinition.AppMetadataStore.VERSION_FIELD,
SortOrder.DESC)) {
if (iterator.hasNext()) {
return decodeRow(iterator.next());
return decodeRow(iterator.next(), true);
}
}
// This is the case when the app currently doesn't exist
Expand Down Expand Up @@ -700,6 +700,16 @@ void writeApplication(String namespaceId, String appId, String versionId,
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.
*
Expand Down Expand Up @@ -2478,15 +2488,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<>();
Expand Down Expand Up @@ -2624,7 +2625,7 @@ private <T> List<T> scanWithRange(Range range, Type typeofT, StructuredTable tab
return result;
}

private ApplicationMeta decodeRow(StructuredRow row) {
private ApplicationMeta decodeRow(StructuredRow row, boolean isLatest) {
String author = row.getString(StoreDefinition.AppMetadataStore.AUTHOR_FIELD);
String changeSummary = row.getString(StoreDefinition.AppMetadataStore.CHANGE_SUMMARY_FIELD);
Long creationTimeMillis = row.getLong(StoreDefinition.AppMetadataStore.CREATION_TIME_FIELD);
Expand All @@ -2644,7 +2645,13 @@ 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,
isLatest || Boolean.TRUE.equals(latest));
}

private ApplicationMeta decodeRow(StructuredRow row) {
return decodeRow(row, false);
}

private void writeToStructuredTableWithPrimaryKeys(
Expand Down
52 changes: 50 additions & 2 deletions cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/AppMetadataStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -1528,6 +1535,47 @@ 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, false);

TransactionRunners.run(transactionRunner, context -> {
AppMetadataStore metaStore = AppMetadataStore.create(context);
metaStore.createApplicationVersion(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());
Assert.assertTrue(latestAppMeta.getIsLatest());
}

/**
* Creates a new run of {@code programRunId} in the completed state with a starting time of {@code
* startingTime} and returns its corresponding run id.
Expand Down
10 changes: 10 additions & 0 deletions cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/store/NoSqlAppMetadataStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

0 comments on commit 3872346

Please sign in to comment.