RepositoryService: fixes for isAncestor/Descendant, better Javadoc

Old repo: isAncestor is not reflexive anymore (isDescendant never was),
SecurityTest* pass fine.
Added OP_IS_DESCENDANT/ANCESTOR used in new repo.
New repo tests update/cleanup.

repo/repo-api/src/main/java/com/evolveum/midpoint/repo/api/RepositoryService.java

@@ -117,7 +117,9 @@ public interface RepositoryService {
     String OP_MODIFY_OBJECT = "modifyObject";
     String OP_MODIFY_OBJECT_DYNAMICALLY = "modifyObjectDynamically";
     String OP_GET_VERSION = "getVersion";
-    String OP_IS_ANY_SUBORDINATE = "isAnySubordinate";
+    String OP_IS_ANY_SUBORDINATE = "isAnySubordinate"; // TODO remove with old repo, not public op anymore
+    String OP_IS_DESCENDANT = "isDescendant";
+    String OP_IS_ANCESTOR = "isAncestor";
     String OP_ADVANCE_SEQUENCE = "advanceSequence";
     String OP_RETURN_UNUSED_VALUES_TO_SEQUENCE = "returnUnusedValuesToSequence";
     String OP_EXECUTE_QUERY_DIAGNOSTICS = "executeQueryDiagnostics";
@@ -443,11 +445,17 @@ <T extends ObjectType> SearchResultMetadata searchObjectsIterative(Class<T> type
             throws SchemaException;
 
     /**
-     * Returns `true` if the `object` is under the organization identified with `ancestorOrg`.
-     * For this method *organization is NOT under itself*, that is `isDescendant(org, oidOfThatOrg)`
-     * returns `false` - which is not a symmetric behavior with {@link #isAncestor(PrismObject, String)}.
-     * On the other hand, the `object` here can be non-organization as the actual tested objects
-     * are targets of its `parentOrgRefs`.
+     * Returns `true` if the `object` is under the organization identified with `ancestorOrgOid`.
+     * The `object` can either be an Org or any other object in which case all the targets
+     * of its `parentOrgRefs` are tested.
+     *
+     * Examples (from the perspective of the first parameter):
+     *
+     * * User belonging to Org with `ancestorOrgOid` returns true.
+     * * Organization under Org with `ancestorOrgOid` returns true (in any depth).
+     * * User belonging to Org under another Org with `ancestorOrgOid` returns true (any depth).
+     * * Organization with `ancestorOrgOid` returns `false`, as it is not considered
+     * to be its own descendant.
      *
      * @param object object of any type tested to belong under Org with `ancestorOrgOid`
      * @param ancestorOrgOid identifier of ancestor organization
@@ -456,12 +464,20 @@ <O extends ObjectType> boolean isDescendant(PrismObject<O> object, String ancest
             throws SchemaException;
 
     /**
-     * Returns `true` if the organization identified with `descendantOrgOid` is under `object`.
-     * TODO reconsider reflexivity!
-     * For this method organization is under itself, that is `isAncestor(org, oidOfThatOrg)`
-     * returns `true`.
+     * Returns `true` if the `object` is above organization identified with `descendantOrgOid`.
      * Despite type parameter, only `PrismObject<OrgType>` can return `true`.
      *
+     * Examples (from the perspective of the first parameter):
+     *
+     * * Any other type than `Org` used for `object` returns `false`.
+     * * Organization being a parent of another organization with `descendantOrgOid` returns `true`.
+     * This means that Organization with `descendantOrgOid` has `parentOrgRef` to `object`.
+     * * Organization higher in the organization hierarchy than Org with `descendantOrgOid`
+     * returns `true`, for any number of levels between them as long as it's possible to traverse
+     * from Org identified by `descendantOrgOid` to `object` using any number of `parentOrgRefs`.
+     * * Organization with `descendantOrgOid` returns `false`, as it is not considered
+     * to be its own ancestor.
+     *
      * @param object potential ancestor organization
      * @param descendantOrgOid identifier of potential descendant organization
      */

repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoAddDeleteObjectTest.java

@@ -884,7 +884,7 @@ public void test320AddShadowWithAttributes()
         addExtensionValue(extensionContainer, "string", "string-value");
 
         ShadowAttributesType attributesContainer = new ShadowAttributesHelper(object)
-                .set(new QName("http://example.com/p", "string-mv"), DOMUtil.XSD_STRING,
+                .set(new QName("https://example.com/p", "string-mv"), DOMUtil.XSD_STRING,
                         "string-value1", "string-value2")
                 .attributesContainer();
 

repo/repo-sqale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoSearchTest.java

@@ -21,7 +21,6 @@
 import java.math.BigInteger;
 import java.time.Instant;
 import java.util.Arrays;
-import java.util.List;
 import java.util.UUID;
 import java.util.function.Function;
 import javax.xml.namespace.QName;
@@ -83,7 +82,6 @@ public class SqaleRepoSearchTest extends SqaleRepoBaseTest {
     private String task1Oid; // task has more attribute type variability
     private String task2Oid;
     private String shadow1Oid; // shadow with owner
-    private String service1Oid; // object with integer attribute
     private String case1Oid; // Closed case, two work items
     private String accCertCampaign1Oid;
 
@@ -148,7 +146,7 @@ public void initObjects() throws Exception {
                 .objectClass(SchemaConstants.RI_ACCOUNT_OBJECT_CLASS)
                 .extension(new ExtensionType(prismContext));
         addExtensionValue(shadow1.getExtension(), "string", "string-value");
-        ItemName shadowAttributeName = new ItemName("http://example.com/p", "string-mv");
+        ItemName shadowAttributeName = new ItemName("https://example.com/p", "string-mv");
         ShadowAttributesHelper attributesHelper = new ShadowAttributesHelper(shadow1)
                 .set(shadowAttributeName, DOMUtil.XSD_STRING, "string-value1", "string-value2");
         shadowAttributeDefinition = attributesHelper.getDefinition(shadowAttributeName);
@@ -295,11 +293,6 @@ public void initObjects() throws Exception {
                         .asPrismObject(),
                 null, result);
 
-        service1Oid = repositoryService.addObject(
-                new ServiceType(prismContext).name("service-1")
-                        // TODO integer attribute
-                        .asPrismObject(),
-                null, result);
         case1Oid = repositoryService.addObject(
                 new CaseType(prismContext).name("case-1")
                         .state("closed")
@@ -1458,7 +1451,7 @@ public void test590SearchObjectWithAssignmentExtension() throws SchemaException
     public void test591SearchShadowWithAttribute() throws SchemaException {
         searchObjectTest("with assignment extension item equal to value", ShadowType.class,
                 f -> f.itemWithDef(shadowAttributeDefinition,
-                        ShadowType.F_ATTRIBUTES, new QName("http://example.com/p", "string-mv"))
+                        ShadowType.F_ATTRIBUTES, new QName("https://example.com/p", "string-mv"))
                         .eq("string-value2"),
                 shadow1Oid);
     }
@@ -1704,7 +1697,7 @@ public void test970IsAncestor() throws Exception {
         expect("isAncestor returns true for parent-descendant (deep child) orgs");
         assertTrue(repositoryService.isAncestor(rootOrg, org111Oid));
 
-        expect("isAncestor returns false for the same org");
+        expect("isAncestor returns false for the same org (not ancestor of itself)");
         assertFalse(repositoryService.isAncestor(rootOrg, org1Oid));
 
         expect("isAncestor returns false for unrelated orgs");
@@ -1730,7 +1723,7 @@ public void test971IsDescendant() throws Exception {
                 repositoryService.getObject(OrgType.class, org112Oid, null, operationResult),
                 org1Oid));
 
-        expect("isDescendant returns false for the same org");
+        expect("isDescendant returns false for the same org (not descendant of itself)");
         assertFalse(repositoryService.isDescendant(org11, org11Oid));
 
         expect("isDescendant returns false for unrelated orgs");

repo/repo-sqale/src/test/resources/schema/extension.xsd

@@ -7,7 +7,7 @@
   -->
 
 <xsd:schema elementFormDefault="qualified"
-        targetNamespace="http://example.com/p"
+        targetNamespace="https://example.com/p"
         xmlns:c="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
         xmlns:a="http://prism.evolveum.com/xml/ns/public/annotation-3"
         xmlns:xsd="http://www.w3.org/2001/XMLSchema"

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryServiceImpl.java

@@ -1259,12 +1259,12 @@ public <O extends ObjectType> boolean isDescendant(
     @Override
     public <O extends ObjectType> boolean isAncestor(
             PrismObject<O> object, String descendantOrgOid) {
-        if (object.getOid() == null) {
+        // object is not considered ancestor of itself
+        if (object.getOid() == null || object.getOid().equals(descendantOrgOid)) {
             return false;
         }
-        Collection<String> oidList = new ArrayList<>(1);
-        oidList.add(descendantOrgOid);
-        return isAnySubordinate(object.getOid(), oidList);
+
+        return isAnySubordinate(object.getOid(), List.of(descendantOrgOid));
     }
 
     @Override