----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74608/#review225896 -----------------------------------------------------------
intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java Lines 122 (patched) <https://reviews.apache.org/r/74608/#comment314301> Kindly format with the correct indentation repository/src/main/java/org/apache/atlas/repository/ogm/AtlasRuleDTO.java Lines 39 (patched) <https://reviews.apache.org/r/74608/#comment314302> Kindly format with the correct indentation repository/src/main/java/org/apache/atlas/repository/ogm/AtlasRuleDTO.java Lines 117 (patched) <https://reviews.apache.org/r/74608/#comment314303> Kindly format with the correct indentation. repository/src/main/java/org/apache/atlas/rulesengine/AtlasEntityAuditFilterService.java Lines 209 (patched) <https://reviews.apache.org/r/74608/#comment314304> isValid can be avoided here like below for better readability. private void validateRuleAction(String action) throws AtlasBaseException { if (StringUtils.isNotEmpty(action)) { for (RuleAction.Result res : RuleAction.Result.values()){ if (res.name().equals(action)) { return; } } throw new AtlasBaseException(AtlasErrorCode.INVALID_RULE_ACTION); } } repository/src/main/java/org/apache/atlas/rulesengine/AtlasEntityAuditFilterService.java Lines 224 (patched) <https://reviews.apache.org/r/74608/#comment314305> Its very big function and should be broken down like - private void validateRuleExprFormat(AtlasRule.RuleExpr ruleExpr) throws AtlasBaseException { if (ruleExpr == null) { return; } List<AtlasRule.RuleExprObject> allExpressions = ruleExpr.getRuleExprObjList(); List<List<String>> recordedTypeNamesList = new ArrayList<>(); for (AtlasRule.RuleExprObject ruleExprObj : allExpressions) { validateTypeName(ruleExprObj.getTypeName(), recordedTypeNamesList); AtlasRule.Condition condition = ruleExprObj.getCondition(); List<AtlasRule.RuleExprObject.Criterion> criterion = ruleExprObj.getCriterion(); validateConditionAndCriteria(condition, criterion); validateAttributes(ruleExprObj.getTypeName(), condition, criterion); } } private void validateTypeName(String typeName, List<List<String>> recordedTypeNamesList) throws AtlasBaseException { if (Strings.isNullOrEmpty(typeName) || "null".equals(typeName)) { throw new AtlasBaseException(AtlasErrorCode.MISSING_MANDATORY_TYPENAME_IN_RULE_EXPR); } if (isDuplicateTypeNameValue(recordedTypeNamesList, typeName)) { throw new AtlasBaseException(AtlasErrorCode.DUPLICATE_TYPENAME_IN_RULE_EXPR, typeName); } recordedTypeNamesList.add(Arrays.asList(typeName.split(",")); getEntityTypes(typeName); } private void validateConditionAndCriteria(AtlasRule.Condition condition, List<AtlasRule.RuleExprObject.Criterion> criterion) throws AtlasBaseException { if (condition == null && CollectionUtils.isEmpty(criterion)) { throw new AtlasBaseException(AtlasErrorCode.MISSING_CRITERIA_CONDITION, "condition and criteria"); } } private void validateAttributes(String typeName, AtlasRule.Condition condition, List<AtlasRule.RuleExprObject.Criterion> criterion) throws AtlasBaseException { Set<AtlasEntityType> entityTypes = getEntityTypes(typeName); for (AtlasEntityType entityType : entityTypes) { if (condition != null && CollectionUtils.isNotEmpty(criterion)) { validateCriteriaList(entityType, criterion); } else { validateExpression(entityType, ruleExprObj.getOperator(), ruleExprObj.getAttributeName(), ruleExprObj.getAttributeValue() ); } } } repository/src/main/java/org/apache/atlas/rulesengine/AtlasEntityAuditFilterService.java Lines 292 (patched) <https://reviews.apache.org/r/74608/#comment314306> Kinldy consider to refactor this like- private void validateExternalAttribute(String attrName, String attrValue, AtlasRule.RuleExprObject.Operator operator) throws AtlasBaseException { if (ATTR_OPERATION_TYPE.equals(attrName)) { EntityAuditEventV2.EntityAuditActionV2[] enumConstants = EntityAuditEventV2.EntityAuditActionV2.class.getEnumConstants(); if (isValidOperator(enumConstants, operator, attrValue)) { return; } } throw new AtlasBaseException(AtlasErrorCode.INVALID_OPERATOR_ON_ATTRIBUTE, operator.getSymbol(), ATTR_OPERATION_TYPE); } private boolean isValidOperator(EntityAuditEventV2.EntityAuditActionV2[] enumConstants, AtlasRule.RuleExprObject.Operator operator, String attrValue) { switch (operator) { case EQ: return Arrays.stream(enumConstants).anyMatch(e -> e.name().equals(attrValue)); case STARTS_WITH: return Arrays.stream(enumConstants).anyMatch(e -> AtlasRuleUtils.startsWithFunc.apply(new String[]{e.name(), attrValue})); case ENDS_WITH: return Arrays.stream(enumConstants).anyMatch(e -> AtlasRuleUtils.endsWithFunc.apply(new String[]{e.name(), attrValue})); case CONTAINS: return Arrays.stream(enumConstants).anyMatch(e -> AtlasRuleUtils.containsFunc.apply(new String[]{e.name(), attrValue})); case CONTAINS_IGNORECASE: return Arrays.stream(enumConstants).anyMatch(e -> AtlasRuleUtils.containsIgnoreCaseFunc.apply(new String[]{e.name(), attrValue})); case NOT_CONTAINS: return Arrays.stream(enumConstants).anyMatch(e -> AtlasRuleUtils.notContainsFunc.apply(new String[]{e.name(), attrValue})); case NOT_CONTAINS_IGNORECASE: return Arrays.stream(enumConstants).anyMatch(e -> AtlasRuleUtils.notContainsIgnoreCaseFunc.apply(new String[]{e.name(), attrValue})); default: return false; } } repository/src/main/java/org/apache/atlas/rulesengine/AtlasEntityAuditFilterService.java Lines 641 (patched) <https://reviews.apache.org/r/74608/#comment314307> Please consider breaking it down into something like the following for improved readability and easier maintenance. - private String convertRuleExprObj(JsonObject ruleExprObj) { String op = extractStringProperty(ruleExprObj, PROPERTY_KEY_CONDITION, PROPERTY_KEY_OPERATOR); String typeNameValue = extractStringProperty(ruleExprObj, PROPERTY_KEY_TYPENAME); boolean includeSubTypes = extractBooleanProperty(ruleExprObj, PROPERTY_KEY_INCLUDESUBTYPES); if (op == null && !ALL_ENTITY_TYPES.equals(typeNameValue)) { return getMatchingTypeNameCondition(typeNameValue, includeSubTypes); } String typeNameConditionStr = null; String conditionRes = null; String criteriaRes = null; if (typeNameValue != null && !ALL_ENTITY_TYPES.equals(typeNameValue)) { typeNameConditionStr = getMatchingTypeNameCondition(typeNameValue, includeSubTypes); } if ("and".equalsIgnoreCase(op) || "or".equalsIgnoreCase(op)) { JsonArray criterion = ruleExprObj.getAsJsonArray(PROPERTY_KEY_CRITERION); String result = convertCriterionArray(criterion); conditionRes = "{\"" + op + "\": [" + result + "]}"; } else { criteriaRes = formatSingleCriterion(ruleExprObj); } if (typeNameConditionStr != null) { if (conditionRes != null) { return "{\"and\": [" + typeNameConditionStr + ", " + conditionRes + "]}"; } else { return "{\"and\": [" + typeNameConditionStr + ", " + criteriaRes + "]}"; } } else if (conditionRes != null) { return conditionRes; } else { return criteriaRes; } } private String extractStringProperty(JsonObject jsonObject, String... propertyKeys) { for (String propertyKey : propertyKeys) { if (jsonObject.has(propertyKey)) { return jsonObject.get(propertyKey).getAsString(); } } return null; } private boolean extractBooleanProperty(JsonObject jsonObject, String propertyKey) { if (jsonObject.has(propertyKey)) { return jsonObject.get(propertyKey).getAsBoolean(); } return true; } private String convertCriterionArray(JsonArray criterion) { StringBuilder result = new StringBuilder(); for (JsonElement criteria : criterion) { JsonObject subCondition = criteria.getAsJsonObject(); String subResult = convertRuleExprObj(subCondition); result.append(subResult).append(","); } if (result.length() > 0) { result.deleteCharAt(result.length() - 1); } return result.toString(); } private String formatSingleCriterion(JsonObject ruleExprObj) { String op = ruleExprObj.get(PROPERTY_KEY_CONDITION).getAsString().toLowerCase(); String attrName = formatAttrName(ruleExprObj.get(PROPERTY_KEY_ATTRIBUTENAME).getAsString()); String criteriaRes = "{\"" + op + "\": [{\"var\":\"" + attrName + "\"}"; if (ruleExprObj.has(PROPERTY_KEY_ATTRIBUTEVALUE)) { String attrVal = ruleExprObj.get(PROPERTY_KEY_ATTRIBUTEVALUE).getAsString(); criteriaRes += "," + attrVal; } criteriaRes += "]}"; return criteriaRes; } - Sidharth Mishra On Oct. 22, 2023, 5:33 a.m., Sheetal Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74608/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2023, 5:33 a.m.) > > > Review request for atlas, Jayendra Parab, Mandar Ambawane, Pinal Shah, Prasad > Pawar, Radhika Kundam, and Sidharth Mishra. > > > Bugs: ATLAS-4797 > https://issues.apache.org/jira/browse/ATLAS-4797 > > > Repository: atlas > > > Description > ------- > > Currently, Atlas has to persist all audit events without any constraints > which can make audit data grow exponentially after some time. > > This feature is specifically to reduce Atlas audit storage where generation > of audit events can be controlled based on the user's use-case/criteria > > Note: Audit data mentioned here is exclusive of Admin Audit Data which > persists in atlas_janus table. Filtering will be applicable to the entity > audit data which persists in ATLAS_ENTITY_AUDIT_EVENTS table. > > Precommit details : > Latest PC build: > https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1473 > Failed tests: EntityV2JerseyResourceIT.testSetLabelsByTypeName:986 expected > [2] but found [1] > > > Diffs > ----- > > addons/hbase-bridge/pom.xml 278b6c6d3 > addons/hbase-testing-util/pom.xml c4f3a99ee > addons/hive-bridge-shim/pom.xml 39f16ceee > addons/hive-bridge/pom.xml 356ac9542 > addons/kafka-bridge/pom.xml 093db7229 > addons/models/0000-Area0/0010-base_model.json a4a9248ec > addons/sqoop-bridge/pom.xml 4b6eac98f > authorization/pom.xml 7a1108487 > distro/src/conf/atlas-application.properties b5734d7a8 > graphdb/api/pom.xml 4ba89b20f > graphdb/janus-hbase2/pom.xml c2a2e74f0 > intg/pom.xml 43a172c1b > intg/src/main/java/org/apache/atlas/AtlasConfiguration.java df886753f > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 77a6fd8c3 > intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java > 3afd27e7d > repository/pom.xml 8fd744b44 > > repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java > 9b011ab06 > > repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java > a99f9b383 > repository/src/main/java/org/apache/atlas/repository/ogm/AtlasRuleDTO.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/repository/ogm/DataAccess.java > e63152475 > > repository/src/main/java/org/apache/atlas/rulesengine/AtlasEntityAuditFilterService.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/rulesengine/AtlasRule.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/rulesengine/AtlasRuleUtils.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/rulesengine/AtlasRulesEngine.java > PRE-CREATION > repository/src/main/java/org/apache/atlas/rulesengine/RuleAction.java > PRE-CREATION > server-api/pom.xml de311c329 > test-tools/src/main/resources/solr/core-template/solrconfig.xml 1550052b4 > webapp/src/main/java/org/apache/atlas/web/filters/ActiveServerFilter.java > a25a51b5a > webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java > b19095b48 > webapp/src/test/java/org/apache/atlas/web/resources/AdminResourceTest.java > a4d794615 > > > Diff: https://reviews.apache.org/r/74608/diff/10/ > > > Testing > ------- > > Manual testing done > > > Thanks, > > Sheetal Shah > >