Skip to content

Commit

Permalink
Fix GraphQL Nested UPDATE bug. (#974)
Browse files Browse the repository at this point in the history
* Fix bug in GraphQL where we add the parent relationship during an UPDATE request

* Reverting owasp check back to level 6

* Inspection rework
  • Loading branch information
aklish authored Sep 19, 2019
1 parent e2de2a8 commit 7b7aba5
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,12 @@ public interface PersistentResource<T> {
T getObject();
Class<T> getResourceClass();
RequestScope getRequestScope();

/**
* Returns whether or not this resource was created in this transaction.
* @return True if this resource is newly created.
*/
default boolean isNewlyCreated() {
return getRequestScope().getNewResources().contains(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public String toString() {
return String.format("PersistentResource{type=%s, id=%s}", type, uuid.orElse(getId()));
}

/**
/**
* Create a resource in the database.
* @param parent - The immediate ancestor in the lineage or null if this is a root.
* @param entityClass the entity class
Expand Down Expand Up @@ -1270,8 +1270,6 @@ protected Map<String, Object> getAttributes() {
*/
protected void setValueChecked(String fieldName, Object newValue) {
Object existingValue = getValueUnchecked(fieldName);
ChangeSpec spec = new ChangeSpec(this, fieldName, existingValue, newValue);
boolean isNewlyCreated = requestScope.getNewPersistentResources().contains(this);

// TODO: Need to refactor this logic. For creates this is properly converted in the executor. This logic
// should be explicitly encapsulated here, not there.
Expand Down Expand Up @@ -1587,8 +1585,7 @@ protected Set<String> filterFields(Collection<String> fields) {
*/
private void triggerUpdate(String fieldName, Object original, Object value) {
ChangeSpec changeSpec = new ChangeSpec(this, fieldName, original, value);
boolean isNewlyCreated = requestScope.getNewPersistentResources().contains(this);
CRUDEvent.CRUDAction action = isNewlyCreated
CRUDEvent.CRUDAction action = isNewlyCreated()
? CRUDEvent.CRUDAction.CREATE
: CRUDEvent.CRUDAction.UPDATE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public <A extends Annotation> ExpressionResult checkPermission(Class<A> annotati

Function<Expression, ExpressionResult> expressionExecutor = (expression) -> {
// for newly created object in PatchRequest limit to User checks
if (requestScope.getNewPersistentResources().contains(resource)) {
if (resource.isNewlyCreated()) {
return executeUserChecksDeferInline(annotationClass, expression);
}
return executeExpressions(expression, annotationClass, Expression.EvaluationMode.INLINE_CHECKS_ONLY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,9 @@ private ConnectionContainer upsertOrUpdateObjects(Environment context,
/* fixup relationships */
for (Entity entity : entitySet) {
graphWalker(entity, this::updateRelationship);
if (!context.isRoot()) { /* add relation between parent and nested entity */
PersistentResource childResource = entity.toPersistentResource();
if (!context.isRoot() && childResource.isNewlyCreated()) {
/* add relation between parent and nested entity */
context.parentResource.addRelation(context.field.getName(), entity.toPersistentResource());
}
}
Expand Down

0 comments on commit 7b7aba5

Please sign in to comment.