Add parameter validation for test case parameter names (#22493)

* Add parameter validation for test case parameter names

Ensure that parameter names in test case parameterValues match the names
defined in the test definition parameterDefinition. This addresses issue
#10623 by preventing test cases from being created with invalid parameter names.

- Enhanced validateTestParameters method with parameter name validation
- Added comprehensive error messaging
- Added test coverage for the new validation logic

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Teddy <TeddyCr@users.noreply.github.com>

* fix: test case + GX exception

* fix: breaking tests

* fix: failing test

---------

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Teddy <TeddyCr@users.noreply.github.com>
This commit is contained in:
Teddy 2025-07-25 09:01:36 +02:00 committed by GitHub
parent 66dfcdb764
commit 11ec377245
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 103 additions and 30 deletions

View File

@ -109,4 +109,26 @@ yarn parse-schema # Parse JSON schemas for frontend
- JWT-based authentication with OAuth2/SAML support
- Role-based access control defined in Java entities
- Security configurations in `conf/openmetadata.yaml`
- Never commit secrets - use environment variables or secure vaults
- Never commit secrets - use environment variables or secure vaults
## Code Generation Standards
### Comments Policy
- **Do NOT add unnecessary comments** - write self-documenting code
- Only include comments for:
- Complex business logic that isn't obvious
- Non-obvious algorithms or workarounds
- Public API JavaDoc documentation
- TODO/FIXME with ticket references
- Avoid obvious comments like `// increment counter` or `// create new user`
### Java Code Requirements
- **Always mention** running `mvn spotless:apply` when generating/modifying .java files
- Use clear, descriptive variable and method names instead of comments
- Follow existing project patterns and conventions
- Generate production-ready code, not tutorial code
### Response Format
- Provide clean code blocks without unnecessary explanations
- Assume readers are experienced developers
- Focus on functionality over education

View File

@ -26,12 +26,15 @@ import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.stream.Collectors;
import lombok.SneakyThrows;
import lombok.extern.slf4j.Slf4j;
import org.jdbi.v3.sqlobject.transaction.Transaction;
import org.jetbrains.annotations.NotNull;
import org.openmetadata.schema.EntityInterface;
import org.openmetadata.schema.EntityTimeSeriesInterface;
import org.openmetadata.schema.api.feed.CloseTask;
@ -44,6 +47,7 @@ import org.openmetadata.schema.tests.TestCaseParameter;
import org.openmetadata.schema.tests.TestCaseParameterValidationRule;
import org.openmetadata.schema.tests.TestCaseParameterValue;
import org.openmetadata.schema.tests.TestDefinition;
import org.openmetadata.schema.tests.TestPlatform;
import org.openmetadata.schema.tests.TestSuite;
import org.openmetadata.schema.tests.type.Resolved;
import org.openmetadata.schema.tests.type.TestCaseFailureReasonType;
@ -427,7 +431,11 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
Entity.getEntity(test.getTestDefinition(), "", Include.NON_DELETED);
test.setTestDefinition(testDefinition.getEntityReference());
validateTestParameters(test.getParameterValues(), testDefinition.getParameterDefinition());
validateTestParameters(
test.getParameterValues(),
testDefinition.getParameterDefinition(),
testDefinition.getTestPlatforms(),
testDefinition);
validateColumnTestCase(entityLink, testDefinition.getEntityType());
}
@ -500,30 +508,57 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
}
private void validateTestParameters(
List<TestCaseParameterValue> parameterValues, List<TestCaseParameter> parameterDefinition) {
List<TestCaseParameterValue> parameterValues,
List<TestCaseParameter> parameterDefinition,
List<TestPlatform> testPlatforms,
TestDefinition testDefinition) {
if (parameterValues != null) {
if (parameterDefinition.isEmpty() && !parameterValues.isEmpty()) {
throw new IllegalArgumentException(
"Parameter Values doesn't match Test Definition Parameters");
}
Map<String, Object> values = new HashMap<>();
for (TestCaseParameterValue testCaseParameterValue : parameterValues) {
values.put(testCaseParameterValue.getName(), testCaseParameterValue.getValue());
}
for (TestCaseParameter parameter : parameterDefinition) {
if (Boolean.TRUE.equals(parameter.getRequired())
&& (!values.containsKey(parameter.getName())
|| values.get(parameter.getName()) == null)) {
throw new IllegalArgumentException(
"Required parameter " + parameter.getName() + " is not passed in parameterValues");
if (!testPlatforms.contains(TestPlatform.GREAT_EXPECTATIONS)
&& !testDefinition.getName().equals("tableDiff")) {
Set<String> definedParameterNames =
parameterDefinition.stream()
.map(TestCaseParameter::getName)
.collect(Collectors.toSet());
Map<String, Object> values = getParameterValuesMap(parameterValues, definedParameterNames);
for (TestCaseParameter parameter : parameterDefinition) {
if (Boolean.TRUE.equals(parameter.getRequired())
&& (!values.containsKey(parameter.getName())
|| values.get(parameter.getName()) == null)) {
throw new IllegalArgumentException(
"Required parameter " + parameter.getName() + " is not passed in parameterValues");
}
validateParameterRule(parameter, values);
}
validateParameterRule(parameter, values);
}
}
}
private @NotNull Map<String, Object> getParameterValuesMap(
List<TestCaseParameterValue> parameterValues, Set<String> definedParameterNames) {
Map<String, Object> values = new HashMap<>();
for (TestCaseParameterValue testCaseParameterValue : parameterValues) {
String parameterName = testCaseParameterValue.getName();
if (!definedParameterNames.contains(parameterName)) {
throw new IllegalArgumentException(
String.format(
"Parameter '%s' is not defined in the test definition. Defined parameters are: %s",
parameterName, definedParameterNames));
}
values.put(parameterName, testCaseParameterValue.getValue());
}
return values;
}
private void validateColumnTestCase(
EntityLink entityLink, TestDefinitionEntityType testDefinitionEntityType) {
if (testDefinitionEntityType.equals(TestDefinitionEntityType.COLUMN)) {

View File

@ -516,6 +516,19 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
() -> createAndCheckEntity(create1, ADMIN_AUTH_HEADERS),
BAD_REQUEST,
"Required parameter missingCountValue is not passed in parameterValues");
CreateTestCase create2 = createRequest(test);
create2
.withEntityLink(TABLE_LINK)
.withTestDefinition(TEST_DEFINITION3.getFullyQualifiedName())
.withParameterValues(
List.of(
new TestCaseParameterValue().withName("missingCountValue").withValue("10"),
new TestCaseParameterValue().withName("invalidParameter").withValue("20")));
assertResponseContains(
() -> createAndCheckEntity(create2, ADMIN_AUTH_HEADERS),
BAD_REQUEST,
"Parameter 'invalidParameter' is not defined in the test definition. Defined parameters are: [missingValueMatch, missingCountValue]");
}
@Test
@ -526,7 +539,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(TABLE_LINK)
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("100").withName("min")));
List.of(new TestCaseParameterValue().withValue("100").withName("minValue")));
TestCase testCase = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
// Change the test with PUT request
@ -555,7 +568,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(TABLE_LINK)
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("100").withName("max")));
List.of(new TestCaseParameterValue().withValue("100").withName("maxValue")));
TestCase testCase = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
CreateTestCaseResult createTestCaseResult =
new CreateTestCaseResult()
@ -694,7 +707,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(TABLE_LINK_2)
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("100").withName("max")));
List.of(new TestCaseParameterValue().withValue("100").withName("maxValue")));
TestCase testCase = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
expectedTestCaseList.add(create);
CreateTestCase create1 =
@ -702,7 +715,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(TABLE_LINK_2)
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("20").withName("max")));
List.of(new TestCaseParameterValue().withValue("20").withName("maxValue")));
createAndCheckEntity(create1, ADMIN_AUTH_HEADERS);
expectedTestCaseList.add(create1);
Map<String, Object> queryParams = new HashMap<>();
@ -816,7 +829,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(String.format("<#E::table::%s>", tableFQN))
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("20").withName("min")));
List.of(new TestCaseParameterValue().withValue("20").withName("minValue")));
TestCase testCase = createEntity(create, ADMIN_AUTH_HEADERS);
testCaseCount++;
CreateTestCaseResult createTestCaseResult =
@ -902,7 +915,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(String.format("<#E::table::%s>", tableFQN))
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("20").withName("max")));
List.of(new TestCaseParameterValue().withValue("20").withName("maxValue")));
if (i == 2) {
// create 1 test cases with USER21_TEAM as owner
create.withOwners(List.of(teamRef));
@ -1133,7 +1146,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(String.format("<#E::table::%s>", table.getFullyQualifiedName()))
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("20").withName("min")));
List.of(new TestCaseParameterValue().withValue("20").withName("minValue")));
createEntity(create, ADMIN_AUTH_HEADERS);
create =
createRequest(testInfo)
@ -2086,13 +2099,14 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
validTestCase
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withName("minLength").withValue("10")));
List.of(new TestCaseParameterValue().withName("minValue").withValue("10")));
createEntity(validTestCase, ADMIN_AUTH_HEADERS);
validTestCase = createRequest(test, 1);
validTestCase
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(List.of(new TestCaseParameterValue().withName("max").withValue("10")));
.withParameterValues(
List.of(new TestCaseParameterValue().withName("maxValue").withValue("10")));
createEntity(validTestCase, ADMIN_AUTH_HEADERS);
CreateTestCase invalidTestCase = createRequest(test, 2);
@ -2104,7 +2118,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
new TestCaseParameterValue().withName("maxLength").withValue("5")));
assertResponseContains(
() -> createEntity(invalidTestCase, ADMIN_AUTH_HEADERS), BAD_REQUEST, "Value");
() -> createEntity(invalidTestCase, ADMIN_AUTH_HEADERS), BAD_REQUEST, "Parameter");
CreateTestCase invalidTestCaseMixedTypes = createRequest(test, 3);
invalidTestCaseMixedTypes
@ -2115,7 +2129,9 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
new TestCaseParameterValue().withName("maxLength").withValue("5")));
assertResponseContains(
() -> createEntity(invalidTestCaseMixedTypes, ADMIN_AUTH_HEADERS), BAD_REQUEST, "Value");
() -> createEntity(invalidTestCaseMixedTypes, ADMIN_AUTH_HEADERS),
BAD_REQUEST,
"Parameter");
}
@Test
@ -2155,7 +2171,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(TABLE_LINK)
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("100").withName("max")));
List.of(new TestCaseParameterValue().withValue("100").withName("maxValue")));
TestCase testCase = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
List<String> columns = Arrays.asList(C1, C2, C3);
@ -2209,7 +2225,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(TABLE_LINK)
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("100").withName("max")));
List.of(new TestCaseParameterValue().withValue("100").withName("maxValue")));
TestCase testCase = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
List<String> columns = Arrays.asList("NOT_A_COLUMN", C1, C2, C3);
@ -2248,7 +2264,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(TABLE_LINK)
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("100").withName("max")));
List.of(new TestCaseParameterValue().withValue("100").withName("maxValue")));
TestCase testCase = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
List<String> columns = Arrays.asList(C1, C2, C3);
@ -2338,7 +2354,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(TABLE_LINK)
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("100").withName("max")));
List.of(new TestCaseParameterValue().withValue("100").withName("maxValue")));
TestCase testCase = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
String inspectionQuery = "SELECT * FROM test_table WHERE column1 = 'value1'";
putInspectionQuery(testCase, inspectionQuery);
@ -2765,7 +2781,7 @@ public class TestCaseResourceTest extends EntityResourceTest<TestCase, CreateTes
.withEntityLink(TABLE_LINK_2)
.withTestDefinition(TEST_DEFINITION4.getFullyQualifiedName())
.withParameterValues(
List.of(new TestCaseParameterValue().withValue("100").withName("max")));
List.of(new TestCaseParameterValue().withValue("100").withName("maxValue")));
TestCase testCase = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
CreateTestCaseResult createTestCaseResult =