SqaleTableMapping contracts clarified, but some TODO still left there

QueryTableMapping#toSchemaObject(): added JdbcSession parameter
5-param toSchemaObject renamed to toSchemaObjectInternal, should go away
later, when we're done with forceFull flag.
Conversion method with ref-name resolution named toSchemaObjectComplete.

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java

@@ -222,7 +222,7 @@ private <S extends ObjectType> S readByOid(
             throw new ObjectNotFoundException(schemaType, oid.toString());
         }
 
-        return rootMapping.toSchemaObjectWithResolvedNames(result, root, options, jdbcSession, false);
+        return rootMapping.toSchemaObjectComplete(result, root, options, jdbcSession, false);
     }
 
     @Override
@@ -706,7 +706,7 @@ RootUpdateContext<S, Q, R> prepareUpdateContext(
             throw new ObjectNotFoundException(schemaType, oid.toString());
         }
 
-        S object = rootMapping.toSchemaObjectWithResolvedNames(
+        S object = rootMapping.toSchemaObjectComplete(
                 result, entityPath, getOptions, jdbcSession, RepoModifyOptions.isForceReindex(options));
 
         R rootRow = rootMapping.newRowObject();

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/qmodel/QAuditDeltaMapping.java

@@ -17,6 +17,7 @@
 import com.evolveum.midpoint.prism.polystring.PolyString;
 import com.evolveum.midpoint.repo.sqale.SqaleRepoContext;
 import com.evolveum.midpoint.repo.sqale.mapping.SqaleTableMapping;
+import com.evolveum.midpoint.repo.sqlbase.JdbcSession;
 import com.evolveum.midpoint.schema.GetOperationOptions;
 import com.evolveum.midpoint.schema.SelectorOptions;
 import com.evolveum.midpoint.util.exception.SchemaException;
@@ -90,7 +91,7 @@ private <T> T parseBytes(byte[] bytes, String identifier, Class<T> clazz) {
 
     @Override
     public ObjectDeltaOperationType toSchemaObject(
-            Tuple row, QAuditDelta entityPath, Collection<SelectorOptions<GetOperationOptions>> options)
+            @NotNull Tuple row, @NotNull QAuditDelta entityPath, @NotNull JdbcSession jdbcSession, Collection<SelectorOptions<GetOperationOptions>> options)
             throws SchemaException {
         throw new UnsupportedOperationException();
     }

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/qmodel/QAuditEventRecordMapping.java

@@ -400,7 +400,7 @@ public void beforeTransformation(List<Tuple> rowTuples, QAuditEventRecord entity
             @Override
             public AuditEventRecordType transform(Tuple tuple, QAuditEventRecord entityPath,
                     Collection<SelectorOptions<GetOperationOptions>> options) {
-                return toSchemaObjectSafe(tuple, entityPath, options, jdbcSession, false);
+                return toSchemaObjectCompleteSafe(tuple, entityPath, options, jdbcSession, false);
             }
         };
     }

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/qmodel/QAuditRefValueMapping.java

@@ -16,6 +16,7 @@
 
 import com.evolveum.midpoint.repo.sqale.SqaleRepoContext;
 import com.evolveum.midpoint.repo.sqale.mapping.SqaleTableMapping;
+import com.evolveum.midpoint.repo.sqlbase.JdbcSession;
 import com.evolveum.midpoint.schema.GetOperationOptions;
 import com.evolveum.midpoint.schema.SelectorOptions;
 import com.evolveum.midpoint.util.exception.SchemaException;
@@ -57,7 +58,7 @@ public AuditEventRecordReferenceType toSchemaObject(MAuditRefValue row) {
 
     @Override
     public AuditEventRecordReferenceType toSchemaObject(
-            Tuple row, QAuditRefValue entityPath, Collection<SelectorOptions<GetOperationOptions>> options)
+            @NotNull Tuple row, @NotNull QAuditRefValue entityPath, @NotNull JdbcSession jdbcSession, Collection<SelectorOptions<GetOperationOptions>> options)
             throws SchemaException {
         throw new UnsupportedOperationException(); // implemented in service
     }

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/mapping/SqaleTableMapping.java

@@ -281,9 +281,17 @@ public S toSchemaObject(R row) throws SchemaException {
 
     /**
      * Transforms row Tuple containing {@link R} under entity path and extension columns.
+     * While public, for Sqale repo it should only be called for internal mapping purposes.
+     *
+     * *Do not call this in result list transformers* because the results would not have resolved reference names (if requested).
+     * Notice that the default implementation of {@link #createRowTransformer} in this class calls
+     * {@link #toSchemaObjectCompleteSafe} which is the right thing to call in result list transformers.
      */
     @Override
-    public S toSchemaObject(Tuple tuple, Q entityPath,
+    public S toSchemaObject(
+            @NotNull Tuple tuple,
+            @NotNull Q entityPath,
+            @NotNull JdbcSession jdbcSession,
             Collection<SelectorOptions<GetOperationOptions>> options)
             throws SchemaException {
         S schemaObject = toSchemaObject(tuple.get(entityPath));
@@ -511,45 +519,72 @@ protected Collection<? extends QName> fullObjectItemsToSkip() {
         return Collections.emptyList();
     }
 
-    // TODO: this resolves the names after toSchemaObject(3 params), but...
-    //  called version of toSchemaObject() doesn't have forceFull flag, so sometimes we need to override this version.
-    //  But calling this via super...() means it can't resolve the names in refs from additional parts
-    //  that are not loaded yet (which is done after super call).
-    //  What is the recommended template for override? Wouldn't using this method with additional stuff in the middle be better?
-    //  1. call toSchemaObject(rowTuple, entityPath, options) - just like here, don't call this 5 param method via super!
-    //  2. do additional stuff, utilizing forceFull flag (and even jdbcSession if necessary)
-    //  3. call resolveReferenceNames just like in this method - at the end
-    //  Alternative:
-    //  - make this method final
-    //  - override only toSchemaObject - but add forceFull flag (do we need jdbcSession as well? so far not)
-    public S toSchemaObject(
+    /**
+     * TODO: This should be merged with toSchemaObject and forceFull parameter should be deprecated.
+     *  Proper usage of updateGetOptions() should replace it - see QShadowMapping where toSchemaObject
+     *  is overridden and force reindex works as well.
+     *
+     * In the meantime:
+     *
+     * This is "internal" method in the sense it can be overridden to customize the default transformation behavior.
+     * It is public so one mapper can call it on another mapper, but otherwise should not be called from repo
+     * service or similar places - these should use {@link #toSchemaObjectComplete}.
+     *
+     * *Should I override this or {@link #toSchemaObject} that is called from this method?*
+     * Prefer overriding {@link #toSchemaObject} as we want to get rid of this version and forceFull flag.
+     *
+     * *Do not call this in result list transformers* because the results would not have resolved reference names (if requested).
+     * Notice that the default implementation of {@link #createRowTransformer} in this class calls
+     * {@link #toSchemaObjectCompleteSafe} which is the right thing to call in result list transformers.
+     *
+     * @param forceFull true when reindex is forced on the modified object, otherwise false
+     */
+    @Deprecated
+    public S toSchemaObjectInternal(
             Tuple rowTuple,
             Q entityPath,
             Collection<SelectorOptions<GetOperationOptions>> options,
             @NotNull JdbcSession jdbcSession,
             boolean forceFull) throws SchemaException {
-        return toSchemaObject(rowTuple, entityPath, options);
+        return toSchemaObject(rowTuple, entityPath, jdbcSession, options);
     }
 
-    public S toSchemaObjectWithResolvedNames(
+    /**
+     * Converts tuple to schema object and resolves reference names if necessary.
+     * This is the method called from the "outside" of mappers to obtain complete object.
+     * This method is final to ensure the reference names resolution is the last step performed on the complete object.
+     * Method {@link #toSchemaObjectInternal} prepares the object; can be overridden by object/container mappers as necessary.
+     *
+     * @param forceFull true when reindex is forced on the modified object, otherwise false
+     */
+    public final S toSchemaObjectComplete(
             Tuple rowTuple,
             Q entityPath,
             Collection<SelectorOptions<GetOperationOptions>> options,
             @NotNull JdbcSession jdbcSession,
-            boolean forceFull) throws SchemaException {
-        S schemaObject = toSchemaObject(rowTuple, entityPath, options, jdbcSession, forceFull);
+            @Deprecated boolean forceFull) throws SchemaException {
+        S schemaObject = toSchemaObjectInternal(rowTuple, entityPath, options, jdbcSession, forceFull);
         schemaObject = resolveReferenceNames(schemaObject, jdbcSession, options);
         return schemaObject;
     }
 
-    public S toSchemaObjectSafe(
+    /**
+     * Version of {@link #toSchemaObjectComplete} with custom schema exception treatment.
+     * By default, it is simply wrapped into runtime exception, but is more sophisticated for object mapping.
+     *
+     * This method should be used when each row in return list should have its own exception treatment, which is
+     * the default behavior in midPoint.
+     * Instead of failing the whole search because of single-object schema error, a placeholder object
+     * for the row can be returned, possibly with error indicated.
+     */
+    public S toSchemaObjectCompleteSafe(
             Tuple tuple,
             Q entityPath,
             Collection<SelectorOptions<GetOperationOptions>> options,
             @NotNull JdbcSession jdbcSession,
-            boolean forceFull) {
+            @Deprecated boolean forceFull) {
         try {
-            return toSchemaObjectWithResolvedNames(tuple, entityPath, options, jdbcSession, forceFull);
+            return toSchemaObjectComplete(tuple, entityPath, options, jdbcSession, forceFull);
         } catch (SchemaException e) {
             throw new RepositoryMappingException(e);
         }
@@ -564,6 +599,6 @@ protected <O> O resolveReferenceNames(O object, JdbcSession session, Collection<
     public ResultListRowTransformer<S, Q, R> createRowTransformer(
             SqlQueryContext<S, Q, R> sqlQueryContext, JdbcSession jdbcSession) {
         return (tuple, entityPath, options) ->
-                toSchemaObjectSafe(tuple, entityPath, options, jdbcSession, false);
+                toSchemaObjectCompleteSafe(tuple, entityPath, options, jdbcSession, false);
     }
 }

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/qmodel/accesscert/QAccessCertificationCampaignMapping.java

@@ -142,12 +142,11 @@ public void storeRelatedEntities(
         }
     }
 
-    // TODO rework to toSchemaObject
     @Override
-    public AccessCertificationCampaignType toSchemaObjectWithResolvedNames(Tuple rowTuple, QAccessCertificationCampaign entityPath,
+    public AccessCertificationCampaignType toSchemaObjectInternal(Tuple rowTuple, QAccessCertificationCampaign entityPath,
             Collection<SelectorOptions<GetOperationOptions>> options, @NotNull JdbcSession jdbcSession,
             boolean forceFull) throws SchemaException {
-        AccessCertificationCampaignType base = super.toSchemaObjectWithResolvedNames(rowTuple, entityPath, options, jdbcSession, forceFull);
+        AccessCertificationCampaignType base = super.toSchemaObjectInternal(rowTuple, entityPath, options, jdbcSession, forceFull);
         if (forceFull || shouldLoadCases(options)) {
             loadCases(base, options, jdbcSession, forceFull);
         }
@@ -195,7 +194,7 @@ private void loadCases(AccessCertificationCampaignType base, Collection<Selector
         }
         List<Tuple> rows = query.fetch();
         for (Tuple row : rows) {
-            AccessCertificationCaseType c = casesMapping.toSchemaObjectWithResolvedNames(row, qcase, options, jdbcSession, forceFull);
+            AccessCertificationCaseType c = casesMapping.toSchemaObjectInternal(row, qcase, options, jdbcSession, forceFull);
             cases.add(c.asPrismContainerValue());
         }
     }

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/qmodel/accesscert/QAccessCertificationCaseMapping.java

@@ -132,8 +132,8 @@ private QAccessCertificationCaseMapping(@NotNull SqaleRepoContext repositoryCont
 
     @Override
     public AccessCertificationCaseType toSchemaObject(
-            Tuple row, QAccessCertificationCase entityPath,
-            Collection<SelectorOptions<GetOperationOptions>> options) throws SchemaException {
+            @NotNull Tuple row, @NotNull QAccessCertificationCase entityPath,
+            @NotNull JdbcSession jdbcSession, Collection<SelectorOptions<GetOperationOptions>> options) throws SchemaException {
         return parseSchemaObject(
                 Objects.requireNonNull(row.get(entityPath.fullObject)),
                 Objects.requireNonNull(row.get(entityPath.ownerOid)) + ","
@@ -264,7 +264,7 @@ public ResultListRowTransformer<AccessCertificationCaseType, QAccessCertificatio
                 PrismContainerValue<AccessCertificationCaseType> value = container.findValue(cid);
                 if (value == null) {
                     // value is not present, load it from full object
-                    AccessCertificationCaseType valueObj = toSchemaObject(tuple, entityPath, options);
+                    AccessCertificationCaseType valueObj = toSchemaObjectComplete(tuple, entityPath, options, jdbcSession, false);
                     //noinspection unchecked
                     value = valueObj.asPrismContainerValue();
                     container.add(value);

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/qmodel/accesscert/QAccessCertificationWorkItemMapping.java

@@ -224,7 +224,7 @@ private PrismContainerValue<AccessCertificationCaseType> loadCase(
         }
         try {
             //noinspection unchecked
-            return mapping.toSchemaObject(result, root, Collections.emptyList()).asPrismContainerValue();
+            return mapping.toSchemaObject(result, root, jdbcSession, Collections.emptyList()).asPrismContainerValue();
         } catch (SchemaException e) {
             throw new SystemException(e);
         }

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/qmodel/focus/QFocusMapping.java

@@ -188,9 +188,9 @@ protected Collection<? extends QName> fullObjectItemsToSkip() {
     }
 
     @Override
-    public S toSchemaObject(Tuple row, Q entityPath, Collection<SelectorOptions<GetOperationOptions>> options)
+    public S toSchemaObject(@NotNull Tuple row, @NotNull Q entityPath, @NotNull JdbcSession jdbcSession, Collection<SelectorOptions<GetOperationOptions>> options)
             throws SchemaException {
-        S focus = super.toSchemaObject(row, entityPath, options);
+        S focus = super.toSchemaObject(row, entityPath, jdbcSession, options);
 
         byte[] photo = row.get(entityPath.photo);
         if (photo != null) {

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/qmodel/lookuptable/QLookupTableMapping.java

@@ -98,12 +98,11 @@ protected Collection<? extends QName> fullObjectItemsToSkip() {
         return Collections.singleton(F_ROW);
     }
 
-    // TODO rework to toSchemaObject
     @Override
-    public LookupTableType toSchemaObjectWithResolvedNames(
+    public LookupTableType toSchemaObjectInternal(
             Tuple rowTuple, QLookupTable entityPath, Collection<SelectorOptions<GetOperationOptions>> options,
             @NotNull JdbcSession session, boolean forceFull) throws SchemaException {
-        LookupTableType base = super.toSchemaObjectWithResolvedNames(rowTuple, entityPath, options, session, forceFull);
+        LookupTableType base = super.toSchemaObjectInternal(rowTuple, entityPath, options, session, forceFull);
 
         if (forceFull || SelectorOptions.hasToLoadPath(F_ROW, options)) {
             @Nullable GetOperationOptions rowOptions = findLookupTableGetOption(options);

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/qmodel/object/QObjectMapping.java

@@ -151,7 +151,10 @@ public R newRowObject() {
 
     // region transformation
     @Override
-    public S toSchemaObject(Tuple row, Q entityPath,
+    public S toSchemaObject(
+            @NotNull Tuple row,
+            @NotNull Q entityPath,
+            @NotNull JdbcSession jdbcSession,
             Collection<SelectorOptions<GetOperationOptions>> options)
             throws SchemaException {
         byte[] fullObject = Objects.requireNonNull(row.get(entityPath.fullObject));
@@ -170,14 +173,14 @@ public S toSchemaObject(Tuple row, Q entityPath,
      * should not spoil the whole result list.
      */
     @Override
-    public S toSchemaObjectSafe(
+    public S toSchemaObjectCompleteSafe(
             Tuple tuple,
             Q entityPath,
             Collection<SelectorOptions<GetOperationOptions>> options,
             @NotNull JdbcSession jdbcSession,
             boolean forceFull) {
         try {
-            return toSchemaObjectWithResolvedNames(tuple, entityPath, options, jdbcSession, forceFull);
+            return toSchemaObjectComplete(tuple, entityPath, options, jdbcSession, forceFull);
         } catch (SchemaException e) {
             try {
                 PrismObject<S> errorObject = prismContext().createObject(schemaType());

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/qmodel/shadow/QShadowMapping.java

@@ -135,9 +135,9 @@ public MShadow newRowObject() {
     }
 
     @Override
-    public ShadowType toSchemaObject(Tuple row, QShadow entityPath,
-            Collection<SelectorOptions<GetOperationOptions>> options) throws SchemaException {
-        ShadowType shadowType = super.toSchemaObject(row, entityPath, options);
+    public ShadowType toSchemaObject(@NotNull Tuple row, @NotNull QShadow entityPath,
+            @NotNull JdbcSession jdbcSession, Collection<SelectorOptions<GetOperationOptions>> options) throws SchemaException {
+        ShadowType shadowType = super.toSchemaObject(row, entityPath, jdbcSession, options);
         // FIXME: we store it because provisioning now sends it to repo, but it should be transient
         shadowType.asPrismObject().removeContainer(ShadowType.F_ASSOCIATION);
 

repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/qmodel/task/QTaskMapping.java

@@ -176,10 +176,11 @@ protected Collection<? extends QName> fullObjectItemsToSkip() {
     }
 
     @Override
-    public TaskType toSchemaObject(
-            Tuple row, QTask entityPath, Collection<SelectorOptions<GetOperationOptions>> options)
-            throws SchemaException {
-        TaskType task = super.toSchemaObject(row, entityPath, options);
+    public TaskType toSchemaObject(@NotNull Tuple row,
+            @NotNull QTask entityPath,
+            @NotNull JdbcSession jdbcSession,
+            Collection<SelectorOptions<GetOperationOptions>> options) throws SchemaException {
+        TaskType task = super.toSchemaObject(row, entityPath, jdbcSession, options);
         // We need to check options too for proper setting of incompleteness.
         byte[] fullResult = row.get(entityPath.fullResult);
         if (fullResult != null) {

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

@@ -30,18 +30,15 @@
 
 import com.evolveum.midpoint.prism.*;
 import com.evolveum.midpoint.prism.path.ItemName;
-import com.evolveum.midpoint.prism.path.ItemPath;
 import com.evolveum.midpoint.prism.polystring.PolyString;
 import com.evolveum.midpoint.prism.query.ObjectQuery;
-import com.evolveum.midpoint.prism.query.builder.S_FilterEntryOrEmpty;
 import com.evolveum.midpoint.repo.api.RepositoryService;
 import com.evolveum.midpoint.repo.sqale.SqaleRepoBaseTest;
 import com.evolveum.midpoint.repo.sqale.qmodel.focus.QFocus;
 import com.evolveum.midpoint.repo.sqale.qmodel.object.MObject;
 import com.evolveum.midpoint.repo.sqale.qmodel.object.MObjectType;
 import com.evolveum.midpoint.repo.sqale.qmodel.object.QAssignmentHolder;
 import com.evolveum.midpoint.repo.sqale.qmodel.object.QObject;
-import com.evolveum.midpoint.repo.sqlbase.QueryException;
 import com.evolveum.midpoint.repo.sqlbase.filtering.item.PolyStringItemFilterProcessor;
 import com.evolveum.midpoint.schema.SchemaService;
 import com.evolveum.midpoint.schema.SearchResultList;

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/audit/mapping/AuditTableMapping.java

@@ -14,6 +14,7 @@
 import org.jetbrains.annotations.Nullable;
 
 import com.evolveum.midpoint.repo.sql.data.common.other.RObjectType;
+import com.evolveum.midpoint.repo.sqlbase.JdbcSession;
 import com.evolveum.midpoint.repo.sqlbase.SqlRepoContext;
 import com.evolveum.midpoint.repo.sqlbase.mapping.QueryTableMapping;
 import com.evolveum.midpoint.repo.sqlbase.querydsl.FlexibleRelationalPathBase;
@@ -39,7 +40,7 @@ protected AuditTableMapping(
 
     @Override
     public S toSchemaObject(
-            Tuple tuple, Q entityPath, Collection<SelectorOptions<GetOperationOptions>> options) {
+            @NotNull Tuple tuple, @NotNull Q entityPath, @NotNull JdbcSession jdbcSession, Collection<SelectorOptions<GetOperationOptions>> options) {
         S schemaObject = toSchemaObject(tuple.get(entityPath));
         processExtensionColumns(schemaObject, tuple, entityPath);
         return schemaObject;