You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@causeway.apache.org by ah...@apache.org on 2024/06/11 10:26:03 UTC

(causeway) branch 3772-sorting.hollow.bug created (now aab9ce2bf6)

This is an automated email from the ASF dual-hosted git repository.

ahuber pushed a change to branch 3772-sorting.hollow.bug
in repository https://gitbox.apache.org/repos/asf/causeway.git


      at aab9ce2bf6 CAUSEWAY-3772: moves sorting responsibility from wicket to DataTableInteractive

This branch includes the following new commits:

     new aab9ce2bf6 CAUSEWAY-3772: moves sorting responsibility from wicket to DataTableInteractive

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(causeway) 01/01: CAUSEWAY-3772: moves sorting responsibility from wicket to DataTableInteractive

Posted by ah...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ahuber pushed a commit to branch 3772-sorting.hollow.bug
in repository https://gitbox.apache.org/repos/asf/causeway.git

commit aab9ce2bf6c3e36cd41d22c85f9032f6661782c5
Author: Andi Huber <ah...@apache.org>
AuthorDate: Tue Jun 11 12:25:58 2024 +0200

    CAUSEWAY-3772: moves sorting responsibility from wicket to
    DataTableInteractive
---
 .../core/metamodel/object/ManagedObjects.java      | 52 +-------------
 .../core/metamodel/object/MmSortUtils.java         | 84 ++++++++++++++++++++++
 .../tabular/interactive/DataTableInteractive.java  | 34 ++++++++-
 .../core/metamodel/object/ManagedObjectTest.java   |  2 +-
 .../CollectionContentsSortableDataProvider.java    | 47 +++++++-----
 5 files changed, 151 insertions(+), 68 deletions(-)

diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/object/ManagedObjects.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/object/ManagedObjects.java
index 56f35394fe..cc787edfc2 100644
--- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/object/ManagedObjects.java
+++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/object/ManagedObjects.java
@@ -18,7 +18,6 @@
  */
 package org.apache.causeway.core.metamodel.object;
 
-import java.util.Comparator;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Supplier;
@@ -34,13 +33,11 @@ import org.apache.causeway.commons.collections.Can;
 import org.apache.causeway.commons.functional.Try;
 import org.apache.causeway.commons.internal.base._Casts;
 import org.apache.causeway.commons.internal.base._NullSafe;
-import org.apache.causeway.commons.internal.base._Objects;
 import org.apache.causeway.commons.internal.exceptions._Exceptions;
 import org.apache.causeway.commons.internal.reflection._GenericResolver.ResolvedMethod;
 import org.apache.causeway.core.metamodel.commons.ClassExtensions;
 import org.apache.causeway.core.metamodel.consent.InteractionInitiatedBy;
 import org.apache.causeway.core.metamodel.spec.ObjectSpecification;
-import org.apache.causeway.core.metamodel.spec.feature.ObjectAssociation;
 
 import lombok.NonNull;
 import lombok.val;
@@ -297,59 +294,14 @@ public final class ManagedObjects {
                     : obj.getPojo();
     }
 
-    // -- COMPARE UTILITIES
+    // -- EQUALITY
 
     public boolean pojoEquals(final @Nullable ManagedObject a, final @Nullable ManagedObject b) {
         val aPojo = MmUnwrapUtils.single(a);
         val bPojo = MmUnwrapUtils.single(b);
         return Objects.equals(aPojo, bPojo);
     }
-
-    public int compare(final @Nullable ManagedObject p, final @Nullable ManagedObject q) {
-        return NATURAL_NULL_FIRST.compare(p, q);
-    }
-
-    public Comparator<ManagedObject> orderingBy(final ObjectAssociation sortProperty, final boolean ascending) {
-
-        final Comparator<ManagedObject> comparator = ascending
-                ? NATURAL_NULL_FIRST
-                : NATURAL_NULL_FIRST.reversed();
-
-        return (p, q) -> {
-            val pSort = sortProperty.get(p, InteractionInitiatedBy.FRAMEWORK);
-            val qSort = sortProperty.get(q, InteractionInitiatedBy.FRAMEWORK);
-            return comparator.compare(pSort, qSort);
-        };
-
-    }
-
-    // -- PREDEFINED COMPARATOR
-
-    static final Comparator<ManagedObject> NATURAL_NULL_FIRST = new Comparator<ManagedObject>(){
-        @SuppressWarnings({"rawtypes" })
-        @Override
-        public int compare(final @Nullable ManagedObject a, final @Nullable ManagedObject b) {
-            val aPojo = MmUnwrapUtils.single(a);
-            val bPojo = MmUnwrapUtils.single(b);
-            if(Objects.equals(aPojo, bPojo)) {
-                return 0;
-            }
-            if((aPojo==null
-                    || aPojo instanceof Comparable)
-                && (bPojo==null
-                        || bPojo instanceof Comparable)) {
-                return _Objects.compareNullsFirst((Comparable)aPojo, (Comparable)bPojo);
-            }
-            final int hashCompare = Integer.compare(Objects.hashCode(aPojo), Objects.hashCode(bPojo));
-            if(hashCompare!=0) {
-                return hashCompare;
-            }
-            //XXX on hash-collision we return an arbitrary non-equal relation (unspecified behavior)
-            return -1;
-        }
-
-    };
-
+    
     // -- DEFAULTS UTILITIES
 
     public ManagedObject nullToEmpty(
diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/object/MmSortUtils.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/object/MmSortUtils.java
new file mode 100644
index 0000000000..347c827cf0
--- /dev/null
+++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/object/MmSortUtils.java
@@ -0,0 +1,84 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.causeway.core.metamodel.object;
+
+import java.util.Comparator;
+import java.util.Objects;
+
+import org.springframework.lang.Nullable;
+
+import org.apache.causeway.commons.internal.base._Objects;
+import org.apache.causeway.core.metamodel.consent.InteractionInitiatedBy;
+import org.apache.causeway.core.metamodel.spec.feature.ObjectAssociation;
+
+import lombok.val;
+import lombok.experimental.UtilityClass;
+
+@UtilityClass
+public class MmSortUtils {
+
+    public enum SortDirection {
+        ASCENDING,
+        DESCENDING;
+    }
+
+    public Comparator<ManagedObject> orderingBy(final ObjectAssociation sortProperty, final SortDirection sortDirection) {
+        final Comparator<ManagedObject> comparator = (sortDirection != SortDirection.DESCENDING)
+                ? NATURAL_NULL_FIRST
+                : NATURAL_NULL_FIRST.reversed();
+
+        return (p, q) -> {
+            val pSort = sortProperty.get(p, InteractionInitiatedBy.FRAMEWORK);
+            val qSort = sortProperty.get(q, InteractionInitiatedBy.FRAMEWORK);
+            return comparator.compare(pSort, qSort);
+        };
+    }
+
+    public int compare(final @Nullable ManagedObject p, final @Nullable ManagedObject q) {
+        return NATURAL_NULL_FIRST.compare(p, q);
+    }
+
+    // -- PREDEFINED COMPARATOR
+
+    final Comparator<ManagedObject> NATURAL_NULL_FIRST = new Comparator<ManagedObject>(){
+        @SuppressWarnings({"rawtypes" })
+        @Override
+        public int compare(final @Nullable ManagedObject a, final @Nullable ManagedObject b) {
+            val aPojo = MmUnwrapUtils.single(a);
+            val bPojo = MmUnwrapUtils.single(b);
+            if(Objects.equals(aPojo, bPojo)) {
+                return 0;
+            }
+            if((aPojo==null
+                    || aPojo instanceof Comparable)
+                && (bPojo==null
+                        || bPojo instanceof Comparable)) {
+                return _Objects.compareNullsFirst((Comparable)aPojo, (Comparable)bPojo);
+            }
+            final int hashCompare = Integer.compare(Objects.hashCode(aPojo), Objects.hashCode(bPojo));
+            if(hashCompare!=0) {
+                return hashCompare;
+            }
+            //XXX on hash-collision we return an arbitrary non-equal relation (unspecified behavior)
+            return -1;
+        }
+
+    };
+
+}
diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/tabular/interactive/DataTableInteractive.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/tabular/interactive/DataTableInteractive.java
index 80b098ad59..4378e77bf4 100644
--- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/tabular/interactive/DataTableInteractive.java
+++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/tabular/interactive/DataTableInteractive.java
@@ -19,6 +19,7 @@
 package org.apache.causeway.core.metamodel.tabular.interactive;
 
 import java.io.Serializable;
+import java.util.Comparator;
 import java.util.Map;
 import java.util.Optional;
 import java.util.UUID;
@@ -52,6 +53,7 @@ import org.apache.causeway.core.metamodel.interactions.managed.ManagedCollection
 import org.apache.causeway.core.metamodel.interactions.managed.ManagedMember;
 import org.apache.causeway.core.metamodel.interactions.managed.MultiselectChoices;
 import org.apache.causeway.core.metamodel.object.ManagedObject;
+import org.apache.causeway.core.metamodel.object.MmSortUtils;
 import org.apache.causeway.core.metamodel.object.PackedManagedObject;
 import org.apache.causeway.core.metamodel.spec.ObjectSpecification;
 import org.apache.causeway.core.metamodel.spec.feature.ObjectMember;
@@ -110,6 +112,7 @@ implements MultiselectChoices {
     @Getter private final @NonNull LazyObservable<Can<DataRow>> dataRowsFiltered;
     @Getter private final @NonNull LazyObservable<Can<DataRow>> dataRowsSelected;
     @Getter private final _BindableAbstract<Boolean> selectAllToggle;
+    @Getter private final _BindableAbstract<ColumnSort> columnSort;
 
     @Getter private final @NonNull LazyObservable<Can<DataColumn>> dataColumns;
     @Getter private final @NonNull LazyObservable<String> title;
@@ -128,12 +131,13 @@ implements MultiselectChoices {
             MetaModelContext.instanceElseFail()::injectServicesInto));
 
         searchArgument = _Bindables.forValue(null);
+        columnSort = _Bindables.forValue(null);
 
         dataRowsFiltered = _Observables.lazy(()->
             dataElements.getValue().stream()
                 //XXX future extension: filter by searchArgument
                 .filter(this::ignoreHidden)
-                .sorted(managedMember.getMetaModel().getElementComparator()
+                .sorted(sortingComparator()
                         .orElseGet(()->(a, b)->0)) // else don't sort (no-op comparator for streams)
                 .map(domainObject->new DataRow(this, domainObject))
                 .collect(Can.toCan()));
@@ -163,6 +167,10 @@ implements MultiselectChoices {
             dataRowsSelected.invalidate();
         });
 
+        columnSort.addListener((e,o,n)->{
+            dataRowsFiltered.invalidate();
+        });
+
         dataColumns = _Observables.lazy(()->
             managedMember.getElementType()
             .streamAssociationsForColumnRendering(managedMember.getIdentifier(), managedMember.getOwner())
@@ -207,6 +215,30 @@ implements MultiselectChoices {
                 .findFirst());
     }
 
+    // -- SORTING
+
+    /**
+     * Sorting helper class, that has the column index to be sorted by and the sort direction.
+     */
+    @RequiredArgsConstructor
+    public static class ColumnSort implements Serializable {
+        private static final long serialVersionUID = 1L;
+        final int columnIndex;
+        final MmSortUtils.SortDirection sortDirection;
+        Optional<Comparator<ManagedObject>> asComparator(final Can<DataColumn> columns) {
+            val columnToSort = columns.get(columnIndex).orElse(null);
+            val sortProperty = columnToSort.getAssociationMetaModel().getSpecialization().leftIfAny();
+            return Optional.ofNullable(sortProperty)
+                    .map(prop->MmSortUtils.orderingBy(sortProperty, sortDirection));
+        }
+    }
+
+    private Optional<Comparator<ManagedObject>> sortingComparator() {
+        return Optional.ofNullable(columnSort.getValue())
+                .flatMap(sort->sort.asComparator(dataColumns.getValue()))
+                .or(()->managedMember.getMetaModel().getElementComparator());
+    }
+
     // -- TOGGLE ALL
 
     final AtomicBoolean isToggleAllEvent = new AtomicBoolean();
diff --git a/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/object/ManagedObjectTest.java b/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/object/ManagedObjectTest.java
index 1912f933be..e46080ec8b 100644
--- a/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/object/ManagedObjectTest.java
+++ b/core/metamodel/src/test/java/org/apache/causeway/core/metamodel/object/ManagedObjectTest.java
@@ -242,7 +242,7 @@ class ManagedObjectTest {
                 .collect(Collectors.toList());
 
         // when, then ... if broken throws java.lang.IllegalArgumentException: Comparison method violates its general contract!
-        managedObjects.sort(ManagedObjects.NATURAL_NULL_FIRST);
+        managedObjects.sort(MmSortUtils.NATURAL_NULL_FIRST);
     }
 
 }
diff --git a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsSortableDataProvider.java b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsSortableDataProvider.java
index bf9e9deb63..b2746494af 100644
--- a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsSortableDataProvider.java
+++ b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsSortableDataProvider.java
@@ -20,22 +20,24 @@ package org.apache.causeway.viewer.wicket.ui.components.collectioncontents.ajaxt
 
 import java.util.Iterator;
 import java.util.Optional;
+import java.util.OptionalInt;
 
 import org.apache.wicket.extensions.ajax.markup.html.repeater.data.table.AjaxFallbackDefaultDataTable;
 import org.apache.wicket.extensions.markup.html.repeater.util.SortParam;
 import org.apache.wicket.extensions.markup.html.repeater.util.SortableDataProvider;
 import org.apache.wicket.model.IModel;
 
+import org.springframework.lang.Nullable;
+
 import org.apache.causeway.applib.annotation.TableDecorator;
-import org.apache.causeway.commons.collections.Can;
 import org.apache.causeway.core.metamodel.facets.object.tabledec.TableDecoratorFacet;
-import org.apache.causeway.core.metamodel.object.ManagedObjects;
-import org.apache.causeway.core.metamodel.spec.feature.OneToOneAssociation;
+import org.apache.causeway.core.metamodel.object.MmSortUtils;
 import org.apache.causeway.core.metamodel.tabular.interactive.DataRow;
 import org.apache.causeway.core.metamodel.tabular.interactive.DataTableInteractive;
 import org.apache.causeway.viewer.wicket.model.models.EntityCollectionModelAbstract;
 import org.apache.causeway.viewer.wicket.model.models.interaction.coll.DataRowWkt;
 
+import lombok.NonNull;
 import lombok.val;
 
 /**
@@ -77,26 +79,39 @@ extends SortableDataProvider<DataRow, String> {
 
     @Override
     public Iterator<DataRow> iterator(final long skip, final long limit) {
-        val visibleRows = getDataTableModel().getDataRowsFiltered().getValue();
-        return sorted(visibleRows).iterator(Math.toIntExact(skip), Math.toIntExact(limit));
+        var dataTable = getDataTableModel();
+        // honor (single) column sort (if any)
+        dataTable.getColumnSort().setValue(columnSort().orElse(null));
+        return dataTable.getDataRowsFiltered().getValue()
+                .iterator(Math.toIntExact(skip), Math.toIntExact(limit));
     }
 
     // -- HELPER
 
-    private Can<DataRow> sorted(final Can<DataRow> dataRows) {
-        val sort = getSort();
-        val sortProperty = lookupPropertyFor(sort).orElse(null);
-        if(sortProperty != null) {
-            val objComparator = ManagedObjects.orderingBy(sortProperty, sort.isAscending());
-            return dataRows.sorted((a, b)->objComparator.compare(a.getRowElement(), b.getRowElement()));
+    private Optional<DataTableInteractive.ColumnSort> columnSort() {
+        val sortParam = getSort();
+        return lookupColumnIndexFor(sortParam).stream()
+                .mapToObj(columnIndex->
+                    new DataTableInteractive.ColumnSort(columnIndex, sortDirection(sortParam)))
+                .findFirst();
+    }
+
+    private OptionalInt lookupColumnIndexFor(final @Nullable SortParam<String> sortParam) {
+        if(sortParam==null) return OptionalInt.empty();
+        int columnIndex = 0;
+        for(var column : getDataTableModel().getDataColumns().getValue()) {
+            if(column.getAssociationMetaModel().getId().equals(sortParam.getProperty())) {
+                return OptionalInt.of(columnIndex);
+            }
+            ++columnIndex;
         }
-        return dataRows;
+        return OptionalInt.empty();
     }
 
-    private Optional<OneToOneAssociation> lookupPropertyFor(final SortParam<String> sort) {
-        return Optional.ofNullable(sort)
-        .map(SortParam::getProperty)
-        .flatMap(getDataTableModel().getElementType()::getProperty);
+    private static MmSortUtils.SortDirection sortDirection(final @NonNull SortParam<String> sortParam) {
+        return sortParam.isAscending()
+                ? MmSortUtils.SortDirection.ASCENDING
+                : MmSortUtils.SortDirection.DESCENDING;
     }
 
 }
\ No newline at end of file