rkundam commented on code in PR #333:
URL: https://github.com/apache/atlas/pull/333#discussion_r2049346056
##########
webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java:
##########
@@ -885,7 +885,56 @@ private void validateSearchParameters(SearchParameters
parameters) throws AtlasB
if (StringUtils.isNotEmpty(parameters.getQuery()) &&
parameters.getQuery().length() > maxFullTextQueryLength) {
throw new
AtlasBaseException(AtlasErrorCode.INVALID_QUERY_LENGTH,
Constants.MAX_FULLTEXT_QUERY_STR_LENGTH);
}
+
+ validateEntityFilter(parameters);
+ }
+ }
+
+ private void validateEntityFilter(SearchParameters parameters) throws
AtlasBaseException {
+ FilterCriteria entityFilter = parameters.getEntityFilters();
+
+ if (entityFilter == null) {
+ return;
}
+
+ if (entityFilter.getCriterion() != null &&
+ !entityFilter.getCriterion().isEmpty()) {
+ if (entityFilter.getCondition() == null ||
StringUtils.isEmpty(entityFilter.getCondition().toString())) {
+ throw new AtlasBaseException("Condition (AND/OR) must be
specified when using multiple filters.");
+ }
+
+ for (FilterCriteria filterCriteria : entityFilter.getCriterion()) {
+ if (filterCriteria.getOperator() == null) {
+ throw new
AtlasBaseException(AtlasErrorCode.INVALID_OPERATOR,
filterCriteria.getAttributeName());
+ }
+
+ if (StringUtils.isBlank(filterCriteria.getAttributeName())) {
+ throw new
AtlasBaseException(AtlasErrorCode.BLANK_NAME_ATTRIBUTE);
+ }
+
+ if (requiresValue(filterCriteria.getOperator()) &&
StringUtils.isBlank(filterCriteria.getAttributeValue())) {
+ throw new
AtlasBaseException(AtlasErrorCode.BLANK_VALUE_ATTRIBUTE);
+ }
+ }
+ }
+ else {
+ if (entityFilter.getOperator() == null) {
+ throw new AtlasBaseException(AtlasErrorCode.INVALID_OPERATOR,
entityFilter.getAttributeName());
Review Comment:
Here it only performs a null check for the operator. However, based on the
use case mentioned in the Jira, it seems like additional validation for invalid
or unsupported operators is expected. Please ensure those checks are included.
##########
webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java:
##########
@@ -885,7 +885,56 @@ private void validateSearchParameters(SearchParameters
parameters) throws AtlasB
if (StringUtils.isNotEmpty(parameters.getQuery()) &&
parameters.getQuery().length() > maxFullTextQueryLength) {
throw new
AtlasBaseException(AtlasErrorCode.INVALID_QUERY_LENGTH,
Constants.MAX_FULLTEXT_QUERY_STR_LENGTH);
}
+
+ validateEntityFilter(parameters);
+ }
+ }
+
+ private void validateEntityFilter(SearchParameters parameters) throws
AtlasBaseException {
+ FilterCriteria entityFilter = parameters.getEntityFilters();
+
+ if (entityFilter == null) {
+ return;
}
+
+ if (entityFilter.getCriterion() != null &&
+ !entityFilter.getCriterion().isEmpty()) {
+ if (entityFilter.getCondition() == null ||
StringUtils.isEmpty(entityFilter.getCondition().toString())) {
+ throw new AtlasBaseException("Condition (AND/OR) must be
specified when using multiple filters.");
+ }
+
+ for (FilterCriteria filterCriteria : entityFilter.getCriterion()) {
+ if (filterCriteria.getOperator() == null) {
Review Comment:
It appears to have some redundancy. Consider extracting the verification
logic into a separate method. This will help in reusing the snippet for both
single FilterCriteria and multiple FilterCriteria scenarios, improving
maintainability and readability.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
