From cd3cbcb613ea7b1836b77954d72a40fc4effa5f8 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Wed, 11 Apr 2018 20:01:51 +0200 Subject: [PATCH] Use 'distinct' when displaying objects (MID-3955) The GUI uses 'distinct' search option when displaying objects, unless disabled in the system configuration. Important change in repo: query interpreter now optimizes usage of 'distinct'. If there are no joins, distinct is not applied even if requested. This heuristic would need to be changed if it would be found to be ineffective (see e.g. QueryInterpreter2Test.test0350QuerySubtreeDistinctCount). --- .../gui/api/component/ObjectListPanel.java | 2 + .../gui/api/util/WebComponentUtil.java | 12 +++ .../data/BaseSortableDataProvider.java | 59 ++++++----- .../component/data/ObjectDataProvider.java | 10 +- .../data/RepositoryObjectDataProvider.java | 23 +++-- .../SelectableBeanObjectDataProvider.java | 6 +- .../web/component/search/SearchFactory.java | 6 +- .../admin/server/dto/NodeDtoProvider.java | 4 +- .../admin/server/dto/TaskDtoProvider.java | 7 +- .../midpoint/schema/GetOperationOptions.java | 85 +++++++++++++++- .../midpoint/schema/SelectorOptions.java | 1 - .../schema/util/AdminGuiConfigTypeUtil.java | 8 +- .../repo/sql/QueryInterpreter2Test.java | 98 ++++++++++++++----- .../repo/sql/helpers/ObjectRetriever.java | 4 +- .../repo/sql/query2/QueryInterpreter2.java | 6 +- .../repo/sql/query2/hqm/HibernateQuery.java | 1 + .../sql/query2/hqm/RootHibernateQuery.java | 10 +- 17 files changed, 252 insertions(+), 90 deletions(-) diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/component/ObjectListPanel.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/component/ObjectListPanel.java index 65c8ba96c19..17b7e231f9a 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/component/ObjectListPanel.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/component/ObjectListPanel.java @@ -57,6 +57,7 @@ import com.evolveum.midpoint.web.component.util.SelectableBean; import com.evolveum.midpoint.web.session.PageStorage; import com.evolveum.midpoint.web.session.UserProfileStorage.TableId; +import org.jetbrains.annotations.NotNull; /** * @author katkav @@ -361,6 +362,7 @@ public SelectableBean createDataObjectWrapper(O obj) { return bean; } + @NotNull @Override protected List createObjectOrderings(SortParam sortParam) { List customOrdering = createCustomOrdering(sortParam); diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/util/WebComponentUtil.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/util/WebComponentUtil.java index d24a437dbfc..2387cd0e8b7 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/util/WebComponentUtil.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/api/util/WebComponentUtil.java @@ -416,6 +416,18 @@ public static void safeResultCleanup(OperationResult result, Trace logger) { } } + public static GuiObjectListType getDefaultGuiObjectListType(PageBase pageBase) { + AdminGuiConfigurationType config = pageBase.getPrincipal().getAdminGuiConfiguration(); + if (config == null) { + return null; + } + GuiObjectListsType lists = config.getObjectLists(); + if (lists == null) { + return null; + } + return lists.getDefault(); + } + public enum Channel { // TODO: move this to schema component LIVE_SYNC(SchemaConstants.CHANGE_CHANNEL_LIVE_SYNC_URI), diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/BaseSortableDataProvider.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/BaseSortableDataProvider.java index d9456524019..c9c02d2640e 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/BaseSortableDataProvider.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/BaseSortableDataProvider.java @@ -17,6 +17,7 @@ package com.evolveum.midpoint.web.component.data; import com.evolveum.midpoint.gui.api.page.PageBase; +import com.evolveum.midpoint.gui.api.util.WebComponentUtil; import com.evolveum.midpoint.model.api.*; import com.evolveum.midpoint.prism.PrismContext; import com.evolveum.midpoint.prism.path.ItemPath; @@ -25,7 +26,9 @@ import com.evolveum.midpoint.prism.query.ObjectQuery; import com.evolveum.midpoint.prism.query.OrderDirection; import com.evolveum.midpoint.repo.api.RepositoryService; +import com.evolveum.midpoint.schema.GetOperationOptions; import com.evolveum.midpoint.schema.SchemaConstantsGenerated; +import com.evolveum.midpoint.schema.SelectorOptions; import com.evolveum.midpoint.schema.result.OperationResult; import com.evolveum.midpoint.task.api.TaskManager; import com.evolveum.midpoint.util.logging.Trace; @@ -34,8 +37,8 @@ import com.evolveum.midpoint.web.security.MidPointApplication; import com.evolveum.midpoint.wf.api.WorkflowManager; import com.evolveum.midpoint.xml.ns._public.common.common_3.AdminGuiConfigurationType; +import com.evolveum.midpoint.xml.ns._public.common.common_3.DistinctSearchOptionType; import com.evolveum.midpoint.xml.ns._public.common.common_3.GuiObjectListType; -import com.evolveum.midpoint.xml.ns._public.common.common_3.GuiObjectListsType; import org.apache.commons.lang.Validate; import org.apache.wicket.Component; import org.apache.wicket.extensions.markup.html.repeater.data.sort.SortOrder; @@ -45,6 +48,7 @@ import org.apache.wicket.model.IModel; import org.apache.wicket.model.Model; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import javax.xml.namespace.QName; import java.io.Serializable; @@ -93,47 +97,47 @@ public BaseSortableDataProvider(Component component, boolean useCache, boolean u } protected ModelService getModel() { - MidPointApplication application = (MidPointApplication) MidPointApplication.get(); + MidPointApplication application = MidPointApplication.get(); return application.getModel(); } protected RepositoryService getRepositoryService() { - MidPointApplication application = (MidPointApplication) MidPointApplication.get(); + MidPointApplication application = MidPointApplication.get(); return application.getRepositoryService(); } protected TaskManager getTaskManager() { - MidPointApplication application = (MidPointApplication) MidPointApplication.get(); + MidPointApplication application = MidPointApplication.get(); return application.getTaskManager(); } protected PrismContext getPrismContext() { - MidPointApplication application = (MidPointApplication) MidPointApplication.get(); + MidPointApplication application = MidPointApplication.get(); return application.getPrismContext(); } protected TaskService getTaskService() { - MidPointApplication application = (MidPointApplication) MidPointApplication.get(); + MidPointApplication application = MidPointApplication.get(); return application.getTaskService(); } protected ModelInteractionService getModelInteractionService() { - MidPointApplication application = (MidPointApplication) MidPointApplication.get(); + MidPointApplication application = MidPointApplication.get(); return application.getModelInteractionService(); } protected WorkflowService getWorkflowService() { - MidPointApplication application = (MidPointApplication) MidPointApplication.get(); + MidPointApplication application = MidPointApplication.get(); return application.getWorkflowService(); } protected ModelAuditService getAuditService() { - MidPointApplication application = (MidPointApplication) MidPointApplication.get(); + MidPointApplication application = MidPointApplication.get(); return application.getAuditService(); } protected WorkflowManager getWorkflowManager() { - MidPointApplication application = (MidPointApplication) MidPointApplication.get(); + MidPointApplication application = MidPointApplication.get(); return application.getWorkflowManager(); } @@ -193,27 +197,30 @@ protected boolean checkOrderingSettings() { return false; } - public boolean isOrderingDisabled() { - if (!checkOrderingSettings()) { - return false; - } + public boolean isDistinct() { + GuiObjectListType def = WebComponentUtil.getDefaultGuiObjectListType((PageBase) component.getPage()); + return def == null || def.getDistinct() != DistinctSearchOptionType.NEVER; // change after other options are added + } - PageBase page = (PageBase) component.getPage(); - AdminGuiConfigurationType config = page.getPrincipal().getAdminGuiConfiguration(); - if (config == null) { - return false; - } - GuiObjectListsType lists = config.getObjectLists(); - if (lists == null) { - return false; + protected Collection> createDefaultOptions() { + return getDistinctRelatedOptions(); // probably others in the future + } + + @Nullable + protected Collection> getDistinctRelatedOptions() { + if (isDistinct()) { + return SelectorOptions.createCollection(GetOperationOptions.createDistinct()); + } else { + return null; } + } - GuiObjectListType def = lists.getDefault(); - if (def == null) { + public boolean isOrderingDisabled() { + if (!checkOrderingSettings()) { return false; } - - return def.isDisableSorting(); + GuiObjectListType def = WebComponentUtil.getDefaultGuiObjectListType((PageBase) component.getPage()); + return def != null && def.isDisableSorting(); } protected ObjectPaging createPaging(long offset, long pageSize) { diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/ObjectDataProvider.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/ObjectDataProvider.java index efb5f0000e5..a54f51eaf62 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/ObjectDataProvider.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/ObjectDataProvider.java @@ -78,6 +78,12 @@ public List getSelectedData() { return allSelected; } + // Here we apply the distinct option. It is easier and more reliable to apply it here than to do at all the places + // where options for this provider are defined. + private Collection> getOptionsToUse() { + return GetOperationOptions.merge(options, getDistinctRelatedOptions()); + } + @Override public Iterator internalIterator(long first, long count) { @@ -120,7 +126,7 @@ public Iterator internalIterator(long first, long count) { LOGGER.trace("Query {} with {}", type.getSimpleName(), query.debugDump()); } - List> list = getModel().searchObjects(type, query, options, task, result); + List> list = getModel().searchObjects(type, query, getOptionsToUse(), task, result); if (LOGGER.isTraceEnabled()) { LOGGER.trace("Query {} resulted in {} objects", type.getSimpleName(), list.size()); @@ -169,7 +175,7 @@ protected int internalSize() { OperationResult result = new OperationResult(OPERATION_COUNT_OBJECTS); try { Task task = getPage().createSimpleTask(OPERATION_COUNT_OBJECTS); - count = getModel().countObjects(type, getQuery(), options, task, result); + count = getModel().countObjects(type, getQuery(), getOptionsToUse(), task, result); } catch (Exception ex) { result.recordFatalError("Couldn't count objects.", ex); LoggingUtils.logUnexpectedException(LOGGER, "Couldn't count objects", ex); diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/RepositoryObjectDataProvider.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/RepositoryObjectDataProvider.java index 42ea9665416..3411a4fb5b7 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/RepositoryObjectDataProvider.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/RepositoryObjectDataProvider.java @@ -38,6 +38,7 @@ import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowType; import org.apache.commons.lang.Validate; import org.apache.wicket.Component; +import org.jetbrains.annotations.NotNull; import java.io.Serializable; import java.util.*; @@ -78,12 +79,7 @@ public Iterator internalIterator(long first, long count) { } query.setPaging(paging); - //RAW and DEFAULT retrieve option selected - Collection> options = new ArrayList<>(); - GetOperationOptions opt = GetOperationOptions.createRaw(); - opt.setRetrieve(RetrieveOption.DEFAULT); - options.add(SelectorOptions.create(ItemPath.EMPTY_PATH, opt)); - + Collection> options = getOptions(); List> list = getModel().searchObjects((Class) type, query, options, getPage().createSimpleTask(OPERATION_SEARCH_OBJECTS), result); for (PrismObject object : list) { @@ -101,6 +97,15 @@ public Iterator internalIterator(long first, long count) { return getAvailableData().iterator(); } + @NotNull + private Collection> getOptions() { + Collection> options = new ArrayList<>(); + GetOperationOptions opt = GetOperationOptions.createRaw(); + opt.setRetrieve(RetrieveOption.DEFAULT); + options.add(SelectorOptions.create(ItemPath.EMPTY_PATH, opt)); + return GetOperationOptions.merge(createDefaultOptions(), options); + } + @Override protected boolean checkOrderingSettings() { return true; @@ -171,16 +176,14 @@ protected int internalSize() { int count = 0; OperationResult result = new OperationResult(OPERATION_COUNT_OBJECTS); try { - count = getModel().countObjects(type, getQuery(), - SelectorOptions.createCollection(ItemPath.EMPTY_PATH, GetOperationOptions.createRaw()), + count = getModel().countObjects(type, getQuery(), getOptions(), getPage().createSimpleTask(OPERATION_COUNT_OBJECTS), result); } catch (Exception ex) { result.recordFatalError("Couldn't count objects.", ex); } finally { result.computeStatusIfUnknown(); } - - getPage().showResult(result, false); + getPage().showResult(result, false); LOGGER.trace("end::internalSize()"); return count; } diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/SelectableBeanObjectDataProvider.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/SelectableBeanObjectDataProvider.java index 797ea099e7c..9bd2d0918a7 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/SelectableBeanObjectDataProvider.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/data/SelectableBeanObjectDataProvider.java @@ -161,7 +161,7 @@ public Iterator> internalIterator(long offset, long pageSize) LOGGER.trace("Query {} with {}", type.getSimpleName(), query.debugDump()); } - if (ResourceType.class.equals(type) && (options == null || options.isEmpty())){ + if (ResourceType.class.equals(type) && (options == null || options.isEmpty())) { options = SelectorOptions.createCollection(GetOperationOptions.createNoFetch()); } Collection> currentOptions = options; @@ -178,6 +178,7 @@ public Iterator> internalIterator(long offset, long pageSize) (o) -> o.setDefinitionProcessing(FULL)); } } + currentOptions = GetOperationOptions.merge(currentOptions, getDistinctRelatedOptions()); List> list = (List)getModel().searchObjects(type, query, currentOptions, task, result); if (LOGGER.isTraceEnabled()) { @@ -240,7 +241,8 @@ protected int internalSize() { Task task = getPage().createSimpleTask(OPERATION_COUNT_OBJECTS); OperationResult result = task.getResult(); try { - Integer counted = getModel().countObjects(type, getQuery(), options, task, result); + Collection> currentOptions = GetOperationOptions.merge(options, getDistinctRelatedOptions()); + Integer counted = getModel().countObjects(type, getQuery(), currentOptions, task, result); count = defaultIfNull(counted, 0); } catch (Exception ex) { result.recordFatalError("Couldn't count objects.", ex); diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/search/SearchFactory.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/search/SearchFactory.java index 97cf4359404..2503ff10179 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/search/SearchFactory.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/component/search/SearchFactory.java @@ -217,11 +217,11 @@ private static boolean isFullTextSearchEnabled(ModelServi return FullTextSearchConfigurationUtil.isEnabledFor(modelServiceLocator.getModelInteractionService().getSystemConfiguration(result) .getFullTextSearch(), type); } catch (SchemaException | ObjectNotFoundException ex) { - throw new SystemException(ex); + throw new SystemException(ex); } } - private static SearchBoxModeType getDefaultSearchType (ModelServiceLocator modelServiceLocator, Class type) { + private static SearchBoxModeType getDefaultSearchType(ModelServiceLocator modelServiceLocator, Class type) { OperationResult result = new OperationResult(LOAD_ADMIN_GUI_CONFIGURATION); try { AdminGuiConfigurationType guiConfig = modelServiceLocator.getModelInteractionService().getAdminGuiConfiguration(null, result); @@ -238,7 +238,7 @@ private static SearchBoxModeType getDefaultSearchType (Mo } return null; } catch (SchemaException | ObjectNotFoundException ex) { - throw new SystemException(ex); + throw new SystemException(ex); } } diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/NodeDtoProvider.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/NodeDtoProvider.java index d85d198b688..9a19b1af0fc 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/NodeDtoProvider.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/NodeDtoProvider.java @@ -60,7 +60,7 @@ public Iterator internalIterator(long first, long count) { } query.setPaging(paging); - List> nodes = getModel().searchObjects(NodeType.class, query, null, task, result); + List> nodes = getModel().searchObjects(NodeType.class, query, createDefaultOptions(), task, result); for (PrismObject node : nodes) { getAvailableData().add(createNodeDto(node)); @@ -104,7 +104,7 @@ protected int internalSize() { OperationResult result = new OperationResult(OPERATION_COUNT_NODES); Task task = getTaskManager().createTaskInstance(OPERATION_COUNT_NODES); try { - count = getModel().countObjects(NodeType.class, getQuery(), null, task, result); + count = getModel().countObjects(NodeType.class, getQuery(), createDefaultOptions(), task, result); result.recomputeStatus(); } catch (Exception ex) { LoggingUtils.logUnexpectedException(LOGGER, "Unhandled exception when counting nodes", ex); diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/TaskDtoProvider.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/TaskDtoProvider.java index b0101296cfd..b0b87313ff6 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/TaskDtoProvider.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/web/page/admin/server/dto/TaskDtoProvider.java @@ -84,8 +84,9 @@ public Iterator internalIterator(long first, long count) { propertiesToGet.add(TaskType.F_NEXT_RUN_START_TIMESTAMP); propertiesToGet.add(TaskType.F_NEXT_RETRY_TIMESTAMP); } - Collection> searchOptions = - GetOperationOptions.createRetrieveAttributesOptions(propertiesToGet.toArray(new QName[0])); + Collection> searchOptions = GetOperationOptions.merge( + createDefaultOptions(), + GetOperationOptions.createRetrieveAttributesOptions(propertiesToGet.toArray(new QName[0]))); List> tasks = getModel().searchObjects(TaskType.class, query, searchOptions, operationTask, result); for (PrismObject task : tasks) { try { @@ -140,7 +141,7 @@ protected int internalSize() { OperationResult result = new OperationResult(OPERATION_COUNT_TASKS); Task task = getTaskManager().createTaskInstance(OPERATION_COUNT_TASKS); try { - count = getModel().countObjects(TaskType.class, getQuery(), null, task, result); + count = getModel().countObjects(TaskType.class, getQuery(), createDefaultOptions(), task, result); result.recomputeStatus(); } catch (Exception ex) { LoggingUtils.logUnexpectedException(LOGGER, "Unhandled exception when counting tasks", ex); diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptions.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptions.java index 30ab61bb610..4c7be7228d4 100644 --- a/infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptions.java +++ b/infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptions.java @@ -20,7 +20,8 @@ import com.evolveum.midpoint.xml.ns._public.common.common_3.GetOperationOptionsType; import com.evolveum.midpoint.xml.ns._public.common.common_3.IterationMethodType; import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType; -import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections4.CollectionUtils; +import org.jetbrains.annotations.NotNull; import javax.xml.namespace.QName; @@ -175,6 +176,10 @@ public class GetOperationOptions extends AbstractOptions implements Serializable */ private Boolean executionPhase; + /* + * !!! After adding option here don't forget to update equals, clone, merge, etc. !!! + */ + public RetrieveOption getRetrieve() { return retrieve; } @@ -916,4 +921,82 @@ public static GetOperationOptions fromRestOptions(List options, Definiti } + @NotNull + @SafeVarargs + public static Collection> merge(Collection>... parts) { + Collection> merged = new ArrayList<>(); + for (Collection> part : parts) { + for (SelectorOptions increment : CollectionUtils.emptyIfNull(part)) { + if (increment != null) { // should always be so + Collection existing = SelectorOptions.findOptionsForPath(merged, increment.getItemPath()); + if (existing.isEmpty()) { + merged.add(increment); + } else if (existing.size() == 1) { + existing.iterator().next().merge(increment.getOptions()); + } else { + throw new AssertionError("More than one options for path: " + increment.getItemPath()); + } + } + } + } + return merged; + } + + @SuppressWarnings("ConstantConditions") + private void merge(GetOperationOptions increment) { + if (increment == null) { + return; + } + if (increment.retrieve != null) { + this.retrieve = increment.retrieve; + } + if (increment.resolve != null) { + this.resolve = increment.resolve; + } + if (increment.resolveNames != null) { + this.resolveNames = increment.resolveNames; + } + if (increment.noFetch != null) { + this.noFetch = increment.noFetch; + } + if (increment.raw != null) { + this.raw = increment.raw; + } + if (increment.tolerateRawData != null) { + this.tolerateRawData = increment.tolerateRawData; + } + if (increment.doNotDiscovery != null) { + this.doNotDiscovery = increment.doNotDiscovery; + } + if (increment.relationalValueSearchQuery != null) { + this.relationalValueSearchQuery = increment.relationalValueSearchQuery; + } + if (increment.allowNotFound != null) { + this.allowNotFound = increment.allowNotFound; + } + if (increment.readOnly != null) { + this.readOnly = increment.readOnly; + } + if (increment.pointInTimeType != null) { + this.pointInTimeType = increment.pointInTimeType; + } + if (increment.staleness != null) { + this.staleness = increment.staleness; + } + if (increment.distinct != null) { + this.distinct = increment.distinct; + } + if (increment.attachDiagData != null) { + this.attachDiagData = increment.attachDiagData; + } + if (increment.definitionProcessing != null) { + this.definitionProcessing = increment.definitionProcessing; + } + if (increment.iterationMethod != null) { + this.iterationMethod = increment.iterationMethod; + } + if (increment.executionPhase != null) { + this.executionPhase = increment.executionPhase; + } + } } diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/SelectorOptions.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/SelectorOptions.java index 41eb0b67adc..568aa5ac2b8 100644 --- a/infra/schema/src/main/java/com/evolveum/midpoint/schema/SelectorOptions.java +++ b/infra/schema/src/main/java/com/evolveum/midpoint/schema/SelectorOptions.java @@ -403,6 +403,5 @@ public void shortDump(StringBuilder sb) { sb.append(options); } } - //endregion } diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/AdminGuiConfigTypeUtil.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/AdminGuiConfigTypeUtil.java index 8ba00e10b8b..fc3571c0142 100644 --- a/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/AdminGuiConfigTypeUtil.java +++ b/infra/schema/src/main/java/com/evolveum/midpoint/schema/util/AdminGuiConfigTypeUtil.java @@ -188,13 +188,7 @@ private static boolean isTheSameObjectForm(ObjectFormType oldForm, ObjectFormTyp private static void mergeList(GuiObjectListsType objectLists, GuiObjectListType newList) { // We support only the default object lists now, so simply replace the existing definition with the // latest definition. We will need a more sophisticated merging later. - Iterator iterator = objectLists.getObjectList().iterator(); - while (iterator.hasNext()) { - GuiObjectListType currentList = iterator.next(); - if (currentList.getType().equals(newList.getType())) { - iterator.remove(); - } - } + objectLists.getObjectList().removeIf(currentList -> currentList.getType().equals(newList.getType())); objectLists.getObjectList().add(newList.clone()); } diff --git a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/QueryInterpreter2Test.java b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/QueryInterpreter2Test.java index 5b7106ad352..bb59824477c 100644 --- a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/QueryInterpreter2Test.java +++ b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/QueryInterpreter2Test.java @@ -2064,8 +2064,9 @@ public void test0350QuerySubtreeDistinctCount() throws Exception { .build(); String real = getInterpretedQuery2(session, OrgType.class, query, true, distinct()); + // we probably do not need 'distinct' here String expected = "select\n" - + " count(distinct o.oid)\n" + + " count(o.oid)\n" + "from\n" + " ROrg o\n" + "where\n" @@ -4405,7 +4406,7 @@ public void test1100ProcessStartTimestamp() throws Exception { } @Test - public void test1101AvailabilityStatus() throws Exception { + public void test1110AvailabilityStatus() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(ResourceType.class, prismContext) @@ -4432,7 +4433,7 @@ public void test1101AvailabilityStatus() throws Exception { } @Test - public void test1102NullRefSingle() throws Exception { + public void test1120NullRefSingle() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(ResourceType.class, prismContext) @@ -4458,7 +4459,7 @@ public void test1102NullRefSingle() throws Exception { } @Test // the same as test0142QueryUserAccountRefNull, but keeping because of test structure - public void test1103NullRefMulti() throws Exception { + public void test1130NullRefMulti() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(UserType.class, prismContext) @@ -4486,7 +4487,7 @@ public void test1103NullRefMulti() throws Exception { } @Test - public void test1104NullEqSingle() throws Exception { + public void test1140NullEqSingle() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(UserType.class, prismContext) @@ -4512,7 +4513,7 @@ public void test1104NullEqSingle() throws Exception { } @Test - public void test1105NullEqMulti() throws Exception { + public void test1150NullEqMulti() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(UserType.class, prismContext) @@ -4540,7 +4541,7 @@ public void test1105NullEqMulti() throws Exception { } @Test - public void test1106AbstractRoleParameters() throws Exception { + public void test1160AbstractRoleParameters() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(RoleType.class, prismContext) @@ -4572,7 +4573,7 @@ public void test1106AbstractRoleParameters() throws Exception { } @Test - public void test1107ExistsShadowPendingOperation() throws Exception { + public void test1170ExistsShadowPendingOperation() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(ShadowType.class, prismContext) @@ -4599,7 +4600,7 @@ public void test1107ExistsShadowPendingOperation() throws Exception { } @Test - public void test1108OperationFatalError() throws Exception { + public void test1180OperationFatalError() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(ObjectType.class, prismContext) @@ -4628,7 +4629,7 @@ public void test1108OperationFatalError() throws Exception { } @Test - public void test1109OperationSuccessForGivenTask() throws Exception { + public void test1190OperationSuccessForGivenTask() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(ObjectType.class, prismContext) @@ -4666,7 +4667,7 @@ public void test1109OperationSuccessForGivenTask() throws Exception { } @Test - public void test1110OperationLastFailures() throws Exception { + public void test1200OperationLastFailures() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(ObjectType.class, prismContext) @@ -4701,7 +4702,7 @@ public void test1110OperationLastFailures() throws Exception { } @Test - public void test1111PersonaRef() throws Exception { + public void test1210PersonaRef() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(FocusType.class, prismContext) @@ -4732,7 +4733,7 @@ public void test1111PersonaRef() throws Exception { } @Test - public void test1112DistinctAndOrderBy() throws Exception { + public void test1220IgnorableDistinctAndOrderBy() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(UserType.class, prismContext) @@ -4740,6 +4741,33 @@ public void test1112DistinctAndOrderBy() throws Exception { .build(); String real = getInterpretedQuery2(session, UserType.class, query, false, distinct()); String expected; + expected = "select u.oid,\n" + + " u.fullObject,\n" + + " u.stringsCount,\n" + + " u.longsCount,\n" + + " u.datesCount,\n" + + " u.referencesCount,\n" + + " u.polysCount,\n" + + " u.booleansCount\n" + + "from\n" + + " RUser u\n" + + "order by u.nameCopy.orig asc\n"; + assertEqualsIgnoreWhitespace(expected, real); + } finally { + close(session); + } + } + + @Test + public void test1230ApplicableDistinctAndOrderBy() throws Exception { + Session session = open(); + try { + ObjectQuery query = QueryBuilder.queryFor(UserType.class, prismContext) + .item(UserType.F_EMPLOYEE_TYPE).startsWith("e") + .asc(UserType.F_NAME) + .build(); + String real = getInterpretedQuery2(session, UserType.class, query, false, distinct()); + String expected; SqlRepositoryConfiguration config = getConfiguration(); if (config.isUsingOracle() || config.isUsingSQLServer()) { expected = "select\n" @@ -4750,7 +4778,8 @@ public void test1112DistinctAndOrderBy() throws Exception { + " u.datesCount,\n" + " u.referencesCount,\n" + " u.polysCount,\n" - + " u.booleansCount\n" + + " u.booleansCount,\n" + + " u.nameCopy.orig\n" + "from\n" + " RUser u\n" + "where\n" @@ -4758,8 +4787,8 @@ public void test1112DistinctAndOrderBy() throws Exception { + " select distinct\n" + " u.oid\n" + " from\n" - + " RUser u)\n" - + "order by u.name.orig asc"; + + " RUser u left join u.employeeType e where e like :e )\n" + + "order by u.nameCopy.orig asc"; } else { expected = "select distinct\n" + " u.oid,\n" @@ -4772,7 +4801,7 @@ public void test1112DistinctAndOrderBy() throws Exception { + " u.booleansCount,\n" + " u.nameCopy.orig\n" + "from\n" - + " RUser u\n" + + " RUser u left join u.employeeType e where e like :e\n" + "order by u.nameCopy.orig asc\n"; } assertEqualsIgnoreWhitespace(expected, real); @@ -4782,7 +4811,7 @@ public void test1112DistinctAndOrderBy() throws Exception { } @Test - public void test1113DistinctUserWithAssignment() throws Exception { + public void test1240DistinctUserWithAssignment() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(UserType.class, prismContext) @@ -4845,7 +4874,7 @@ public void test1113DistinctUserWithAssignment() throws Exception { } @Test - public void test1114CampaignEndTimestamp() throws Exception { + public void test1250CampaignEndTimestamp() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(AccessCertificationCampaignType.class, prismContext) @@ -4879,7 +4908,7 @@ public void test1114CampaignEndTimestamp() throws Exception { } @Test - public void test1115CampaignEndTimestamp2() throws Exception { + public void test1260CampaignEndTimestamp2() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(AccessCertificationCampaignType.class, prismContext) @@ -4914,13 +4943,28 @@ public void test1115CampaignEndTimestamp2() throws Exception { } @Test - public void test1116DistinctWithCount() throws Exception { + public void test1270IgnorableDistinctWithCount() throws Exception { + Session session = open(); + try { + ObjectQuery query = QueryBuilder.queryFor(UserType.class, prismContext) + .build(); + String real = getInterpretedQuery2(session, UserType.class, query, true, distinct()); + String expected = "select count(u.oid) from RUser u"; + assertEqualsIgnoreWhitespace(expected, real); + } finally { + close(session); + } + } + + @Test + public void test1280ApplicableDistinctWithCount() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(UserType.class, prismContext) + .item(UserType.F_EMPLOYEE_TYPE).startsWith("a") .build(); String real = getInterpretedQuery2(session, UserType.class, query, true, distinct()); - String expected = "select count(distinct u.oid) from RUser u"; + String expected = "select count(distinct u.oid) from RUser u left join u.employeeType e where e like :e"; assertEqualsIgnoreWhitespace(expected, real); } finally { close(session); @@ -4928,7 +4972,7 @@ public void test1116DistinctWithCount() throws Exception { } @Test - public void test1200OidEqTest() throws Exception { + public void test1300OidEqTest() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(ObjectType.class, prismContext) @@ -4955,7 +4999,7 @@ public void test1200OidEqTest() throws Exception { } @Test - public void test1210OwnerOidEqTest() throws Exception { + public void test1310OwnerOidEqTest() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(AccessCertificationCaseType.class, prismContext) @@ -4976,7 +5020,7 @@ public void test1210OwnerOidEqTest() throws Exception { } @Test - public void test1220OidGeLtTest() throws Exception { + public void test1320OidGeLtTest() throws Exception { Session session = open(); try { ObjectQuery query = QueryBuilder.queryFor(ObjectType.class, prismContext) @@ -5114,7 +5158,7 @@ private void assertEqualsIgnoreWhitespace(String expected, String real) { } @Test - public void test1220QueryOrderByNameOrigLimit20() throws Exception { + public void test1400QueryOrderByNameOrigLimit20() throws Exception { Session session = open(); try { @@ -5135,7 +5179,7 @@ public void test1220QueryOrderByNameOrigLimit20() throws Exception { } @Test - public void test1230QueryOrderByNameOrigLimit20() throws Exception { + public void test1410QueryOrderByNameOrigLimit20() throws Exception { Session session = open(); try { 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 index 585ba6b6845..f0c65fcdce2 100644 --- 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 @@ -309,11 +309,9 @@ public int countObjectsAttempt(Class type, ObjectQuery session = baseHelper.beginReadOnlyTransaction(); Number longCount; if (query == null || query.getFilter() == null) { - if (GetOperationOptions.isDistinct(SelectorOptions.findRootOptions(options))) { - throw new UnsupportedOperationException("Distinct option is not supported here"); // TODO - } // this is 5x faster than count with 3 inner joins, it can probably improved also for queries which // filters uses only properties from concrete entities like RUser, RRole by improving interpreter [lazyman] + // note: distinct can be ignored here, as there is no filter, so no joins NativeQuery sqlQuery = session.createNativeQuery("SELECT COUNT(*) FROM " + RUtil.getTableName(hqlType, session)); longCount = (Number) sqlQuery.uniqueResult(); } else { diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/QueryInterpreter2.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/QueryInterpreter2.java index 40d45aff942..c727bbca644 100644 --- a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/QueryInterpreter2.java +++ b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/query2/QueryInterpreter2.java @@ -109,8 +109,8 @@ public SqlRepositoryConfiguration getRepoConfiguration() { public RootHibernateQuery interpret(ObjectQuery query, @NotNull Class type, Collection> options, @NotNull PrismContext prismContext, boolean countingObjects, @NotNull Session session) throws QueryException { - boolean distinct = GetOperationOptions.isDistinct(SelectorOptions.findRootOptions(options)); - LOGGER.trace("Interpreting query for type '{}' (counting={}, distinct={}), query:\n{}", type, countingObjects, distinct, query); + boolean distinctRequested = GetOperationOptions.isDistinct(SelectorOptions.findRootOptions(options)); + LOGGER.trace("Interpreting query for type '{}' (counting={}, distinctRequested={}), query:\n{}", type, countingObjects, distinctRequested, query); InterpretationContext context = new InterpretationContext(this, type, prismContext, session); interpretQueryFilter(context, query); @@ -120,6 +120,7 @@ public RootHibernateQuery interpret(ObjectQuery query, @NotNull Class