ATLAS-5005 : Basic search entity filter validation #326
Conversation
| @@ -179,6 +179,8 @@ public enum AtlasErrorCode { | |||
| LINEAGE_ON_DEMAND_NOT_ENABLED(400, "ATLAS-400-00-100", "Lineage on demand config: {0} is not enabled"), | |||
| INVALID_TOPIC_NAME(400, "ATLAS-400-00-101", "Unsupported topic name : {0}"), | |||
| UNSUPPORTED_TYPE_NAME(400, "ATLAS-400-00-102", "Unsupported {0} name. Names such as 'description','version','options','name','servicetype' are not supported"), | |||
| INVALID_OPERATOR(400, "ATLAS-400-00-103", "Invalid Operator Passed: for attribute name: {0}"), | |||
There was a problem hiding this comment.
Suggested correction for the error message : "Invalid operator specified for attribute: {0}"
| @@ -179,6 +179,8 @@ public enum AtlasErrorCode { | |||
| LINEAGE_ON_DEMAND_NOT_ENABLED(400, "ATLAS-400-00-100", "Lineage on demand config: {0} is not enabled"), | |||
| INVALID_TOPIC_NAME(400, "ATLAS-400-00-101", "Unsupported topic name : {0}"), | |||
| UNSUPPORTED_TYPE_NAME(400, "ATLAS-400-00-102", "Unsupported {0} name. Names such as 'description','version','options','name','servicetype' are not supported"), | |||
| INVALID_OPERATOR(400, "ATLAS-400-00-103", "Invalid Operator Passed: for attribute name: {0}"), | |||
| BLANK_ATTRIBUTES(400, "ATLAS-400-00-104", "Attribute Name and Attribute Value can't be empty!"), | |||
There was a problem hiding this comment.
consider splitting attribute name and value into separate error codes; as it is, the respective checks are also handled in different code blocks
|
|
||
| if (parameters.getEntityFilters().getCriterion() != null && | ||
| !parameters.getEntityFilters().getCriterion().isEmpty()) { | ||
| if (StringUtils.isEmpty(parameters.getEntityFilters().getCondition().toString())) { |
There was a problem hiding this comment.
This check is not safe: if getCondition() returns null, toString() will throw a NullPointerException
| } | ||
|
|
||
| private void validateEntityFilter(SearchParameters parameters) throws AtlasBaseException { | ||
| if (parameters.getEntityFilters() == null) { |
There was a problem hiding this comment.
Repeated call to parameters.getEntityFilters() makes the code verbose and harder to read
Consider defining FilterCriteria entityFilter = parameters.getEntityFilters();
and reuse entityFilter throughout the method.
| atlasClientV2.facetedSearch(params); | ||
| } | ||
| catch (AtlasServiceException e) { | ||
| assertTrue(e.getMessage().contains("ATLAS-400-00-103")); |
There was a problem hiding this comment.
Use assertTrue with a failure message like below to provide a clear context when a test case fails:
assertTrue(e.getMessage().contains("ATLAS-400-00-103"),
"Expected error code ATLAS-400-00-103 in exception message: " + e.getMessage());
| atlasClientV2.facetedSearch(params); | ||
| } | ||
| catch (AtlasServiceException e) { | ||
| assertTrue(e.getMessage().contains("ATLAS-400-00-104")); |
There was a problem hiding this comment.
Use assertTrue with a failure message like below to provide a clear context when a test case fails:
assertTrue(e.getMessage().contains("ATLAS-400-00-104"),
"Expected error code ATLAS-400-00-103 in exception message: " + e.getMessage());
| .filter(params -> params.getEntityFilters() != null && params.getEntityFilters().getOperator() != null) | ||
| .forEach(params -> { | ||
| try { | ||
| atlasClientV2.facetedSearch(params); |
There was a problem hiding this comment.
Currently, if facetedSearch() does not throw an exception, the test will silently succeed. Add an explicit fail() if the method does not throw
| .filter(params -> params.getEntityFilters() != null && params.getEntityFilters().getAttributeName() != null) | ||
| .forEach(params -> { | ||
| try { | ||
| atlasClientV2.facetedSearch(params); |
There was a problem hiding this comment.
Currently, if facetedSearch() does not throw an exception, the test will silently succeed. Add an explicit fail() if the method does not throw
| @@ -224,6 +240,50 @@ public void testSavedSearch(String jsonFile) { | |||
| } | |||
| } | |||
|
|
|||
| @Test(dataProvider = "attributeSearchJSONNamesInvalidOperator") | |||
There was a problem hiding this comment.
Consider using slightly clearer name like invalidOperatorTestFiles instead of attributeSearchJSONNamesInvalidOperator
| } | ||
| } | ||
|
|
||
| @Test(dataProvider = "attributeSearchJSONNamesEmptyAttribute") |
There was a problem hiding this comment.
Consider using slightly clearer name like emptyAttributeTestFiles instead of attributeSearchJSONNamesEmptyAttribute
aditya-gupta36
force-pushed
the
ATLAS-5005
branch
from
a1b2e94 to
ab3257e
Compare
What changes were proposed in this pull request?
Validation for FilterCriteria parameter was absence while API call.
How was this patch tested?
unit tests, manual tests, API curl command testing done, Raised the patch for pre-commit build.
PC:- https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1803/console