Merge pull request #554 from nscuro/sort-persistence-check

Check persistence metadata when sorting

alpine-infra/src/main/java/alpine/persistence/AbstractAlpineQueryManager.java

@@ -18,18 +18,20 @@
  */
 package alpine.persistence;
 
-import alpine.common.logging.Logger;
-import alpine.resources.AlpineRequest;
 import alpine.common.validation.RegexSequence;
+import alpine.resources.AlpineRequest;
 import io.jsonwebtoken.lang.Collections;
 import org.apache.commons.collections4.CollectionUtils;
 import org.datanucleus.api.jdo.JDOQuery;
+
 import javax.jdo.FetchPlan;
 import javax.jdo.PersistenceManager;
 import javax.jdo.Query;
-import java.lang.reflect.Field;
+import javax.jdo.metadata.MemberMetadata;
+import javax.jdo.metadata.TypeMetadata;
 import java.security.Principal;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -48,7 +50,6 @@
  */
 public abstract class AbstractAlpineQueryManager implements AutoCloseable {
 
-    private static final Logger LOGGER = Logger.getLogger(AbstractAlpineQueryManager.class);
     private static final ServiceLoader<IPersistenceManagerFactory> IpmfServiceLoader = ServiceLoader.load(IPersistenceManagerFactory.class);
 
     protected final Principal principal;
@@ -262,7 +263,7 @@ public void advancePagination() {
      * @return a Collection of objects
      * @since 1.0.0
      */
-    public Query decorate(final Query query) {
+    public <T> Query<T> decorate(final Query<T> query) {
         // Clear the result to fetch if previously specified (i.e. by getting count)
         query.setResult(null);
         if (pagination != null && pagination.isPaginated()) {
@@ -272,17 +273,38 @@ public Query decorate(final Query query) {
         }
         if (orderBy != null && RegexSequence.Pattern.STRING_IDENTIFIER.matcher(orderBy).matches() && orderDirection != OrderDirection.UNSPECIFIED) {
             // Check to see if the specified orderBy field is defined in the class being queried.
-            boolean found = false;
-            final org.datanucleus.store.query.Query iq = ((JDOQuery) query).getInternalQuery();
+            // NB: Only persistent fields can be used as sorting subject.
+            final org.datanucleus.store.query.Query<T> iq = ((JDOQuery<T>) query).getInternalQuery();
             final String candidateField = orderBy.contains(".") ? orderBy.substring(0, orderBy.indexOf('.')) : orderBy;
-            for (final Field field: iq.getCandidateClass().getDeclaredFields()) {
-                if (candidateField.equals(field.getName())) {
-                    found = true;
+            final TypeMetadata candidateTypeMetadata = pm.getPersistenceManagerFactory().getMetadata(iq.getCandidateClassName());
+            if (candidateTypeMetadata == null) {
+                // NB: If this happens then the entire query is broken and needs programmatic fixing.
+                // Throwing an exception here to make this painfully obvious.
+                throw new IllegalStateException("""
+                        Persistence type metadata for candidate class %s could not be found. \
+                        Querying for non-persistent types is not supported, correct your query.\
+                        """.formatted(iq.getCandidateClassName()));
+            }
+            boolean foundPersistentMember = false;
+            for (final MemberMetadata memberMetadata : candidateTypeMetadata.getMembers()) {
+                if (candidateField.equals(memberMetadata.getName())) {
+                    foundPersistentMember = true;
                     break;
                 }
             }
-            if (found) {
+            if (foundPersistentMember) {
                 query.setOrdering(orderBy + " " + orderDirection.name().toLowerCase());
+            } else {
+                // Is it a non-persistent (transient) field?
+                final boolean foundNonPersistentMember = Arrays.stream(iq.getCandidateClass().getDeclaredFields())
+                        .anyMatch(field -> field.getName().equals(candidateField));
+                if (foundNonPersistentMember) {
+                    throw new NotSortableException(iq.getCandidateClass().getSimpleName(), candidateField,
+                            "The field is computed and can not be queried or sorted by");
+                }
+
+                throw new NotSortableException(iq.getCandidateClass().getSimpleName(), candidateField,
+                        "The field does not exist");
             }
         }
         return query;

alpine-infra/src/main/java/alpine/persistence/NotSortableException.java

@@ -0,0 +1,46 @@
+/*
+ * This file is part of Alpine.
+ *
+ * Licensed 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.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (c) Steve Springett. All Rights Reserved.
+ */
+package alpine.persistence;
+
+public class NotSortableException extends IllegalArgumentException {
+
+    private final String resourceName;
+    private final String fieldName;
+    private final String reason;
+
+    NotSortableException(final String resourceName, final String fieldName, final String reason) {
+        super("Can not sort by %s#%s: %s".formatted(resourceName, fieldName, reason));
+        this.resourceName = resourceName;
+        this.fieldName = fieldName;
+        this.reason = reason;
+    }
+
+    public String getResourceName() {
+        return resourceName;
+    }
+
+    public String getFieldName() {
+        return fieldName;
+    }
+
+    public String getReason() {
+        return reason;
+    }
+
+}