From 9a00823c25270e27f4d2bb2c9c49383041c674a8 Mon Sep 17 00:00:00 2001 From: nscuro Date: Sun, 21 Apr 2024 13:30:10 +0200 Subject: [PATCH] Check persistence metadata when sorting Only persistent fields can be queried and sorted by. Trying to sort by a non-persistent field will cause an exception to be thrown in the persistence layer. To users it is not immediately obvious which field is persistent, and which is computed. It would be good to provide a helpful error message as to what went wrong when a sorting request can not be fulfilled. Instead of checking the candidate classes' fields directly, leverage the type metadata of DataNucleus to figure out which field is persistent. Use specific exception messages for whether a field is not persistent, or doesn't exist at all. Applications built on Alpine can then define a custom `ExceptionMapper` to transform the new `NotSortableException` into an API-friendly format if desired. Alternatively, it can be handled with any generic `IllegalArgumentException` handling logic. Signed-off-by: nscuro --- .../AbstractAlpineQueryManager.java | 44 +++++++++++++----- .../persistence/NotSortableException.java | 46 +++++++++++++++++++ 2 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 alpine-infra/src/main/java/alpine/persistence/NotSortableException.java diff --git a/alpine-infra/src/main/java/alpine/persistence/AbstractAlpineQueryManager.java b/alpine-infra/src/main/java/alpine/persistence/AbstractAlpineQueryManager.java index 6753289c..924cb312 100644 --- a/alpine-infra/src/main/java/alpine/persistence/AbstractAlpineQueryManager.java +++ b/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 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 Query decorate(final Query 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 iq = ((JDOQuery) 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; diff --git a/alpine-infra/src/main/java/alpine/persistence/NotSortableException.java b/alpine-infra/src/main/java/alpine/persistence/NotSortableException.java new file mode 100644 index 00000000..eee3dfd8 --- /dev/null +++ b/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; + } + +}