From 036d44c343032070018fc2abf2fb08b164ebaf09 Mon Sep 17 00:00:00 2001 From: Radovan Semancik Date: Tue, 8 Aug 2017 17:32:31 +0200 Subject: [PATCH] Fixing strange almost non-reproducible NPE when parsing values with expression. --- .../prism/marshaller/BeanUnmarshaller.java | 10 +- .../midpoint/schema/TestParseDiffPatch.java | 114 +++++++-- .../resources/diff/resource-after-const.xml | 232 ++++++++++++++++++ .../midpoint/util/FailableFunction.java | 27 ++ .../midpoint/model/intest/TestResources.java | 56 ++++- 5 files changed, 415 insertions(+), 24 deletions(-) create mode 100644 infra/schema/src/test/resources/diff/resource-after-const.xml create mode 100644 infra/util/src/main/java/com/evolveum/midpoint/util/FailableFunction.java diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/marshaller/BeanUnmarshaller.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/marshaller/BeanUnmarshaller.java index a4c0717b79e..091a478ca3e 100644 --- a/infra/prism/src/main/java/com/evolveum/midpoint/prism/marshaller/BeanUnmarshaller.java +++ b/infra/prism/src/main/java/com/evolveum/midpoint/prism/marshaller/BeanUnmarshaller.java @@ -132,6 +132,9 @@ T unmarshal(@NotNull XNode xnode, @NotNull Class beanClass, @NotNull Pars } private T unmarshalInternal(@NotNull XNode xnode, @NotNull Class beanClass, @NotNull ParsingContext pc) throws SchemaException { + if (beanClass == null) { + throw new IllegalStateException("No bean class for node: " + xnode.debugDump()); + } if (xnode instanceof RootXNode) { XNode subnode = ((RootXNode) xnode).getSubnode(); if (subnode == null) { @@ -163,8 +166,11 @@ private T unmarshalInternal(@NotNull XNode xnode, @NotNull Class beanClas } } else { - if (beanClass.getPackage().getName().equals("java.lang")) { - // We obviously have primitive data type, but we have are asked to unmarshall from map xnode + if (beanClass.getPackage() == null || beanClass.getPackage().getName().equals("java.lang")) { + // We obviously have primitive data type, but we are asked to unmarshall from map xnode + // NOTE: this may happen in XML when we have "empty" element, but it has some whitespace in it + // such as those troublesome newlines. This also happens if there is "empty" element + // but it contains an expression (so it is not PrimitiveXNode but MapXNode). // TODO: more robust implementation // TODO: look for "value" subnode with primitive value and try that. // This is most likely attempt to parse primitive value with dynamic expression. diff --git a/infra/schema/src/test/java/com/evolveum/midpoint/schema/TestParseDiffPatch.java b/infra/schema/src/test/java/com/evolveum/midpoint/schema/TestParseDiffPatch.java index 4d1e79a2ac7..6fd6486b003 100644 --- a/infra/schema/src/test/java/com/evolveum/midpoint/schema/TestParseDiffPatch.java +++ b/infra/schema/src/test/java/com/evolveum/midpoint/schema/TestParseDiffPatch.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2013 Evolveum + * Copyright (c) 2010-2017 Evolveum * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -72,6 +72,16 @@ public class TestParseDiffPatch { private static final String TEST_DIR = "src/test/resources/diff/"; + + private static final File USER_BEFORE_FILE = new File(TEST_DIR, "user-before.xml"); + private static final File USER_AFTER_FILE = new File(TEST_DIR, "user-after.xml"); + private static final File TASK_BEFORE_FILE = new File(TEST_DIR, "task-before.xml"); + private static final File TASK_AFTER_FILE = new File(TEST_DIR, "task-after.xml"); + private static final File RESOURCE_BEFORE_FILE = new File(TEST_DIR, "resource-before.xml"); + private static final File RESOURCE_AFTER_FILE = new File(TEST_DIR, "resource-after.xml"); + private static final File RESOURCE_AFTER_CONST_FILE = new File(TEST_DIR, "resource-after-const.xml"); + private static final File RESOURCE_AFTER_NS_CHANGE_FILE = new File(TEST_DIR, "resource-after-ns-change.xml"); + private static final String RESOURCE_OID = "ef2bc95b-76e0-59e2-86d6-3d4f02d3ffff"; @BeforeSuite public void setup() throws SchemaException, SAXException, IOException { @@ -83,11 +93,9 @@ public void setup() throws SchemaException, SAXException, IOException { public void testUserCredentialsDiff() throws Exception { System.out.println("===[ testUserCredentialsDiff ]==="); - PrismObject userBefore = PrismTestUtil.parseObject( - new File(TEST_DIR, "user-before.xml")); + PrismObject userBefore = PrismTestUtil.parseObject(USER_BEFORE_FILE); userBefore.checkConsistence(); - PrismObject userAfter = PrismTestUtil.parseObject( - new File(TEST_DIR, "user-after.xml")); + PrismObject userAfter = PrismTestUtil.parseObject(USER_AFTER_FILE); userAfter.checkConsistence(); ObjectDelta userDelta = userBefore.diff(userAfter); @@ -113,8 +121,7 @@ public void testUserCredentialsDiff() throws Exception { public void testAssignmentActivationDiff() throws Exception { System.out.println("===[ testUserCredentialsDiff ]==="); - PrismObject userBefore = PrismTestUtil.parseObject( - new File(TEST_DIR, "user-before.xml")); + PrismObject userBefore = PrismTestUtil.parseObject(USER_BEFORE_FILE); PrismObject userAfter = userBefore.clone(); AssignmentType assignmentBefore = new AssignmentType(); ActivationType activation = new ActivationType(); @@ -333,7 +340,7 @@ public void testTask() throws SchemaException, SAXException, IOException, JAXBEx // WHEN - ObjectDelta diffDelta = DiffUtil.diff(new File(TEST_DIR, "task-before.xml"), + ObjectDelta diffDelta = DiffUtil.diff(TASK_BEFORE_FILE, new File(TEST_DIR, "task-after.xml"), TaskType.class, getPrismContext()); // THEN @@ -368,7 +375,7 @@ public void testTask() throws SchemaException, SAXException, IOException, JAXBEx // ROUNDTRIP - PrismObject taskPatch = PrismTestUtil.parseObject(new File(TEST_DIR, "task-before.xml")); + PrismObject taskPatch = PrismTestUtil.parseObject(TASK_BEFORE_FILE); taskPatch.checkConsistence(); // patch @@ -379,7 +386,7 @@ public void testTask() throws SchemaException, SAXException, IOException, JAXBEx diffDelta.checkConsistence(); taskPatch.checkConsistence(); - PrismObject taskAfter = PrismTestUtil.parseObject(new File(TEST_DIR, "task-after.xml")); + PrismObject taskAfter = PrismTestUtil.parseObject(TASK_AFTER_FILE); taskAfter.checkConsistence(); assertTrue("Not equivalent",taskPatch.equivalent(taskAfter)); @@ -401,11 +408,11 @@ public void testTask() throws SchemaException, SAXException, IOException, JAXBEx } @Test - public void testResource() throws SchemaException, SAXException, IOException, JAXBException { + public void testResource() throws Exception { System.out.println("===[ testResource ]==="); - PrismObject resourceBefore = PrismTestUtil.parseObject(new File(TEST_DIR, "resource-before.xml")); - PrismObject resourceAfter = PrismTestUtil.parseObject(new File(TEST_DIR, "resource-after.xml")); + PrismObject resourceBefore = PrismTestUtil.parseObject(RESOURCE_BEFORE_FILE); + PrismObject resourceAfter = PrismTestUtil.parseObject(RESOURCE_AFTER_FILE); resourceBefore.checkConsistence(); resourceAfter.checkConsistence(); @@ -427,7 +434,7 @@ public void testResource() throws SchemaException, SAXException, IOException, JA resourceBefore.checkConsistence(); resourceAfter.checkConsistence(); - assertEquals("Wrong delta OID", "ef2bc95b-76e0-59e2-86d6-3d4f02d3ffff", resourceDelta.getOid()); + assertEquals("Wrong delta OID", RESOURCE_OID, resourceDelta.getOid()); assertEquals("Wrong change type", ChangeType.MODIFY, resourceDelta.getChangeType()); Collection modifications = resourceDelta.getModifications(); assertEquals("Unexpected number of modifications", 7, modifications.size()); @@ -447,6 +454,77 @@ public void testResource() throws SchemaException, SAXException, IOException, JA resourceAfter.checkConsistence(); } + @Test + public void testResourceConst() throws Exception { + System.out.println("===[ testResourceConst ]==="); + + PrismObject resourceBefore = PrismTestUtil.parseObject(RESOURCE_BEFORE_FILE); + PrismObject resourceAfter = PrismTestUtil.parseObject(RESOURCE_AFTER_CONST_FILE); + + resourceBefore.checkConsistence(); + resourceAfter.checkConsistence(); + + // sanity + assertFalse("Equals does not work", resourceBefore.equals(resourceAfter)); + + // WHEN + + ObjectDelta resourceDelta = resourceBefore.diff(resourceAfter); + + // THEN + + System.out.println("DELTA:"); + System.out.println(resourceDelta.debugDump()); + + resourceDelta.checkConsistence(); + resourceDelta.assertDefinitions(true); + resourceBefore.checkConsistence(); + resourceAfter.checkConsistence(); + + assertEquals("Wrong delta OID", RESOURCE_OID, resourceDelta.getOid()); + assertEquals("Wrong change type", ChangeType.MODIFY, resourceDelta.getChangeType()); + Collection modifications = resourceDelta.getModifications(); + assertEquals("Unexpected number of modifications", 2, modifications.size()); + assertConfigurationPropertyChange(resourceDelta, "host"); + assertConfigurationPropertyChange(resourceDelta, "port"); + + resourceDelta.checkConsistence(); + resourceBefore.checkConsistence(); + resourceAfter.checkConsistence(); + } + + @Test + public void testResourceConstLiteral() throws Exception { + System.out.println("===[ testResourceConstLiteral ]==="); + + PrismObject resourceBefore = PrismTestUtil.parseObject(RESOURCE_BEFORE_FILE); + PrismObject resourceAfter = PrismTestUtil.parseObject(RESOURCE_AFTER_CONST_FILE); + + // WHEN + + ObjectDelta resourceDelta = resourceBefore.diff(resourceAfter, true, true); + + // THEN + + System.out.println("DELTA:"); + System.out.println(resourceDelta.debugDump()); + + resourceDelta.checkConsistence(); + resourceDelta.assertDefinitions(true); + resourceBefore.checkConsistence(); + resourceAfter.checkConsistence(); + + assertEquals("Wrong delta OID", RESOURCE_OID, resourceDelta.getOid()); + assertEquals("Wrong change type", ChangeType.MODIFY, resourceDelta.getChangeType()); + Collection modifications = resourceDelta.getModifications(); + assertEquals("Unexpected number of modifications", 2, modifications.size()); + assertConfigurationPropertyChange(resourceDelta, "host"); + assertConfigurationPropertyChange(resourceDelta, "port"); + + resourceDelta.checkConsistence(); + resourceBefore.checkConsistence(); + resourceAfter.checkConsistence(); + } private void assertConfigurationPropertyChange(ObjectDelta resourceDelta, String propName) { resourceDelta.checkConsistence(); @@ -470,8 +548,8 @@ private ItemPath pathTimeouts(String last) { public void testResourceRoundTrip() throws SchemaException, SAXException, IOException, JAXBException { System.out.println("===[ testResourceRoundTrip ]==="); - PrismObject resourceBefore = PrismTestUtil.parseObject(new File(TEST_DIR, "resource-before.xml")); - PrismObject resourceAfter = PrismTestUtil.parseObject(new File(TEST_DIR, "resource-after.xml")); + PrismObject resourceBefore = PrismTestUtil.parseObject(RESOURCE_BEFORE_FILE); + PrismObject resourceAfter = PrismTestUtil.parseObject(RESOURCE_AFTER_FILE); resourceBefore.checkConsistence(); resourceAfter.checkConsistence(); @@ -538,8 +616,8 @@ public void testResourceRoundTrip() throws SchemaException, SAXException, IOExce public void testResourceNsChange() throws SchemaException, SAXException, IOException, JAXBException { System.out.println("===[ testResourceNsChange ]==="); - PrismObject resourceBefore = PrismTestUtil.parseObject(new File(TEST_DIR, "resource-before.xml")); - PrismObject resourceAfter = PrismTestUtil.parseObject(new File(TEST_DIR, "resource-after-ns-change.xml")); + PrismObject resourceBefore = PrismTestUtil.parseObject(RESOURCE_BEFORE_FILE); + PrismObject resourceAfter = PrismTestUtil.parseObject(RESOURCE_AFTER_NS_CHANGE_FILE); resourceBefore.checkConsistence(); resourceAfter.checkConsistence(); diff --git a/infra/schema/src/test/resources/diff/resource-after-const.xml b/infra/schema/src/test/resources/diff/resource-after-const.xml new file mode 100644 index 00000000000..c8135bc5ef3 --- /dev/null +++ b/infra/schema/src/test/resources/diff/resource-after-const.xml @@ -0,0 +1,232 @@ + + + + + + Embedded Test OpenDJ + + + + + + + portNum + + + hostName + dc=example,dc=com + cn=directory manager + secret + uid + + + + 120000 + 1 + 10 + 10 + 150000 + + + 100 + + + -1 + -1 + -1 + -1 + -1 + -1 + -1 + -1 + -1 + -1 + -1 + -1 + + + + http://midpoint.evolveum.com/xml/ns/public/resource/instance/ef2bc95b-76e0-59e2-86d6-3d4f02d3ffff + + + + + + + + + + + icfs:uid + icfs:name + icfs:name + __GROUP__ + + + + + + + + + + + + + + + + + + + This is fake. It is only for namespace testing and similar wild things. + + + + + + + + + + icfs:uid + icfs:name + icfs:name + __ACCOUNT__ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + AccountObjectClass + + ri:pager + + + c:telephoneNumber + + + + + + + + + + + ri:ds-pwp-account-disabled + + true + + + + + + + + + + + + + + + c:name + + $account/attributes/ri:uid + + + + + + diff --git a/infra/util/src/main/java/com/evolveum/midpoint/util/FailableFunction.java b/infra/util/src/main/java/com/evolveum/midpoint/util/FailableFunction.java new file mode 100644 index 00000000000..c2a93b61696 --- /dev/null +++ b/infra/util/src/main/java/com/evolveum/midpoint/util/FailableFunction.java @@ -0,0 +1,27 @@ +/* + * Copyright (c) 2017 Evolveum + * + * Licensed 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 com.evolveum.midpoint.util; + +/** + * @author semancik + * + */ +@FunctionalInterface +public interface FailableFunction { + + R apply(T object) throws Exception; + +} diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestResources.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestResources.java index 9bcbda7c5d3..76c85d00ca9 100644 --- a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestResources.java +++ b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestResources.java @@ -26,6 +26,7 @@ import java.util.Collection; import java.util.List; import java.util.Random; +import java.util.function.Function; import javax.xml.namespace.QName; @@ -36,8 +37,12 @@ import org.springframework.test.context.ContextConfiguration; import org.testng.annotations.Test; import org.w3c.dom.Element; +import org.w3c.dom.Node; import com.evolveum.icf.dummy.resource.DummyResource; +import com.evolveum.midpoint.common.validator.EventHandler; +import com.evolveum.midpoint.common.validator.EventResult; +import com.evolveum.midpoint.common.validator.Validator; import com.evolveum.midpoint.model.api.ModelExecuteOptions; import com.evolveum.midpoint.prism.delta.ItemDelta; import com.evolveum.midpoint.prism.delta.ObjectDelta; @@ -63,6 +68,8 @@ import com.evolveum.midpoint.test.IntegrationTestTools; import com.evolveum.midpoint.test.util.TestUtil; import com.evolveum.midpoint.util.DOMUtil; +import com.evolveum.midpoint.util.FailableFunction; +import com.evolveum.midpoint.util.Holder; import com.evolveum.midpoint.util.exception.CommunicationException; import com.evolveum.midpoint.util.exception.ConfigurationException; import com.evolveum.midpoint.util.exception.ExpressionEvaluationException; @@ -917,8 +924,48 @@ public void test761ModifyConfigurationStringRaw() throws Exception { } @Test - public void test765ModifyConfigurationDiffExpressionRaw() throws Exception { - final String TEST_NAME = "test765ModifyConfigurationDiffExpressionRaw"; + public void test765ModifyConfigurationDiffExpressionRawPrismContextParse() throws Exception { + final String TEST_NAME = "test765ModifyConfigurationDiffExpressionRawPrismContextParse"; + modifyConfigurationDiffExpressionRaw(TEST_NAME, xml -> prismContext.parseObject(xml)); + } + + /** + * This is what GUI "Repository objects" page really does with XML. + */ + @Test + public void test767ModifyConfigurationDiffExpressionRawValidatorParse() throws Exception { + final String TEST_NAME = "test767ModifyConfigurationDiffExpressionRawValidatorParse"; + modifyConfigurationDiffExpressionRaw(TEST_NAME, xml -> { + final Holder> objectHolder = new Holder<>(); + EventHandler handler = new EventHandler() { + + @Override + public EventResult preMarshall(Element objectElement, Node postValidationTree, + OperationResult objectResult) { + return EventResult.cont(); + } + + @Override + public EventResult postMarshall(PrismObject object, Element objectElement, + OperationResult objectResult) { + objectHolder.setValue((PrismObject) object); + return EventResult.cont(); + } + + @Override + public void handleGlobalError(OperationResult currentResult) { + } + }; + Validator validator = new Validator(prismContext, handler); + validator.setVerbose(true); + validator.setValidateSchema(false); + OperationResult result = new OperationResult("validator"); + validator.validateObject(xml, result); + return objectHolder.getValue(); + }); + } + + public void modifyConfigurationDiffExpressionRaw(final String TEST_NAME, FailableFunction> parser) throws Exception { displayTestTile(TEST_NAME); Task task = createTask(TEST_NAME); @@ -931,14 +978,15 @@ public void test765ModifyConfigurationDiffExpressionRaw() throws Exception { String modifiedResourceXml = serializedResource.replace("whatever raw wherever", "useless"); display("New resource XML", modifiedResourceXml); - PrismObject modifiedResource = prismContext.parseObject(modifiedResourceXml); + + PrismObject modifiedResource = parser.apply(modifiedResourceXml); display("New resource", modifiedResource); // just for fun String serializedModifiedResource = prismContext.serializerFor(PrismContext.LANG_XML).serialize(modifiedResource); assertNotNull(serializedModifiedResource); - ObjectDelta diffDelta = resourceBefore.diff(modifiedResource); + ObjectDelta diffDelta = resourceBefore.diff(modifiedResource, true, true); display("Diff delta", diffDelta); // WHEN