Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor : Removed some Implementation smells #147

Merged
merged 3 commits into from
May 13, 2024
+69 −49
Merged

Refactor : Removed some Implementation smells #147

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f7cf2ff
Refactor : Removed some Implementation smells
Mar 30, 2024
795f887
Fix : Dissociate function logic fxix
May 11, 2024
2eab059
Merge branch 'PM-Master:master' into master
Patel-Savan May 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/main/java/gov/nist/csd/pm/pap/PAPGraph.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -429,8 +431,10 @@ 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)))) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good but the logic is slightly different. We don't want to call getAssociationsWithSource if the ua node does not exist. The previous logic relied on the if statement short circuiting if the node did not exist so getAssociationsWithSource was never called. You could change it to something like this:

boolean nodesNotExist = (!nodeExists(ua) || !nodeExists(target));
if (nodesNotExist) {
    return;
}

boolean pathNotExist = (!getAssociationsWithSource(ua).contains(new Association(ua, target)));
if (pathNotExist) {
    return;
}

to get the same logic as the short circuit.

boolean nodesNotExist = (!nodeExists(ua) || !nodeExists(target));
boolean pathNotExist = (!getAssociationsWithSource(ua).contains(new Association(ua, target)));
if (nodesNotExist || pathNotExist) {
return;
}

Expand Down
9 changes: 6 additions & 3 deletions src/main/java/gov/nist/csd/pm/pap/mysql/MysqlGraph.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
93 changes: 51 additions & 42 deletions src/main/java/gov/nist/csd/pm/policy/pml/statement/ForeachStatement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
Expand Down