Do minor code improvements

These precede major changes in "item value" autz specification. (Just to separate these modifications.)
Evolveum · May 18, 2023 · b1e0b70 · b1e0b70
1 parent 30e5108
commit b1e0b70
Show file tree

Hide file tree

Showing 15 changed files with 53 additions and 46 deletions.
diff --git a/...model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelController.java b/...model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelController.java
@@ -642,7 +642,7 @@ private <T extends ObjectType> void applyProvisioningDefinition(
         }
     }
 
-    private class ContainerOperationContext<T extends Containerable> {
+    private class ContainerSearchLikeOpContext<T extends Containerable> {
         final boolean isCertCase;
         final boolean isCaseMgmtWorkItem;
         final boolean isOperationExecution;
@@ -652,7 +652,7 @@ private class ContainerOperationContext<T extends Containerable> {
         private final boolean isAssignment;
 
         // TODO: task and result here are ugly and probably wrong
-        ContainerOperationContext(Class<T> type, ObjectQuery query, Task task, OperationResult result)
+        ContainerSearchLikeOpContext(Class<T> type, ObjectQuery query, Task task, OperationResult result)
                 throws SchemaException, SecurityViolationException, ObjectNotFoundException,
                 ExpressionEvaluationException, CommunicationException, ConfigurationException {
 
@@ -694,7 +694,7 @@ public <T extends Containerable> SearchResultList<T> searchContainers(
                 .addParam(OperationResult.PARAM_QUERY, query);
 
         try {
-            ContainerOperationContext<T> ctx = new ContainerOperationContext<>(type, query, task, result);
+            var ctx = new ContainerSearchLikeOpContext<>(type, query, task, result);
 
             var parsedOptions = preProcessOptionsSecurity(rawOptions, task, result);
             var options = parsedOptions.getCollection();
@@ -730,10 +730,6 @@ public <T extends Containerable> SearchResultList<T> searchContainers(
                     QNameUtil.setTemporarilyTolerateUndeclaredPrefixes(false);
                 }
 
-                if (list == null) {
-                    list = new SearchResultList<>(new ArrayList<>());
-                }
-
                 for (T object : list) {
                     // TODO implement read hook, if necessary
                     executeResolveOptions(object, parsedOptions, task, result);
@@ -743,15 +739,15 @@ public <T extends Containerable> SearchResultList<T> searchContainers(
             }
 
             if (ctx.isCertCase) {
-                list = schemaTransformer.applySchemasAndSecurityToContainers(
+                return schemaTransformer.applySchemasAndSecurityToContainerValues(
                         list, AccessCertificationCampaignType.class, AccessCertificationCampaignType.F_CASE,
                         parsedOptions, task, result);
             } else if (ctx.isCaseMgmtWorkItem || ctx.isOperationExecution || ctx.isAssignment || ctx.isProcessedObject) {
                 // TODO implement security post processing for CaseWorkItems
+                return list;
             } else {
                 throw new IllegalStateException();
             }
-            return list;
         } finally {
             result.close();
             result.cleanup();
@@ -772,7 +768,7 @@ public <T extends Containerable> Integer countContainers(Class<T> type, ObjectQu
                 .addParam(OperationResult.PARAM_QUERY, query);
 
         try {
-            ContainerOperationContext<T> ctx = new ContainerOperationContext<>(type, query, task, result);
+            var ctx = new ContainerSearchLikeOpContext<>(type, query, task, result);
             var parsedOptions = preProcessOptionsSecurity(rawOptions, task, result);
             var options = parsedOptions.getCollection();
             query = ctx.refinedQuery;
@@ -1060,16 +1056,16 @@ public SearchResultList<ObjectReferenceType> searchReferences(
 
         ModelImplUtils.validatePaging(query.getPaging());
 
-        OperationResult operationResult = parentResult.createSubresult(SEARCH_REFERENCES)
+        OperationResult result = parentResult.createSubresult(SEARCH_REFERENCES)
                 .addParam(OperationResult.PARAM_QUERY, query);
 
         try {
-            var parsedOptions = preProcessOptionsSecurity(rawOptions, task, operationResult);
+            var parsedOptions = preProcessOptionsSecurity(rawOptions, task, result);
             var options = parsedOptions.getCollection();
 
-            query = preProcessReferenceQuerySecurity(query, task, operationResult); // TODO not implemented yet!
+            query = preProcessReferenceQuerySecurity(query, task, result); // TODO not implemented yet!
 
-            if (isFilterNone(query, operationResult)) {
+            if (isFilterNone(query, result)) {
                 return new SearchResultList<>(new ArrayList<>());
             }
 
@@ -1079,9 +1075,9 @@ public SearchResultList<ObjectReferenceType> searchReferences(
             try {
                 logQuery(query);
 
-                list = cacheRepositoryService.searchReferences(query, options, operationResult);
+                list = cacheRepositoryService.searchReferences(query, options, result);
             } catch (SchemaException | RuntimeException e) {
-                recordSearchException(e, ObjectManager.REPOSITORY, operationResult);
+                recordSearchException(e, ObjectManager.REPOSITORY, result);
                 throw e;
             } finally {
                 exitModelMethod();
@@ -1090,8 +1086,8 @@ public SearchResultList<ObjectReferenceType> searchReferences(
             // TODO how does schemaTransformer.applySchemasAndSecurityToContainers apply to reference result?
             return list;
         } finally {
-            operationResult.close();
-            operationResult.cleanup();
+            result.close();
+            result.cleanup();
         }
     }
 

diff --git a/...del-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java b/...del-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java
@@ -168,7 +168,7 @@ private static <T extends ObjectType> PrismObject<T> updateFetchResult(PrismObje
     // Expecting that C is a direct child of T.
     // Expecting that container values point to their respective parents (in order to evaluate the security!)
     <C extends Containerable, T extends ObjectType>
-    SearchResultList<C> applySchemasAndSecurityToContainers(
+    SearchResultList<C> applySchemasAndSecurityToContainerValues(
             SearchResultList<C> originalResultList,
             Class<T> parentObjectType,
             ItemName childItemName,

diff --git a/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/util/mock/MockFactory.java b/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/util/mock/MockFactory.java
@@ -343,8 +343,8 @@ public <T extends Containerable> int countContainers(Class<T> type,
             }
 
             @Override
-            public <T extends Containerable> SearchResultList<T> searchContainers(Class<T> type,
-                    ObjectQuery query, Collection<SelectorOptions<GetOperationOptions>> options, OperationResult parentResult) {
+            public @NotNull <T extends Containerable> SearchResultList<T> searchContainers(@NotNull Class<T> type,
+                    @Nullable ObjectQuery query, @Nullable Collection<SelectorOptions<GetOperationOptions>> options, @NotNull OperationResult parentResult) {
                 return new SearchResultList<>(List.of());
             }
 

diff --git a/repo/repo-api/src/main/java/com/evolveum/midpoint/repo/api/RepositoryService.java b/repo/repo-api/src/main/java/com/evolveum/midpoint/repo/api/RepositoryService.java
@@ -351,8 +351,11 @@ <T extends Containerable> int countContainers(Class<T> type, ObjectQuery query,
     /**
      * Search for "sub-object" structures, i.e. containers.
      */
-    <T extends Containerable> SearchResultList<T> searchContainers(Class<T> type, ObjectQuery query,
-            Collection<SelectorOptions<GetOperationOptions>> options, OperationResult parentResult) throws SchemaException;
+    @NotNull <T extends Containerable> SearchResultList<T> searchContainers(
+            @NotNull Class<T> type,
+            @Nullable ObjectQuery query,
+            @Nullable Collection<SelectorOptions<GetOperationOptions>> options,
+            @NotNull OperationResult parentResult) throws SchemaException;
 
     /**
      * Reference count - currently supporting roleMembershipRef and linkRef search.

diff --git a/repo/repo-cache/src/main/java/com/evolveum/midpoint/repo/cache/RepositoryCache.java b/repo/repo-cache/src/main/java/com/evolveum/midpoint/repo/cache/RepositoryCache.java
@@ -130,8 +130,11 @@ public SearchResultMetadata searchReferencesIterative(
     }
 
     @Override
-    public <T extends Containerable> SearchResultList<T> searchContainers(Class<T> type, ObjectQuery query,
-            Collection<SelectorOptions<GetOperationOptions>> options, OperationResult parentResult) throws SchemaException {
+    public @NotNull <T extends Containerable> SearchResultList<T> searchContainers(
+            @NotNull Class<T> type,
+            @Nullable ObjectQuery query,
+            @Nullable Collection<SelectorOptions<GetOperationOptions>> options,
+            @NotNull OperationResult parentResult) throws SchemaException {
         return searchOpHandler.searchContainers(type, query, options, parentResult);
     }
 

diff --git a/repo/repo-cache/src/main/java/com/evolveum/midpoint/repo/cache/handlers/SearchOpHandler.java b/repo/repo-cache/src/main/java/com/evolveum/midpoint/repo/cache/handlers/SearchOpHandler.java
@@ -338,7 +338,7 @@ public <F extends FocusType> PrismObject<F> searchShadowOwner(
         }
     }
 
-    public <T extends Containerable> SearchResultList<T> searchContainers(Class<T> type,
+    public @NotNull <T extends Containerable> SearchResultList<T> searchContainers(Class<T> type,
             ObjectQuery query, Collection<SelectorOptions<GetOperationOptions>> options, OperationResult parentResult)
             throws SchemaException {
         OperationResult result = parentResult.subresult(OP_SEARCH_CONTAINERS)

diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java
@@ -1277,9 +1277,9 @@ private <T extends Containerable> int executeCountContainers(
     }
 
     @Override
-    public <T extends Containerable> SearchResultList<T> searchContainers(
-            Class<T> type, ObjectQuery query,
-            Collection<SelectorOptions<GetOperationOptions>> options, OperationResult parentResult)
+    public @NotNull <T extends Containerable> SearchResultList<T> searchContainers(
+            @NotNull Class<T> type, @Nullable ObjectQuery query,
+            @Nullable Collection<SelectorOptions<GetOperationOptions>> options, @NotNull OperationResult parentResult)
             throws SchemaException {
 
         Objects.requireNonNull(type, "Container type must not be null.");
@@ -1310,8 +1310,8 @@ public <T extends Containerable> SearchResultList<T> searchContainers(
         }
     }
 
-    private <T extends Containerable> SearchResultList<T> executeSearchContainers(Class<T> type,
-            ObjectQuery query, Collection<SelectorOptions<GetOperationOptions>> options)
+    private @NotNull <T extends Containerable> SearchResultList<T> executeSearchContainers(
+            Class<T> type, ObjectQuery query, Collection<SelectorOptions<GetOperationOptions>> options)
             throws RepositoryException, SchemaException {
 
         long opHandle = registerOperationStart(OP_SEARCH_CONTAINERS, type);

diff --git a/.../repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryServiceImpl.java b/.../repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryServiceImpl.java
@@ -287,8 +287,11 @@ private ObjectQuery replaceSimplifiedFilter(ObjectQuery query, ObjectFilter filt
     }
 
     @Override
-    public <T extends Containerable> SearchResultList<T> searchContainers(Class<T> type, ObjectQuery query,
-            Collection<SelectorOptions<GetOperationOptions>> options, OperationResult parentResult)
+    public @NotNull <T extends Containerable> SearchResultList<T> searchContainers(
+            @NotNull Class<T> type,
+            @Nullable ObjectQuery query,
+            @Nullable Collection<SelectorOptions<GetOperationOptions>> options,
+            @NotNull OperationResult parentResult)
             throws SchemaException {
         Validate.notNull(type, "Object type must not be null.");
         Validate.notNull(parentResult, "Operation result must not be null.");

diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectRetriever.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectRetriever.java
@@ -365,7 +365,7 @@ private <T extends ObjectType> List<PrismObject<T>> queryResultToPrismObjects(
         return rv;
     }
 
-    public <C extends Containerable> SearchResultList<C> searchContainersAttempt(Class<C> type, ObjectQuery query,
+    public @NotNull <C extends Containerable> SearchResultList<C> searchContainersAttempt(Class<C> type, ObjectQuery query,
             Collection<SelectorOptions<GetOperationOptions>> options, OperationResult result) throws SchemaException {
 
         boolean assignments = AssignmentType.class.equals(type);

diff --git a/repo/repo-sqlbase/src/main/java/com/evolveum/midpoint/repo/sqlbase/SqlQueryContext.java b/repo/repo-sqlbase/src/main/java/com/evolveum/midpoint/repo/sqlbase/SqlQueryContext.java
@@ -11,6 +11,7 @@
 import java.util.stream.Collectors;
 import javax.xml.namespace.QName;
 
+import com.querydsl.core.QueryMetadata;
 import com.querydsl.core.Tuple;
 import com.querydsl.core.types.Expression;
 import com.querydsl.core.types.OrderSpecifier;
@@ -328,13 +329,14 @@ public PageOf<Tuple> executeQuery(JdbcSession jdbcSession) throws QueryException
 
     private @NotNull Expression<?>[] buildSelectExpressions(Q entity, SQLQuery<?> query) {
         Path<?>[] defaultExpressions = entityPathMapping.selectExpressions(entity, options);
-        if (!query.getMetadata().isDistinct() || query.getMetadata().getOrderBy().isEmpty()) {
+        QueryMetadata metadata = query.getMetadata();
+        if (!metadata.isDistinct() || metadata.getOrderBy().isEmpty()) {
             return defaultExpressions;
         }
 
         // If DISTINCT is used with ORDER BY then anything in ORDER BY must be in SELECT too
         List<Expression<?>> expressions = new ArrayList<>(Arrays.asList(defaultExpressions));
-        for (OrderSpecifier<?> orderSpecifier : query.getMetadata().getOrderBy()) {
+        for (OrderSpecifier<?> orderSpecifier : metadata.getOrderBy()) {
             Expression<?> orderPath = orderSpecifier.getTarget();
             if (!expressions.contains(orderPath)) {
                 expressions.add(orderPath);

diff --git a/repo/repo-sqlbase/src/main/java/com/evolveum/midpoint/repo/sqlbase/SqlQueryExecutor.java b/repo/repo-sqlbase/src/main/java/com/evolveum/midpoint/repo/sqlbase/SqlQueryExecutor.java
@@ -50,7 +50,7 @@ public <S, Q extends FlexibleRelationalPathBase<R>, R> int count(
         }
     }
 
-    public <S, Q extends FlexibleRelationalPathBase<R>, R> SearchResultList<S> list(
+    public @NotNull <S, Q extends FlexibleRelationalPathBase<R>, R> SearchResultList<S> list(
             @NotNull SqlQueryContext<S, Q, R> context,
             ObjectQuery query,
             Collection<SelectorOptions<GetOperationOptions>> options)

diff --git a/...main/java/com/evolveum/midpoint/security/enforcer/impl/AuthorizationFilterEvaluation.java b/...main/java/com/evolveum/midpoint/security/enforcer/impl/AuthorizationFilterEvaluation.java
@@ -62,7 +62,7 @@ class AuthorizationFilterEvaluation<O extends ObjectType> extends AuthorizationE
         this.selectorLabel = selectorLabel;
     }
 
-    public boolean evaluate()
+    boolean computeFilter()
             throws SchemaException, ExpressionEvaluationException, CommunicationException, SecurityViolationException,
             ConfigurationException, ObjectNotFoundException {
 

diff --git a/...src/main/java/com/evolveum/midpoint/security/enforcer/impl/EnforcerDecisionOperation.java b/...src/main/java/com/evolveum/midpoint/security/enforcer/impl/EnforcerDecisionOperation.java
@@ -58,7 +58,7 @@ class EnforcerDecisionOperation<O extends ObjectType, T extends ObjectType>
         this.applicableAutzConsumer = applicableAutzConsumer;
     }
 
-    @NotNull AccessDecision execute(@Nullable AuthorizationPhaseType phase, OperationResult result)
+    @NotNull AccessDecision decideAccess(@Nullable AuthorizationPhaseType phase, OperationResult result)
             throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException,
             CommunicationException, ConfigurationException, SecurityViolationException {
         if (phase == null) {

diff --git a/...l/src/main/java/com/evolveum/midpoint/security/enforcer/impl/EnforcerFilterOperation.java b/...l/src/main/java/com/evolveum/midpoint/security/enforcer/impl/EnforcerFilterOperation.java
@@ -87,11 +87,11 @@ F computeSecurityFilter(@Nullable AuthorizationPhaseType phase, OperationResult
         traceOperationStart();
         F securityFilter;
         if (phase != null) {
-            securityFilter = new Phase(phase, false).compute(result);
+            securityFilter = new Phase(phase, false).computeFilter(result);
         } else {
-            F filterBoth = new Phase(null, true).compute(result); // includeNullPhase is irrelevant here
-            F filterRequest = new Phase(REQUEST, true).compute(result);
-            F filterExecution = new Phase(EXECUTION, true).compute(result);
+            F filterBoth = new Phase(null, true).computeFilter(result); // partialCheck is irrelevant here
+            F filterRequest = new Phase(REQUEST, true).computeFilter(result);
+            F filterExecution = new Phase(EXECUTION, true).computeFilter(result);
             securityFilter =
                     gizmo.or(
                             filterBoth,
@@ -128,7 +128,7 @@ private class Phase {
          *
          * See also {@link SelectorToFilterTranslator}
          */
-        private F compute(OperationResult result)
+        private F computeFilter(OperationResult result)
                 throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException, CommunicationException,
                 ConfigurationException, SecurityViolationException {
 
@@ -164,7 +164,7 @@ private F compute(OperationResult result)
                     continue;
                 }
 
-                var applicable = autzEvaluation.evaluate();
+                var applicable = autzEvaluation.computeFilter();
 
                 if (applicable) {
                     F autzSecurityFilter =

diff --git a/...impl/src/main/java/com/evolveum/midpoint/security/enforcer/impl/SecurityEnforcerImpl.java b/...impl/src/main/java/com/evolveum/midpoint/security/enforcer/impl/SecurityEnforcerImpl.java
@@ -159,7 +159,7 @@ private String getQuotedUsername(MidPointPrincipal principal) {
             CommunicationException, ConfigurationException, SecurityViolationException {
         return new EnforcerDecisionOperation<>(
                 operationUrl, params, applicableAutzConsumer, principal, ownerResolver, beans, task)
-                .execute(phase, result);
+                .decideAccess(phase, result);
     }
 
     @Override