support for import object with oid and overwrite option (MID-5730)

Evolveum · Mar 26, 2020 · 57386c5 · 57386c5
1 parent 7d13d2f
commit 57386c5
Show file tree

Hide file tree

Showing 9 changed files with 170 additions and 41 deletions.
diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectQueryUtil.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ObjectQueryUtil.java
@@ -71,6 +71,16 @@ public static ObjectQuery createNameQuery(PolyString name, PrismContext prismCon
                 .build();
     }
 
+    public static <O extends ObjectType> ObjectQuery createOidQuery(PrismObject<O> object) throws SchemaException {
+        return createOidQuery(object.getOid(), object.getPrismContext());
+    }
+
+    public static ObjectQuery createOidQuery(String oid, PrismContext prismContext) throws SchemaException {
+        return prismContext.queryFor(ObjectType.class)
+                .id(oid)
+                .build();
+    }
+
     public static ObjectQuery createOrigNameQuery(PolyString name, PrismContext prismContext) throws SchemaException {
         return prismContext.queryFor(ObjectType.class)
                 .item(ObjectType.F_NAME).eq(name).matchingOrig()

diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/importer/ObjectImporter.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/importer/ObjectImporter.java
@@ -325,34 +325,64 @@ private <T extends ObjectType> void importObjectToRepository(PrismObject<T> obje
             result.recordSuccess();
 
         } catch (ObjectAlreadyExistsException e) {
-            if (isTrue(options.isOverwrite()) && isNotTrue(options.isKeepOid()) && object.getOid() == null) {
+            if (isTrue(options.isOverwrite()) && isNotTrue(options.isKeepOid())) {
                    // This is overwrite, without keep oid, therefore we do not have conflict on OID
                 // this has to be conflict on name. So try to delete the conflicting object and create new one (with a new OID).
                 result.muteLastSubresultError();
-                ObjectQuery query = ObjectQueryUtil.createNameQuery(object);
-                List<PrismObject<T>> foundObjects = repository.searchObjects(object.getCompileTimeClass(), query, null, result);
-                if (foundObjects.size() == 1) {
-                    PrismObject<T> foundObject = foundObjects.iterator().next();
-                    String deletedOid = deleteObject(foundObject, repository, result);
-                       if (deletedOid != null) {
-                           if (object.canRepresent(TaskType.class)) {
-                               taskManager.onTaskDelete(deletedOid, result);
-                           }
-                           if (isTrue(options.isKeepOid())) {
-                               object.setOid(deletedOid);
-                           }
-                           addObject(object, false, options, task, result);
-                           result.recordSuccess();
-                       } else {
-                           // cannot delete, throw original exception
-                        result.recordFatalError("Object already exists, cannot overwrite", e);
-                        throw e;
-                       }
+                PrismObject<T> foundObject;
+                if (object.getOid() == null) {
+                    ObjectQuery query = ObjectQueryUtil.createNameQuery(object);
+                    List<PrismObject<T>> foundObjects = repository.searchObjects(object.getCompileTimeClass(), query, null, result);
+                    if (foundObjects.size() != 1) {
+                        // Cannot locate conflicting object
+                        String message = "Conflicting object already exists but it was not possible to precisely locate it, "+foundObjects.size()+" objects with same name exist";
+                        result.recordFatalError(message, e);
+                        throw new ObjectAlreadyExistsException(message, e);
+                    }
+                    foundObject = foundObjects.iterator().next();
+                } else {
+                    ObjectQuery queryByName = ObjectQueryUtil.createNameQuery(object);
+                    List<PrismObject<T>> foundObjectsByName = repository.searchObjects(object.getCompileTimeClass(), queryByName, null, result);
+                    ObjectQuery queryByOid = ObjectQueryUtil.createOidQuery(object);
+                    List<PrismObject<T>> foundObjectsByOid = repository.searchObjects(object.getCompileTimeClass(), queryByOid, null, result);
+                    if (foundObjectsByName.size() == 1 && foundObjectsByOid.isEmpty()) {
+                        foundObject = foundObjectsByName.iterator().next();
+                    } else if (foundObjectsByName.isEmpty() && foundObjectsByOid.size() == 1) {
+                        foundObject = foundObjectsByOid.iterator().next();
+                    } else if (foundObjectsByName.size() == 1 && foundObjectsByOid.size() == 1) {
+                        PrismObject<T> foundObjectByName = foundObjectsByName.iterator().next();
+                        PrismObject<T> foundObjectByOid = foundObjectsByOid.iterator().next();
+                        if (foundObjectByName.getOid().equals(foundObjectByOid.getOid())) {
+                            foundObject = foundObjectByName;
+                        } else {
+                            String message = "Conflicting object already exists but it was not possible to precisely locate it, found object by name "+foundObjectByName.getName().getOrig()+
+                                    "(oid:"+foundObjectByName.getOid()+") and found object by oid "+foundObjectByOid.getName().getOrig()+"(oid:"+foundObjectByOid.getOid()+") not same";
+                            result.recordFatalError(message, e);
+                            throw new ObjectAlreadyExistsException(message, e);
+                        }
+                    } else {
+                        String message = "Conflicting object already exists but it was not possible to precisely locate it, "+foundObjectsByName.size()+" objects with same name exist and "+
+                                foundObjectsByOid.size()+" objects with same oid exist";
+                        result.recordFatalError(message, e);
+                        throw new ObjectAlreadyExistsException(message, e);
+                    }
+                }
+
+                String deletedOid = deleteObject(foundObject, repository, result);
+                if (deletedOid != null) {
+                    if (object.canRepresent(TaskType.class)) {
+                        taskManager.onTaskDelete(deletedOid, result);
+                    }
+                    if (isTrue(options.isKeepOid())) {
+                        object.setOid(deletedOid);
+                    }
+                    addObject(object, false, options, task, result);
+                    result.recordSuccess();
+                    // cannot delete, throw original exception
+
                 } else {
-                    // Cannot locate conflicting object
-                    String message = "Conflicting object already exists but it was not possible to precisely locate it, "+foundObjects.size()+" objects with same name exist";
-                    result.recordFatalError(message, e);
-                     throw new ObjectAlreadyExistsException(message, e);
+                    result.recordFatalError("Object already exists, cannot overwrite", e);
+                    throw e;
                 }
             } else {
                 result.recordFatalError(e);

diff --git a/...-intest/src/test/java/com/evolveum/midpoint/model/intest/importer/AbstractImportTest.java b/...-intest/src/test/java/com/evolveum/midpoint/model/intest/importer/AbstractImportTest.java
@@ -238,11 +238,11 @@ public void test003ImportUsers() throws Exception {
         PrismAsserts.assertEqualsPolyString("wrong fullName", "Guybrush Threepwood", guybrush.getFullName());
         assertMetadata(guybrush, startTime, endTime);
 
-        assertUsers(4);
+        assertUsers(6);
 
         // Check audit
         display("Audit", dummyAuditService);
-        dummyAuditService.assertRecords(6);
+        dummyAuditService.assertRecords(10);
     }
 
     // Import the same thing again. Watch how it burns :-)
@@ -270,11 +270,11 @@ public void test004DuplicateImportUsers() throws Exception {
             assertFalse("Unexpected success in subresult", subresult.isSuccess());
         }
 
-        assertUsers(4);
+        assertUsers(6);
 
         // Check audit
         display("Audit", dummyAuditService);
-        dummyAuditService.assertRecords(6);        // 3 requests + 3 failed executions
+        dummyAuditService.assertRecords(10);        // 5 requests + 5 failed executions
     }
 
     // Import the same thing again, this time with overwrite option. This should go well.
@@ -300,7 +300,7 @@ public void test005ImportUsersWithOverwrite() throws Exception {
         // list all users
         List<PrismObject<UserType>> users = modelService.searchObjects(UserType.class, prismContext.queryFactory().createQuery(), null, task, result);
         // Three old users, one new
-        assertEquals(5, users.size());
+        assertEquals(7, users.size());
 
         for (PrismObject<UserType> user : users) {
             UserType userType = user.asObjectable();
@@ -332,11 +332,11 @@ public void test005ImportUsersWithOverwrite() throws Exception {
             }
         }
 
-        assertUsers(5);
+        assertUsers(7);
 
         // Check audit
         display("Audit", dummyAuditService);
-        dummyAuditService.assertRecords(8);        // 1 failed, 7 succeeded
+        dummyAuditService.assertRecords(14);        // 7 request, 2 failed, 5 succeeded
     }
 
     // Import the same thing again, with overwrite and also while keeping OIDs
@@ -363,7 +363,7 @@ public void test006ImportUsersWithOverwriteKeepOid() throws Exception {
         // list all users
         List<PrismObject<UserType>> users = modelService.searchObjects(UserType.class, prismContext.queryFactory().createQuery(), null, task, result);
         // Three old users, one new
-        assertEquals(5, users.size());
+        assertEquals(7, users.size());
 
         for (PrismObject<UserType> user : users) {
             UserType userType = user.asObjectable();
@@ -393,11 +393,11 @@ public void test006ImportUsersWithOverwriteKeepOid() throws Exception {
             }
         }
 
-        assertUsers(5);
+        assertUsers(7);
 
         // Check audit
         display("Audit", dummyAuditService);
-        dummyAuditService.assertRecords(6);
+        dummyAuditService.assertRecords(10);  // 5 request, 5 succeeded
     }
 
     @Test
@@ -666,7 +666,7 @@ public void test040ImportUserHermanNoEncryption() throws Exception {
         assertEquals("Er? Pirate sectrets still hidden?", "m0nk3y", protectedString.getClearValue());
         assertNull("Er? Encrypted data together with clear value?", protectedString.getEncryptedDataType());
 
-        assertUsers(6);
+        assertUsers(8);
 
         // Check audit
         display("Audit", dummyAuditService);
@@ -705,7 +705,7 @@ public void test050ImportUserHermanOverwriteFullProcessing() throws Exception {
         display("Herman", userHerman);
         assertUser(userHerman, USER_HERMAN_OID, USER_HERMAN_USERNAME, "Herman Toothrot", "Herman", "Toothrot");
 
-        assertUsers(6);
+        assertUsers(8);
 
         // Check audit
         display("Audit", dummyAuditService);
@@ -744,7 +744,7 @@ public void test060ImportConstrainedWrongFullProcessing() throws Exception {
             assertFalse("Unexpected success in subresult", subresult.isSuccess());
         }
 
-        assertUsers(6);     // none should be added
+        assertUsers(8);     // none should be added
 
         // Check audit
         display("Audit", dummyAuditService);
@@ -777,7 +777,7 @@ public void test070ImportConstrainedWrong() throws Exception {
         display("Result after import", result);
         TestUtil.assertSuccess("Import has failed (result)", result);
 
-        assertUsers(7);     // one should be added
+        assertUsers(9);     // one should be added
 
         // Check audit
         display("Audit", dummyAuditService);
@@ -836,7 +836,7 @@ public void test200BadImport() throws Exception {
             AssertJUnit.fail("Jack was not imported");
         }
 
-        assertUsers(8);
+        assertUsers(10);
     }
 
     /**
@@ -859,7 +859,7 @@ public void test210ImportRoleOneLegacyDefault() throws Exception {
 
         assertNoObject(RoleType.class, ROLE_ONE_LEGACY_OID);
 
-        assertUsers(8);
+        assertUsers(10);
     }
 
     /**
@@ -884,7 +884,7 @@ public void test212ImportRoleOneLegacyCompat() throws Exception {
 
         assertRoleAfter(ROLE_ONE_LEGACY_OID);
 
-        assertUsers(8);
+        assertUsers(10);
     }
 
     private void assertDummyResource(PrismObject<ResourceType> resource, boolean fromRepo) {

diff --git a/model/model-intest/src/test/resources/importer/import-users-overwrite.json b/model/model-intest/src/test/resources/importer/import-users-overwrite.json
@@ -24,5 +24,22 @@
       "givenName" : "Herman",
       "familyName" : "Toothrot"
     }
+  }, {
+    "user" : {
+      "oid" : "c0c010c0-d34d-b33f-f00d-111111111113",
+      "name" : "elizabethT",
+      "fullName" : "Elizabeth Turner",
+      "givenName" : "Elizabeth",
+      "familyName" : "Turner"
+    }
+  }, {
+    "user" : {
+      "oid" : "12345",
+      "name" : "james",
+      "fullName" : "James Norrington",
+      "givenName" : "James",
+      "familyName" : "Norrington",
+      "subtype" : "admiral"
+    }
   } ]
 }
diff --git a/model/model-intest/src/test/resources/importer/import-users-overwrite.xml b/model/model-intest/src/test/resources/importer/import-users-overwrite.xml
@@ -38,4 +38,21 @@
         <familyName>Toothrot</familyName>
     </user>
 
+    <!-- match oid -->
+    <user oid="c0c010c0-d34d-b33f-f00d-111111111113">
+        <name>elizabethT</name>
+        <fullName>Elizabeth Turner</fullName>
+        <givenName>Elizabeth</givenName>
+        <familyName>Turner</familyName>
+    </user>
+
+    <!-- match name -->
+    <user oid="123345">
+        <name>james</name>
+        <subtype>admiral</subtype>
+        <fullName>James Norrington</fullName>
+        <givenName>James</givenName>
+        <familyName>Norrington</familyName>
+    </user>
+
 </objects>
diff --git a/model/model-intest/src/test/resources/importer/import-users-overwrite.yaml b/model/model-intest/src/test/resources/importer/import-users-overwrite.yaml
@@ -19,3 +19,16 @@ objects:
     fullName: "Herman Toothrot"
     givenName: "Herman"
     familyName: "Toothrot"
+- user:
+    oid: "c0c010c0-d34d-b33f-f00d-111111111113"
+    name: "elizabethT"
+    fullName: "Elizabeth Turner"
+    givenName: "Elizabeth"
+    familyName: "Turner"
+- user:
+    oid: "12345"
+    name: "james"
+    fullName: "James Norrington"
+    givenName: "James"
+    familyName: "Norrington"
+    subtype: "admiral"
diff --git a/model/model-intest/src/test/resources/importer/import-users.json b/model/model-intest/src/test/resources/importer/import-users.json
@@ -37,5 +37,20 @@
       "givenName" : "Guybrush",
       "familyName" : "Threepwood"
     }
+  }, {
+    "user" : {
+      "oid" : "c0c010c0-d34d-b33f-f00d-111111111113",
+      "name" : "elizabethS",
+      "fullName" : "Elizabeth Swann",
+      "givenName" : "Elizabeth",
+      "familyName" : "Swann"
+    }
+  }, {
+    "user" : {
+      "name" : "james",
+      "fullName" : "James Norrington",
+      "givenName" : "James",
+      "familyName" : "Norrington"
+    }
   } ]
 }
diff --git a/model/model-intest/src/test/resources/importer/import-users.xml b/model/model-intest/src/test/resources/importer/import-users.xml
@@ -49,4 +49,20 @@
         <familyName>Threepwood</familyName>
     </user>
 
+    <!-- match oid -->
+    <user oid="c0c010c0-d34d-b33f-f00d-111111111113">
+        <name>elizabethS</name>
+        <fullName>Elizabeth Swann</fullName>
+        <givenName>Elizabeth</givenName>
+        <familyName>Swann</familyName>
+    </user>
+
+    <!-- match name -->
+    <user>
+        <name>james</name>
+        <fullName>James Norrington</fullName>
+        <givenName>James</givenName>
+        <familyName>Norrington</familyName>
+    </user>
+
 </objects>
diff --git a/model/model-intest/src/test/resources/importer/import-users.yaml b/model/model-intest/src/test/resources/importer/import-users.yaml
@@ -28,3 +28,14 @@ objects:
     fullName: "Guybrush Threepwood"
     givenName: "Guybrush"
     familyName: "Threepwood"
+- user:
+    oid: "c0c010c0-d34d-b33f-f00d-111111111113"
+    name: "elizabethS"
+    fullName: "Elizabeth Swann"
+    givenName: "Elizabeth"
+    familyName: "Swann"
+- user:
+    name: "james"
+    fullName: "James Norrington"
+    givenName: "James"
+    familyName: "Norrington"