bdemers commented on code in PR #213: URL: https://github.com/apache/directory-scimple/pull/213#discussion_r1088140566
########## scim-core/src/main/java/org/apache/directory/scim/core/repository/PatchHandlerImpl.java: ########## @@ -0,0 +1,388 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + + * http://www.apache.org/licenses/LICENSE-2.0 + + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.directory.scim.core.repository; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.extern.slf4j.Slf4j; +import org.apache.directory.scim.core.json.ObjectMapperFactory; +import org.apache.directory.scim.core.schema.SchemaRegistry; +import org.apache.directory.scim.spec.filter.AttributeComparisonExpression; +import org.apache.directory.scim.spec.filter.FilterExpressions; +import org.apache.directory.scim.spec.filter.FilterParseException; +import org.apache.directory.scim.spec.filter.ValuePathExpression; +import org.apache.directory.scim.spec.filter.attribute.AttributeReference; +import org.apache.directory.scim.spec.patch.PatchOperation; +import org.apache.directory.scim.spec.patch.PatchOperationPath; +import org.apache.directory.scim.spec.resources.ScimResource; +import org.apache.directory.scim.spec.schema.Schema; +import org.apache.directory.scim.spec.schema.Schema.Attribute; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.function.Predicate; + +import static java.util.stream.Collectors.toList; + +@SuppressWarnings("unchecked") +@Slf4j +public class PatchHandlerImpl implements PatchHandler { + + private static final TypeReference<Map<String, Object>> MAP_TYPE = new TypeReference<>() {}; + + private final Map<PatchOperation.Type, PatchOperationHandler> patchOperationHandlers = Map.of( + PatchOperation.Type.ADD, new AddOperationHandler(), + PatchOperation.Type.REPLACE, new ReplaceOperationHandler(), + PatchOperation.Type.REMOVE, new RemoveOperationHandler() + ); + + private final ObjectMapper objectMapper; + + private final SchemaRegistry schemaRegistry; + + public PatchHandlerImpl(SchemaRegistry schemaRegistry) { + this.schemaRegistry = schemaRegistry; + this.objectMapper = ObjectMapperFactory.createObjectMapper(this.schemaRegistry); + } + + public <T extends ScimResource> T apply(final T original, final List<PatchOperation> patchOperations) { + if (original == null) { + throw new IllegalArgumentException("Original resource is null. Cannot apply patch."); + } + if (patchOperations == null) { + throw new IllegalArgumentException("patchOperations is null. Cannot apply patch."); + } + + Map<String, Object> sourceAsMap = objectAsMap(original); + for (PatchOperation patchOperation : patchOperations) { + if (patchOperation.getPath() == null) { + if (!(patchOperation.getValue() instanceof Map)) { + throw new IllegalArgumentException("Cannot apply patch. value is required"); + } + Map<String, Object> properties = (Map<String, Object>) patchOperation.getValue(); + + for (Map.Entry<String, Object> entry : properties.entrySet()) { + // convert SCIM patch to RFC-6902 patch + PatchOperation newPatchOperation = new PatchOperation(); + newPatchOperation.setOperation(patchOperation.getOperation()); + newPatchOperation.setPath(tryGetOperationPath(entry.getKey())); + newPatchOperation.setValue(entry.getValue()); + + apply(original, sourceAsMap, newPatchOperation); + } + } else { + apply(original, sourceAsMap, patchOperation); + } + + } + return (T) objectMapper.convertValue(sourceAsMap, original.getClass()); + } + + private <T extends ScimResource> void apply(T source, Map<String, Object> sourceAsMap, final PatchOperation patchOperation) { + + final ValuePathExpression valuePathExpression = valuePathExpression(patchOperation); + final AttributeReference attributeReference = attributeReference(valuePathExpression); + + PatchOperationHandler patchOperationHandler = patchOperationHandlers.get(patchOperation.getOperation()); + + // if the attribute has a URN, assume it's an extension that URN does not match the baseUrn + if (attributeReference.hasUrn() && !attributeReference.getUrn().equals(source.getBaseUrn())) { + Schema schema = this.schemaRegistry.getSchema(attributeReference.getUrn()); + Attribute attribute = schema.getAttribute(attributeReference.getAttributeName()); + checkMutability(schema.getAttributeFromPath(attributeReference.getFullAttributeName())); + + patchOperationHandler.applyExtensionValue(source, sourceAsMap, schema, attribute, valuePathExpression, attributeReference.getUrn(), patchOperation.getValue()); + } else { + Schema schema = this.schemaRegistry.getSchema(source.getBaseUrn()); + Attribute attribute = schema.getAttribute(attributeReference.getAttributeName()); + checkMutability(schema.getAttributeFromPath(attributeReference.getFullAttributeName())); + + patchOperationHandler.applyValue(source, sourceAsMap, schema, attribute, valuePathExpression, patchOperation.getValue()); + } + } + + private PatchOperationPath tryGetOperationPath(String key) { + try { + return new PatchOperationPath(key); + } catch (FilterParseException e) { + log.warn("Parsing path failed with exception.", e); + throw new IllegalArgumentException("Cannot parse path expression: " + e.getMessage()); + } + } + + private Map<String, Object> objectAsMap(final Object object) { + return objectMapper.convertValue(object, MAP_TYPE); + } + + public static ValuePathExpression valuePathExpression(final PatchOperation operation) { + return Optional.ofNullable(operation.getPath()) + .map(PatchOperationPath::getValuePathExpression) + .orElseThrow(() -> new IllegalArgumentException("Patch operation must have a value path expression")); + } + + public static AttributeReference attributeReference(final ValuePathExpression expression) { + return Optional.ofNullable(expression.getAttributePath()) + .orElseThrow(() -> new IllegalArgumentException("Patch operation must have an expression with a valid attribute path")); + } + + private static void checkMutability(Attribute attribute) throws IllegalArgumentException { + if (attribute.getMutability().equals(Attribute.Mutability.READ_ONLY)) { + String message = "Can not update a read-only attribute '" + attribute.getName() + "'"; + log.error(message); + throw new IllegalArgumentException(message); + } + } + + private static void checkMutability(Attribute attribute, Object currentValue) throws IllegalArgumentException { Review Comment: My last change litters the code with a few more `checkMutability` calls, to account for rfc7644 sec 3.5.2 > a client MUST NOT modify an attribute that has mutability "readOnly" or "immutable". However, a client MAY "add" a value to an "immutable" attribute if the attribute had no previous value. I'm not really a fan of spreading of how I spread this logic around, but needing the object's current value makes it tricky. I went down the rabbit hole of creating a Collection/Map/Iterator/etc that could wrap a delegate and then make a mutability check if/when the attribute was mutated in the map. It was a neat bit of code, but it was ugly and required special handling for extensions so I threw it away. There is probably something more elegant that could be done, but I'm out of ideas 😆 It's not critical to get this in, but if you have any thoughts let me know! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
