From 48239d4abe88cd810848e0ffe7ee97fa07ba46e4 Mon Sep 17 00:00:00 2001 From: Jun Nemoto Date: Mon, 25 Aug 2025 11:00:50 +0900 Subject: [PATCH 1/2] Fix to avoid using test-fixtures plugin --- client/build.gradle | 4 +- common-test/build.gradle | 49 +++++++++++++++++++ .../dl/client/tool/CommandLineTestUtils.java | 0 settings.gradle | 1 + table-store/build.gradle | 2 +- 5 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 common-test/build.gradle rename {client/src/testFixtures => common-test/src/main}/java/com/scalar/dl/client/tool/CommandLineTestUtils.java (100%) diff --git a/client/build.gradle b/client/build.gradle index 0d7a115b..a529fd2a 100644 --- a/client/build.gradle +++ b/client/build.gradle @@ -1,5 +1,4 @@ plugins { - id 'java-test-fixtures' id 'com.palantir.docker' version "${dockerPluginVersion}" id 'net.ltgt.errorprone' version "${errorpronePluginVersion}" id "com.github.spotbugs" version "${spotbugsPluginVersion}" @@ -35,8 +34,7 @@ dependencies { api group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: "${jacksonVersion}" // for tests - testFixturesImplementation group: 'info.picocli', name: 'picocli', version: "${picoCliVersion}" - testFixturesImplementation group: 'org.assertj', name: 'assertj-core', version: "${assertjVersion}" + testImplementation project(':common-test') // for Error Prone errorprone "com.google.errorprone:error_prone_core:${errorproneVersion}" diff --git a/common-test/build.gradle b/common-test/build.gradle new file mode 100644 index 00000000..f1779708 --- /dev/null +++ b/common-test/build.gradle @@ -0,0 +1,49 @@ +plugins { + id 'net.ltgt.errorprone' version "${errorpronePluginVersion}" + id "com.github.spotbugs" version "${spotbugsPluginVersion}" +} + +dependencies { + implementation project(':client') + implementation group: 'info.picocli', name: 'picocli', version: "${picoCliVersion}" + implementation group: 'org.assertj', name: 'assertj-core', version: "${assertjVersion}" + implementation group: 'org.junit.jupiter', name: 'junit-jupiter-api', version: "${junitVersion}" + runtimeOnly group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: "${junitVersion}" + + // for Error Prone + errorprone "com.google.errorprone:error_prone_core:${errorproneVersion}" + errorproneJavac "com.google.errorprone:javac:${errorproneJavacVersion}" + + // for SpotBugs + spotbugs "com.github.spotbugs:spotbugs:${spotbugsVersion}" + compileOnly "com.github.spotbugs:spotbugs-annotations:${spotbugsVersion}" + testCompileOnly "com.github.spotbugs:spotbugs-annotations:${spotbugsVersion}" +} + +spotless { + java { + target 'src/*/java/**/*.java' + importOrder() + removeUnusedImports() + googleJavaFormat('1.7') + } +} + +spotbugs { + ignoreFailures = false + showStackTraces = true + showProgress = true + effort = 'default' + reportLevel = 'default' + maxHeapSize = '1g' + extraArgs = [ '-nested:false' ] + jvmArgs = [ '-Duser.language=en' ] +} + +spotbugsMain.reports { + html.enabled = true +} + +spotbugsTest.reports { + html.enabled = true +} diff --git a/client/src/testFixtures/java/com/scalar/dl/client/tool/CommandLineTestUtils.java b/common-test/src/main/java/com/scalar/dl/client/tool/CommandLineTestUtils.java similarity index 100% rename from client/src/testFixtures/java/com/scalar/dl/client/tool/CommandLineTestUtils.java rename to common-test/src/main/java/com/scalar/dl/client/tool/CommandLineTestUtils.java diff --git a/settings.gradle b/settings.gradle index 837da0bc..ac4da37c 100644 --- a/settings.gradle +++ b/settings.gradle @@ -5,3 +5,4 @@ include 'client' include 'schema-loader' include 'generic-contracts' include 'table-store' +include 'common-test' diff --git a/table-store/build.gradle b/table-store/build.gradle index dc3cac8e..d9f91e07 100644 --- a/table-store/build.gradle +++ b/table-store/build.gradle @@ -14,7 +14,7 @@ dependencies { implementation group: 'org.partiql', name: 'partiql-parser', version: "${partiqlVersion}" // for test - testImplementation testFixtures(project(':client')) // to use CommandLineTestUtils + testImplementation project(':common-test') // for Error Prone errorprone "com.google.errorprone:error_prone_core:${errorproneVersion}" From 4d1bda6e75d41de54ee939fda54e5bf86cecaa65 Mon Sep 17 00:00:00 2001 From: Jun Nemoto Date: Mon, 25 Aug 2025 11:54:05 +0900 Subject: [PATCH 2/2] Fix IS_NULL and IS_NOT_NULL conditions handling --- .../genericcontracts/table/v1_0_0/Scan.java | 14 ++++-- .../table/v1_0_0/ScanTest.java | 46 +++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/generic-contracts/src/main/java/com/scalar/dl/genericcontracts/table/v1_0_0/Scan.java b/generic-contracts/src/main/java/com/scalar/dl/genericcontracts/table/v1_0_0/Scan.java index 8353461e..ef71b0b2 100644 --- a/generic-contracts/src/main/java/com/scalar/dl/genericcontracts/table/v1_0_0/Scan.java +++ b/generic-contracts/src/main/java/com/scalar/dl/genericcontracts/table/v1_0_0/Scan.java @@ -179,21 +179,25 @@ private boolean hasIndexKeyCondition( } private boolean isPrimaryKeyCondition(JsonNode condition, String keyType) { + String operator = condition.get(Constants.CONDITION_OPERATOR).textValue().toUpperCase(); + if (!operator.equals(Constants.OPERATOR_EQ)) { + return false; + } + String givenType = condition.get(Constants.CONDITION_VALUE).getNodeType().name(); if (!givenType.equalsIgnoreCase(keyType)) { throw new ContractContextException(Constants.INVALID_KEY_TYPE + givenType); } - return condition - .get(Constants.CONDITION_OPERATOR) - .textValue() - .equalsIgnoreCase(Constants.OPERATOR_EQ); + return true; } private boolean isIndexKeyCondition(JsonNode condition, String keyType) { String operator = condition.get(Constants.CONDITION_OPERATOR).textValue().toUpperCase(); if (operator.equals(Constants.OPERATOR_IS_NULL)) { return true; + } else if (!operator.equals(Constants.OPERATOR_EQ)) { + return false; } String givenType = condition.get(Constants.CONDITION_VALUE).getNodeType().name(); @@ -201,7 +205,7 @@ private boolean isIndexKeyCondition(JsonNode condition, String keyType) { throw new ContractContextException(Constants.INVALID_INDEX_KEY_TYPE + givenType); } - return operator.equals(Constants.OPERATOR_EQ); + return true; } private JsonNode getKeyColumnCondition( diff --git a/generic-contracts/src/test/java/com/scalar/dl/genericcontracts/table/v1_0_0/ScanTest.java b/generic-contracts/src/test/java/com/scalar/dl/genericcontracts/table/v1_0_0/ScanTest.java index 6b40d56e..26cff130 100644 --- a/generic-contracts/src/test/java/com/scalar/dl/genericcontracts/table/v1_0_0/ScanTest.java +++ b/generic-contracts/src/test/java/com/scalar/dl/genericcontracts/table/v1_0_0/ScanTest.java @@ -920,6 +920,52 @@ public void invoke_ArgumentsWithInvalidIndexKeyConditionGiven_ShouldThrowExcepti verify(ledger).get(SOME_TABLE_ASSET_ID); } + @Test + public void invoke_ArgumentsWithOnlyIsNullConditionForPrimaryKeyGiven_ShouldThrowException() { + // Arrange + ArrayNode conditions = + mapper + .createArrayNode() + .add(createCondition(SOME_PRIMARY_KEY_COLUMN, Constants.OPERATOR_IS_NULL)); + JsonNode argument = + mapper + .createObjectNode() + .put(Constants.QUERY_TABLE, SOME_TABLE_NAME) + .set(Constants.QUERY_CONDITIONS, conditions); + Asset table = createAsset(SOME_TABLE); + when(ledger.get(SOME_TABLE_ASSET_ID)).thenReturn(Optional.of(table)); + prepareTableAssetId(SOME_TABLE_NAME); + + // Act Assert + assertThatThrownBy(() -> scan.invoke(ledger, argument, null)) + .isExactlyInstanceOf(ContractContextException.class) + .hasMessageContaining(Constants.INVALID_KEY_SPECIFICATION); + verify(ledger).get(SOME_TABLE_ASSET_ID); + } + + @Test + public void invoke_ArgumentsWithOnlyIsNotNullConditionForIndexKeyGiven_ShouldThrowException() { + // Arrange + ArrayNode conditions = + mapper + .createArrayNode() + .add(createCondition(SOME_INDEX_KEY_COLUMN_1, Constants.OPERATOR_IS_NOT_NULL)); + JsonNode argument = + mapper + .createObjectNode() + .put(Constants.QUERY_TABLE, SOME_TABLE_NAME) + .set(Constants.QUERY_CONDITIONS, conditions); + Asset table = createAsset(SOME_TABLE); + when(ledger.get(SOME_TABLE_ASSET_ID)).thenReturn(Optional.of(table)); + prepareTableAssetId(SOME_TABLE_NAME); + + // Act Assert + assertThatThrownBy(() -> scan.invoke(ledger, argument, null)) + .isExactlyInstanceOf(ContractContextException.class) + .hasMessageContaining(Constants.INVALID_KEY_SPECIFICATION); + verify(ledger).get(SOME_TABLE_ASSET_ID); + } + @Test public void invoke_InvalidConditionsGiven_ShouldThrowException() { // Arrange