Fix "get" for resources with broken connectorRef

Exceptions during abstract or inherited resources completion attempts are now handled correctly. This ensures that "getObject" operations on resources with dangling connectorRef values are treated gracefully. This resolves MID-8619.
Evolveum · Sep 6, 2023 · 7ef9e92 · 7ef9e92
1 parent 9e65d34
commit 7ef9e92
Show file tree

Hide file tree

Showing 8 changed files with 177 additions and 24 deletions.
diff --git a/...g-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ProvisioningServiceImpl.java b/...g-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ProvisioningServiceImpl.java
@@ -1129,8 +1129,7 @@ public void expandConfigurationObject(
                 .build();
         try {
             ObjectType objectBean = configurationObject.asObjectable();
-            if (objectBean instanceof ResourceType) {
-                ResourceType resource = (ResourceType) objectBean;
+            if (objectBean instanceof ResourceType resource) {
                 LOGGER.trace("Starting expanding {}", configurationObject);
                 resourceManager.expandResource(resource, result);
                 LOGGER.trace("Finished expanding {}", configurationObject);

diff --git a/...n/java/com/evolveum/midpoint/provisioning/impl/resources/ResourceCompletionOperation.java b/...n/java/com/evolveum/midpoint/provisioning/impl/resources/ResourceCompletionOperation.java
@@ -119,6 +119,13 @@ class ResourceCompletionOperation {
         result = parentResult.createMinorSubresult(OP_COMPLETE_RESOURCE);
         try {
             expand(resource);
+
+            if (ResourceTypeUtil.isAbstract(resource)) {
+                // TODO or should we also try to apply connector schema and expressions?
+                LOGGER.trace("Not continuing with resource completion because it's abstract: {}", resource);
+                return resource;
+            }
+
             applyConnectorSchemaAndExpressions();
 
             ResourceType completed;
@@ -144,11 +151,18 @@ class ResourceCompletionOperation {
     /**
      * Expands the resource by resolving super-resource references.
      */
-    private void expand(@NotNull ResourceType resource)
-            throws SchemaException, ConfigurationException, ObjectNotFoundException {
+    private void expand(@NotNull ResourceType resource) throws StopException {
         if (resource.getSuper() != null) {
             lastExpansionOperation = new ResourceExpansionOperation(resource, beans);
-            lastExpansionOperation.execute(result);
+            try {
+                lastExpansionOperation.execute(result);
+            } catch (SchemaException | ConfigurationException | ObjectNotFoundException | RuntimeException e) {
+                String message =
+                        "An error occurred while expanding super-resource references of " + resource + ": " + e.getMessage();
+                result.recordPartialError(message, e);
+                LOGGER.warn(message, e);
+                throw new StopException();
+            }
         } else {
             // We spare some CPU cycles by not instantiating the expansion operation object.
         }

diff --git a/...in/java/com/evolveum/midpoint/provisioning/impl/resources/ResourceExpansionOperation.java b/...in/java/com/evolveum/midpoint/provisioning/impl/resources/ResourceExpansionOperation.java
@@ -90,6 +90,9 @@ class ResourceExpansionOperation {
         this.beans = beans;
     }
 
+    /**
+     * Executes the expansion operation. Fails hard if e.g. `connectorRef` cannot be resolved.
+     */
     public void execute(OperationResult parentResult) throws SchemaException, ConfigurationException, ObjectNotFoundException {
         OperationResult result = parentResult.createMinorSubresult(OP_EXPAND);
         try {

diff --git a/...impl/src/main/java/com/evolveum/midpoint/provisioning/impl/resources/ResourceManager.java b/...impl/src/main/java/com/evolveum/midpoint/provisioning/impl/resources/ResourceManager.java
@@ -150,27 +150,18 @@ public class ResourceManager {
             @NotNull Task task,
             @NotNull OperationResult result)
             throws ObjectNotFoundException, SchemaException, ExpressionEvaluationException, ConfigurationException {
-        String oid = repositoryObject.getOid();
 
-        if (isAbstract(repositoryObject)) {
-            expandResource(repositoryObject, result);
-            LOGGER.trace("Not putting {} into cache because it's abstract", repositoryObject);
-            return repositoryObject;
-        }
-
-        if (LOGGER.isDebugEnabled()) {
-            LOGGER.debug("Completing and caching fetched resource {}, version {} to cache "
-                            + "(previously cached version {}, options={})",
-                    repositoryObject, repositoryObject.getVersion(), beans.resourceCache.getVersion(oid), options);
-        }
+        logResourceBeforeCompletion(repositoryObject, options);
 
         ResourceCompletionOperation completionOperation = new ResourceCompletionOperation(repositoryObject, options, task, beans);
         ResourceType completedResource = completionOperation.execute(result);
 
         logResourceAfterCompletion(completedResource);
 
         // TODO fix this diagnostics using member methods of the completion operation
-        if (!ResourceTypeUtil.isComplete(completedResource)) {
+        if (isAbstract(completedResource)) {
+            LOGGER.debug("Not putting {} into cache because it's abstract", completedResource);
+        } else if (!ResourceTypeUtil.isComplete(completedResource)) {
             // No not cache non-complete resources (e.g. those retrieved with noFetch)
             LOGGER.debug("Not putting {} into cache because it's not complete: hasSchema={}, hasCapabilitiesCached={}",
                     repositoryObject, hasSchema(completedResource), hasCapabilitiesCached(completedResource));
@@ -188,11 +179,27 @@ public class ResourceManager {
         return completedResource;
     }
 
+    private void logResourceBeforeCompletion(
+            @NotNull ResourceType repositoryObject, @Nullable GetOperationOptions options) {
+        if (!LOGGER.isDebugEnabled()) {
+            return;
+        }
+        if (isAbstract(repositoryObject)) {
+            LOGGER.debug("Partially completing fetched abstract resource {}, version {}",
+                    repositoryObject, repositoryObject.getVersion());
+        } else {
+            String oid = repositoryObject.getOid();
+            LOGGER.debug("Completing and caching fetched resource {}, version {} to cache "
+                            + "(previously cached version {}, options={})",
+                    repositoryObject, repositoryObject.getVersion(), beans.resourceCache.getVersion(oid), options);
+        }
+    }
+
     private void logResourceAfterCompletion(ResourceType completedResource) {
         if (!LOGGER.isTraceEnabled()) {
             return;
         }
-        LOGGER.trace("Resource after completion, before putting into cache:\n{}", completedResource.debugDump());
+        LOGGER.trace("Resource after completion, before (considering) putting into cache:\n{}", completedResource.debugDump());
         Element xsdSchemaElement = ResourceTypeUtil.getResourceXsdSchema(completedResource);
         if (xsdSchemaElement == null) {
             LOGGER.trace("Schema: null");

diff --git a/...g-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/TestDummyNegative.java b/...g-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/TestDummyNegative.java
@@ -16,6 +16,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 import com.evolveum.icf.dummy.resource.*;
@@ -26,6 +27,7 @@
 import com.evolveum.midpoint.repo.sqale.SqaleRepositoryService;
 import com.evolveum.midpoint.schema.*;
 import com.evolveum.midpoint.schema.constants.SchemaConstants;
+import com.evolveum.midpoint.test.TestResource;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
 
 import org.apache.commons.lang3.StringUtils;
@@ -75,7 +77,15 @@ public class TestDummyNegative extends AbstractDummyTest {
             TestDummyNegative::addFragileAttributes
     );
 
-    private static void addFragileAttributes(DummyResourceContoller controller) throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException {
+    private static final TestResource RESOURCE_WITH_BROKEN_CONNECTOR_REF = TestResource.file(
+            TEST_DIR, "resource-with-broken-connector-ref.xml", "fe1de27d-684c-44c8-9cf6-a691fdf392fd");
+    private static final TestResource RESOURCE_TEMPLATE_WITH_BROKEN_CONNECTOR_REF = TestResource.file(
+            TEST_DIR, "resource-template-with-broken-connector-ref.xml", "c41900ce-6b86-476a-a7c9-6eb797f7d405");
+    private static final TestResource RESOURCE_INSTANCE_WITH_BROKEN_CONNECTOR_REF = TestResource.file(
+            TEST_DIR, "resource-instance-with-broken-connector-ref.xml", "201e83f6-184e-46b0-92ec-16d1b639f6cb");
+
+    private static void addFragileAttributes(DummyResourceContoller controller)
+            throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException {
         // This gives us a potential to induce exceptions during ConnId->object conversion.
         controller.addAttrDef(controller.getDummyResource().getAccountObjectClass(),
                 ENABLE_DATE_NAME, Long.class, false, false);
@@ -90,7 +100,8 @@ private static void addFragileAttributes(DummyResourceContoller controller) thro
     private static final String GOOD_ACCOUNT = "good";
     private static final String INCONVERTIBLE_ACCOUNT = "inconvertible";
     private static final String UNSTORABLE_ACCOUNT = "unstorable";
-    private static final String TOTALLY_UNSTORABLE_ACCOUNT = "totally-unstorable" + StringUtils.repeat("-123456789", 30); // too large to be stored in DB
+    private static final String TOTALLY_UNSTORABLE_ACCOUNT =
+            "totally-unstorable" + StringUtils.repeat("-123456789", 30); // too large to be stored in DB
 
     private static final String EXTERNAL_UID_PREFIX = "uid:";
     private static final String GOOD_ACCOUNT_UID = EXTERNAL_UID_PREFIX + GOOD_ACCOUNT;
@@ -106,8 +117,81 @@ public void initSystem(Task initTask, OperationResult initResult) throws Excepti
 
         initDummyResource(RESOURCE_DUMMY_BROKEN_ACCOUNTS_EXTERNAL_UID, initResult);
         testResourceAssertSuccess(RESOURCE_DUMMY_BROKEN_ACCOUNTS_EXTERNAL_UID, initTask, initResult);
+
+        RESOURCE_WITH_BROKEN_CONNECTOR_REF.initRaw(this, initResult);
+        RESOURCE_TEMPLATE_WITH_BROKEN_CONNECTOR_REF.initRaw(this, initResult);
+        RESOURCE_INSTANCE_WITH_BROKEN_CONNECTOR_REF.initRaw(this, initResult);
     }
 
+    //region Broken connectorRef
+
+    /** MID-8619: resource with broken `connectorRef` must be gettable. */
+    @Test
+    public void test100GetResourceWithBrokenConnectorRef() throws Exception {
+        var task = getTestTask();
+        var result = task.getResult();
+
+        when("resource with broken connectorRef is retrieved");
+        var resource = provisioningService
+                .getObject(ResourceType.class, RESOURCE_WITH_BROKEN_CONNECTOR_REF.oid, null, task, result)
+                .asObjectable();
+
+        Consumer<OperationResult> asserter =
+                r -> assertThatOperationResult(r)
+                        .isPartialError()
+                        .hasMessageContaining("An error occurred while applying connector schema")
+                        .hasMessageContaining("Object of type 'ConnectorType' with OID 'cb4bcb48-4325-405d-a4a4-260de5640232' was not found.");
+
+        then("the result is partial error");
+        asserter.accept(result);
+
+        and("fetchResult is the same");
+        asserter.accept(OperationResult.createOperationResult(resource.getFetchResult()));
+    }
+
+    /** MID-8619: resource template with broken `connectorRef` must be gettable. */
+    @Test
+    public void test103GetResourceTemplateWithBrokenConnectorRef() throws Exception {
+        var task = getTestTask();
+        var result = task.getResult();
+
+        when("resource template with broken connectorRef is retrieved");
+        var resource = provisioningService
+                .getObject(ResourceType.class, RESOURCE_TEMPLATE_WITH_BROKEN_CONNECTOR_REF.oid, null, task, result)
+                .asObjectable();
+
+        then("the result is success (there's no reason to get the connector)");
+        assertSuccess(result);
+
+        and("it returns without fetchResult");
+        assertThat(resource.getFetchResult()).as("fetch result").isNull();
+    }
+
+    /** MID-8619: resource instance pointing to a template with broken `connectorRef` must be gettable. */
+    @Test
+    public void test106GetResourceInstanceWithBrokenConnectorRef() throws Exception {
+        var task = getTestTask();
+        var result = task.getResult();
+
+        when("instance of resource template with broken connectorRef is retrieved");
+        var resource = provisioningService
+                .getObject(ResourceType.class, RESOURCE_INSTANCE_WITH_BROKEN_CONNECTOR_REF.oid, null, task, result)
+                .asObjectable();
+
+        Consumer<OperationResult> asserter =
+                r -> assertThatOperationResult(r)
+                        .isPartialError()
+                        .hasMessageContaining("An error occurred while expanding super-resource references")
+                        .hasMessageContaining("Object of type 'ConnectorType' with OID 'cb4bcb48-4325-405d-a4a4-260de5640232' was not found.");
+
+        then("the result is partial error");
+        asserter.accept(result);
+
+        and("fetchResult is the same");
+        asserter.accept(OperationResult.createOperationResult(resource.getFetchResult()));
+    }
+    //endregion
+
     //region Tests for broken schema (in various ways)
     @Test
     public void test110GetResourceBrokenSchemaNetwork() throws Exception {
@@ -135,7 +219,8 @@ private void testGetResourceBrokenSchema(BreakMode breakMode) throws Exception {
         OperationResult result = createOperationResult();
 
         // precondition
-        PrismObject<ResourceType> repoResource = repositoryService.getObject(ResourceType.class, RESOURCE_DUMMY_OID, null, result);
+        PrismObject<ResourceType> repoResource =
+                repositoryService.getObject(ResourceType.class, RESOURCE_DUMMY_OID, null, result);
         display("Repo resource (before)", repoResource);
         PrismContainer<Containerable> schema = repoResource.findContainer(ResourceType.F_SCHEMA);
         assertTrue("Schema found in resource before the test (precondition)", schema == null || schema.isEmpty());
@@ -144,7 +229,8 @@ private void testGetResourceBrokenSchema(BreakMode breakMode) throws Exception {
         try {
 
             when();
-            PrismObject<ResourceType> resource = provisioningService.getObject(ResourceType.class, RESOURCE_DUMMY_OID, null, task, result);
+            PrismObject<ResourceType> resource =
+                    provisioningService.getObject(ResourceType.class, RESOURCE_DUMMY_OID, null, task, result);
 
             then();
             display("Resource with broken schema", resource);
@@ -176,7 +262,8 @@ public void test190GetResource() throws Exception {
         syncServiceMock.reset();
 
         when();
-        PrismObject<ResourceType> resource = provisioningService.getObject(ResourceType.class, RESOURCE_DUMMY_OID, null, task, result);
+        PrismObject<ResourceType> resource =
+                provisioningService.getObject(ResourceType.class, RESOURCE_DUMMY_OID, null, task, result);
 
         then();
         assertSuccess(result);

diff --git a/...rovisioning-impl/src/test/resources/dummy/resource-instance-with-broken-connector-ref.xml b/...rovisioning-impl/src/test/resources/dummy/resource-instance-with-broken-connector-ref.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Copyright (c) 2010-2018 Evolveum and contributors
+  ~
+  ~ This work is dual-licensed under the Apache License 2.0
+  ~ and European Union Public License. See LICENSE file for details.
+  -->
+
+<resource oid="201e83f6-184e-46b0-92ec-16d1b639f6cb"
+        xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3">
+    <name>resource-instance-with-broken-connector-ref</name>
+    <super>
+        <resourceRef oid="c41900ce-6b86-476a-a7c9-6eb797f7d405"/>
+    </super>
+</resource>
diff --git a/...rovisioning-impl/src/test/resources/dummy/resource-template-with-broken-connector-ref.xml b/...rovisioning-impl/src/test/resources/dummy/resource-template-with-broken-connector-ref.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Copyright (c) 2010-2018 Evolveum and contributors
+  ~
+  ~ This work is dual-licensed under the Apache License 2.0
+  ~ and European Union Public License. See LICENSE file for details.
+  -->
+
+<resource oid="c41900ce-6b86-476a-a7c9-6eb797f7d405"
+        xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3">
+    <name>resource-template-with-broken-connector-ref</name>
+    <template>true</template>
+    <abstract>true</abstract>
+    <connectorRef oid="cb4bcb48-4325-405d-a4a4-260de5640232"/> <!-- random value -->
+</resource>
diff --git a/...sioning/provisioning-impl/src/test/resources/dummy/resource-with-broken-connector-ref.xml b/...sioning/provisioning-impl/src/test/resources/dummy/resource-with-broken-connector-ref.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Copyright (c) 2010-2018 Evolveum and contributors
+  ~
+  ~ This work is dual-licensed under the Apache License 2.0
+  ~ and European Union Public License. See LICENSE file for details.
+  -->
+
+<resource oid="fe1de27d-684c-44c8-9cf6-a691fdf392fd"
+        xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3">
+    <name>resource-with-broken-connector-ref</name>
+    <connectorRef oid="cb4bcb48-4325-405d-a4a4-260de5640232"/> <!-- random value -->
+</resource>