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.

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

Fix GraphQL Nested UPDATE bug. #974

Merged
merged 7 commits into from
Sep 19, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
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