Refactor : Removed some Implementation smells (#147)

* Refactor : Removed some Implementation smells * Fix : Dissociate function logic fxix --------- Co-authored-by: Savan Maheshkumar Patel <sv272995@dal.ca>
usnistgov · May 13, 2024 · 29184cf · 29184cf
1 parent 083c688
commit 29184cf
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 49 deletions.
diff --git a/src/main/java/gov/nist/csd/pm/pap/PAPGraph.java b/src/main/java/gov/nist/csd/pm/pap/PAPGraph.java
@@ -289,8 +289,10 @@ private boolean nodeInObligation(String name, Obligation obligation) {
     private boolean nodeInEvent(String name, EventPattern event) {
         // check subject
         EventSubject subject = event.getSubject();
-        if ((subject.getType() == EventSubject.Type.ANY_USER_WITH_ATTRIBUTE && subject.anyUserWithAttribute().equals(name))
-                || (subject.getType() == EventSubject.Type.USERS && subject.users().contains(name))) {
+
+        boolean isAnyUserWithAttribute = (subject.getType() == EventSubject.Type.ANY_USER_WITH_ATTRIBUTE && subject.anyUserWithAttribute().equals(name));
+        boolean isUserWithAttribute = (subject.getType() == EventSubject.Type.USERS && subject.users().contains(name));
+        if (isAnyUserWithAttribute || isUserWithAttribute) {
             return true;
         }
 
@@ -429,8 +431,14 @@ static void checkAccessRightsValid(Graph graph, AccessRightSet accessRightSet) t
 
     @Override
     public void dissociate(String ua, String target) throws PMException {
-        if ((!nodeExists(ua) || !nodeExists(target))
-                || (!getAssociationsWithSource(ua).contains(new Association(ua, target)))) {
+
+        boolean nodesNotExist = (!nodeExists(ua) || !nodeExists(target));
+        if (nodesNotExist) {
+            return;
+        }
+
+        boolean pathNotExist = (!getAssociationsWithSource(ua).contains(new Association(ua, target)));
+        if (pathNotExist) {
             return;
         }
 

diff --git a/src/main/java/gov/nist/csd/pm/pap/mysql/MysqlGraph.java b/src/main/java/gov/nist/csd/pm/pap/mysql/MysqlGraph.java
@@ -509,9 +509,12 @@ private String createNode(String name, NodeType type, Map<String, String> proper
                     INSERT INTO node (node_type_id, name, properties) VALUES (?,?,?)
                     """;
         try(PreparedStatement ps = connection.getConnection().prepareStatement(sql)) {
-            ps.setInt(1, MysqlPolicyStore.getNodeTypeId(type));
-            ps.setString(2, name);
-            ps.setString(3, MysqlPolicyStore.toJSON(properties));
+            int nodeTypeIdIndex = 1;
+            int nameIndex = 2;
+            int propertiesIndex = 3;
+            ps.setInt(nodeTypeIdIndex, MysqlPolicyStore.getNodeTypeId(type));
+            ps.setString(nameIndex, name);
+            ps.setString(propertiesIndex, MysqlPolicyStore.toJSON(properties));
             ps.execute();
 
             assign(name, initialParent);

diff --git a/src/main/java/gov/nist/csd/pm/policy/pml/statement/ForeachStatement.java b/src/main/java/gov/nist/csd/pm/policy/pml/statement/ForeachStatement.java
@@ -34,57 +34,66 @@ public Value execute(ExecutionContext ctx, Policy policy) throws PMException {
 
         Value iterValue = iter.execute(ctx, policy);
         if (iterValue.isArray()) {
-            for (Value v : iterValue.getArrayValue()) {
-                ExecutionContext localExecutionCtx;
-                try {
-                    localExecutionCtx = ctx.copy();
-                } catch (PMLScopeException e) {
-                    throw new RuntimeException(e);
-                }
+            return executeArrayIterator(iterValue, ctx, policy);
+        } else if (iterValue.isMap()) {
+            return executeMapIterator(iterValue,ctx,policy);
+        }
 
-                localExecutionCtx.scope().putValue(varName, v);
+        return new Value();
+    }
 
-                Value value = executeStatementBlock(localExecutionCtx, policy, statements);
+    private Value executeArrayIterator(Value iterValue,ExecutionContext ctx, Policy policy ) throws PMException{
+        for (Value v : iterValue.getArrayValue()) {
+            ExecutionContext localExecutionCtx;
+            try {
+                localExecutionCtx = ctx.copy();
+            } catch (PMLScopeException e) {
+                throw new RuntimeException(e);
+            }
 
-                if (value.isBreak()) {
-                    break;
-                } else if (value.isReturn()) {
-                    return value;
-                }
+            localExecutionCtx.scope().putValue(varName, v);
 
-                ctx.scope().overwriteValues(localExecutionCtx.scope());
-            }
-        } else if (iterValue.isMap()) {
-            for (Value key : iterValue.getMapValue().keySet()) {
-                ExecutionContext localExecutionCtx;
-                try {
-                    localExecutionCtx = ctx.copy();
-                } catch (PMLScopeException e) {
-                    throw new RuntimeException(e);
-                }
-
-                Value mapValue = iterValue.getMapValue().get(key);
-
-                localExecutionCtx.scope().putValue(varName, key);
-                if (valueVarName != null) {
-                    localExecutionCtx.scope().putValue(valueVarName, mapValue);
-                }
-
-                Value value = executeStatementBlock(localExecutionCtx, policy, statements);
-
-                if (value.isBreak()) {
-                    break;
-                } else if (value.isReturn()) {
-                    return value;
-                }
-
-                ctx.scope().overwriteValues(localExecutionCtx.scope());
+            Value value = executeStatementBlock(localExecutionCtx, policy, statements);
+
+            if (value.isBreak()) {
+                break;
+            } else if (value.isReturn()) {
+                return value;
             }
-        }
 
+            ctx.scope().overwriteValues(localExecutionCtx.scope());
+        }
         return new Value();
     }
 
+    private Value executeMapIterator(Value iterValue, ExecutionContext ctx, Policy policy ) throws PMException{
+        for (Value key : iterValue.getMapValue().keySet()) {
+            ExecutionContext localExecutionCtx;
+            try {
+                localExecutionCtx = ctx.copy();
+            } catch (PMLScopeException e) {
+                throw new RuntimeException(e);
+            }
+
+            Value mapValue = iterValue.getMapValue().get(key);
+
+            localExecutionCtx.scope().putValue(varName, key);
+            if (valueVarName != null) {
+                localExecutionCtx.scope().putValue(valueVarName, mapValue);
+            }
+
+            Value value = executeStatementBlock(localExecutionCtx, policy, statements);
+
+            if (value.isBreak()) {
+                break;
+            } else if (value.isReturn()) {
+                return value;
+            }
+
+            ctx.scope().overwriteValues(localExecutionCtx.scope());
+        }
+        return new Value();
+    }
     @Override
     public String toString() {
         return String.format("foreach %s in %s {%s}",