diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/CatalogInfoLiteralAdaptor.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/CatalogInfoLiteralAdaptor.java new file mode 100644 index 000000000..dc28c1ebe --- /dev/null +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/CatalogInfoLiteralAdaptor.java @@ -0,0 +1,151 @@ +/* + * (c) 2024 Open Source Geospatial Foundation - all rights reserved This code is licensed under the + * GPL 2.0 license, available at the root application directory. + */ +package org.geoserver.cloud.backend.pgconfig.catalog.filter; + +import lombok.NonNull; +import lombok.extern.slf4j.Slf4j; + +import org.geoserver.catalog.CatalogInfo; +import org.geoserver.catalog.WorkspaceInfo; +import org.geotools.api.filter.BinaryComparisonOperator; +import org.geotools.api.filter.Filter; +import org.geotools.api.filter.FilterFactory; +import org.geotools.api.filter.MultiValuedFilter.MatchAction; +import org.geotools.api.filter.PropertyIsEqualTo; +import org.geotools.api.filter.PropertyIsGreaterThan; +import org.geotools.api.filter.PropertyIsGreaterThanOrEqualTo; +import org.geotools.api.filter.PropertyIsLessThan; +import org.geotools.api.filter.PropertyIsLessThanOrEqualTo; +import org.geotools.api.filter.PropertyIsNotEqualTo; +import org.geotools.api.filter.expression.Expression; +import org.geotools.api.filter.expression.Literal; +import org.geotools.api.filter.expression.PropertyName; +import org.geotools.factory.CommonFactoryFinder; +import org.geotools.filter.visitor.DuplicatingFilterVisitor; + +import java.util.Optional; +import java.util.Set; + +/** + * Converts binary comparison operators using a property name and a {@link CatalogInfo} instance + * literal as a comparison by id. + * + *

For example, a filter like {@code workspace = WorkspaceInfo[test]} is translated to {@code + * workspace.id = WorkspaceInfo[test].id}, where {@code WorkspaceInfo[test]} is an instance of a + * {@link WorkspaceInfo}. + * + *

+ * + * @since 1.8.1 + */ +@Slf4j(topic = "org.geoserver.cloud.backend.pgconfig.catalog.filter") +class CatalogInfoLiteralAdaptor extends DuplicatingFilterVisitor { + + private static final FilterFactory FACTORY = CommonFactoryFinder.getFilterFactory(null); + + private Set supportedPropertyNames; + + public CatalogInfoLiteralAdaptor(@NonNull Set supportedPropertyNames) { + super(FACTORY); + this.supportedPropertyNames = supportedPropertyNames; + } + + public static Filter replaceCatalogInfoLiterals( + Filter filter, Set supportedPropertyNames) { + var adaptor = new CatalogInfoLiteralAdaptor(supportedPropertyNames); + return (Filter) filter.accept(adaptor, null); + } + + @Override + public Object visit(PropertyIsEqualTo filter, Object notUsed) { + return super.visit(adapt(filter, ff::equal), notUsed); + } + + @Override + public Object visit(PropertyIsNotEqualTo filter, Object notUsed) { + return super.visit(adapt(filter, ff::notEqual), notUsed); + } + + @Override + public Object visit(PropertyIsGreaterThan filter, Object notUsed) { + return super.visit(adapt(filter, ff::greater), notUsed); + } + + @Override + public Object visit(PropertyIsGreaterThanOrEqualTo filter, Object notUsed) { + return super.visit(adapt(filter, ff::greaterOrEqual), notUsed); + } + + @Override + public Object visit(PropertyIsLessThan filter, Object notUsed) { + return super.visit(adapt(filter, ff::less), notUsed); + } + + @Override + public Object visit(PropertyIsLessThanOrEqualTo filter, Object notUsed) { + return super.visit(adapt(filter, ff::lessOrEqual), notUsed); + } + + @FunctionalInterface + private static interface BinaryComparisonBuilder { + F build(Expression left, Expression right, boolean matchCase, MatchAction matchAction); + } + + /** + * Converts a binary comparision operator using a property name and a {@link CatalogInfo} + * instance literal as a comparison by id. + * + *

For example, a filter like {@code workspace = WorkspaceInfo} is translated to {@code + * workspace.id = WorkspaceInfo.id}, where {@code WorkspaceInfo} is an instanceo of a {@link + * WorkspaceInfo}. + */ + private F adapt( + F filter, BinaryComparisonBuilder builder) { + + PropertyName prop = propertyName(filter); + Literal literal = literal(filter); + if (prop != null && literal != null) { + final Filter orig = filter; + String propertyName = prop.getPropertyName(); + Object value = literal.getValue(); + if (value instanceof CatalogInfo info) { + String idProp = "%s.id".formatted(propertyName); + if (supportedPropertyNames.contains(idProp)) { + prop = ff.property(idProp); + literal = ff.literal(info.getId()); + boolean matchingCase = filter.isMatchingCase(); + MatchAction matchAction = filter.getMatchAction(); + filter = builder.build(prop, literal, matchingCase, matchAction); + log.debug( + "Fitler with CatalogInfo literal '{}' translated to '{}'", + orig, + filter); + } + } + } + + return filter; + } + + private PropertyName propertyName(BinaryComparisonOperator filter) { + return propertyName(filter.getExpression1()) + .or(() -> propertyName(filter.getExpression2())) + .orElse(null); + } + + private Literal literal(BinaryComparisonOperator filter) { + return literal(filter.getExpression1()) + .or(() -> literal(filter.getExpression2())) + .orElse(null); + } + + private Optional propertyName(Expression e) { + return Optional.of(e).filter(PropertyName.class::isInstance).map(PropertyName.class::cast); + } + + private Optional literal(Expression e) { + return Optional.of(e).filter(Literal.class::isInstance).map(Literal.class::cast); + } +} diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlCatalogFilterSplitter.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlCatalogFilterSplitter.java index ff41ff8e0..760c0c6ec 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlCatalogFilterSplitter.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlCatalogFilterSplitter.java @@ -4,6 +4,8 @@ */ package org.geoserver.cloud.backend.pgconfig.catalog.filter; +import lombok.NonNull; + import org.geotools.api.filter.Filter; import org.geotools.api.filter.expression.PropertyName; import org.geotools.filter.visitor.PostPreProcessFilterSplittingVisitor; @@ -20,15 +22,14 @@ class PgsqlCatalogFilterSplitter extends PostPreProcessFilterSplittingVisitor { private Set supportedPropertyNames; - public PgsqlCatalogFilterSplitter(Set supportedPropertyNames) { + public PgsqlCatalogFilterSplitter(@NonNull Set supportedPropertyNames) { super(PgsqlFilterCapabilities.capabilities(), null, null); this.supportedPropertyNames = supportedPropertyNames; } public static PgsqlCatalogFilterSplitter split( Filter filter, Set supportedPropertyNames) { - PgsqlCatalogFilterSplitter splitter = - new PgsqlCatalogFilterSplitter(supportedPropertyNames); + var splitter = new PgsqlCatalogFilterSplitter(supportedPropertyNames); filter.accept(splitter, null); return splitter; } diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlFilterCapabilities.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlFilterCapabilities.java index 09113a761..a7dafa092 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlFilterCapabilities.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlFilterCapabilities.java @@ -6,6 +6,7 @@ import lombok.experimental.UtilityClass; +import org.geoserver.function.IsInstanceOf; import org.geotools.api.filter.ExcludeFilter; import org.geotools.api.filter.Id; import org.geotools.api.filter.IncludeFilter; @@ -113,6 +114,9 @@ static FilterCapabilities createFilterCapabilities() { // compare functions caps.addType(FilterFunction_equalTo.class); + // well-known VolatileFunction implementations + caps.addType(IsInstanceOf.class); + return caps; } } diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlFilterToSQL.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlFilterToSQL.java index 192917761..984f79de7 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlFilterToSQL.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlFilterToSQL.java @@ -4,14 +4,22 @@ */ package org.geoserver.cloud.backend.pgconfig.catalog.filter; +import lombok.NonNull; import lombok.Value; +import org.geoserver.catalog.CatalogInfo; +import org.geoserver.catalog.impl.ClassMappings; +import org.geoserver.function.IsInstanceOf; import org.geotools.api.filter.Filter; +import org.geotools.api.filter.PropertyIsEqualTo; import org.geotools.api.filter.PropertyIsLike; import org.geotools.api.filter.expression.Expression; import org.geotools.api.filter.expression.Function; +import org.geotools.api.filter.expression.Literal; +import org.geotools.api.filter.expression.PropertyName; import org.geotools.filter.LengthFunction; import org.geotools.filter.LikeFilterImpl; +import org.geotools.filter.LiteralExpressionImpl; import org.geotools.filter.function.FilterFunction_strConcat; import org.geotools.filter.function.FilterFunction_strEndsWith; import org.geotools.filter.function.FilterFunction_strEqualsIgnoreCase; @@ -28,13 +36,18 @@ import org.geotools.filter.function.math.FilterFunction_abs_3; import org.geotools.filter.function.math.FilterFunction_abs_4; import org.geotools.jdbc.PreparedFilterToSQL; +import org.geotools.util.Converters; import java.io.IOException; import java.io.StringWriter; import java.io.UncheckedIOException; import java.io.Writer; +import java.util.Collection; import java.util.Date; import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * @since 1.4 @@ -60,7 +73,7 @@ public PgsqlFilterToSQL(Writer out) { public static Result evaluate(Filter filter) { StringWriter out = new StringWriter(); - PgsqlFilterToSQL filterToPreparedStatement = new PgsqlFilterToSQL(out); + var filterToPreparedStatement = new PgsqlFilterToSQL(out); filterToPreparedStatement.setSqlNameEscape("\""); filter.accept(filterToPreparedStatement, null); out.flush(); @@ -72,6 +85,56 @@ public static Result evaluate(Filter filter) { return new Result(whereClause, literalValues, literalTypes); } + @Override + public Object visit(PropertyIsEqualTo filter, Object extraData) { + Expression left = filter.getExpression1(); + Expression right = filter.getExpression2(); + + PropertyName prop = + Optional.of(left) + .filter(PropertyName.class::isInstance) + .or(() -> Optional.of(right).filter(PropertyName.class::isInstance)) + .map(PropertyName.class::cast) + .orElse(null); + if (isArray(prop)) { + Expression value = right; + if (right instanceof Literal literal) { + String values = + asList(literal.getValue()).stream() + .map(o -> Converters.convert(o, String.class)) + .map("'%s'"::formatted) + .collect(Collectors.joining(",")); + value = new LiteralExpressionImpl("ARRAY[%s]".formatted(values)); + } + try { + prop.accept(this, extraData); + out.write(" && "); + out.write((String) ((Literal) value).getValue()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + + return extraData; + } + + return super.visit(filter, extraData); + } + + @SuppressWarnings({"unchecked", "java:S6204"}) + private List asList(Object value) { + if (value instanceof Collection) { + // beware Stream.toList() does not support null values but Collectors.toList() does + return ((Collection) value).stream().collect(Collectors.toList()); + } + return List.of(value); + } + + private boolean isArray(PropertyName prop) { + if (null == prop) return false; + String propertyName = prop.getPropertyName(); + return "styles.id".equals(propertyName) || "layers.id".equals(propertyName); + } + /** * Writes the SQL for the Like Filter. Assumes the current java implemented wildcards for the * Like Filter: . for multi and .? for single. And replaces them with the SQL % and _, @@ -262,7 +325,35 @@ protected boolean visitFunction(Function function) throws IOException { out.write(")"); return true; } + if (function instanceof IsInstanceOf instanceOf) { + Expression typeExpr = getParameter(instanceOf, 0, true); + @SuppressWarnings("unchecked") + Class type = typeExpr.evaluate(null, Class.class); + + String types = infoTypes(type); + String f = + """ + "@type" = ANY('{%s}') + """ + .formatted(types); + out.write(f); + return true; + } + // function not supported return false; } + + protected @NonNull String infoTypes(Class clazz) { + ClassMappings cm; + if (clazz.isInterface()) cm = ClassMappings.fromInterface(clazz); + else cm = ClassMappings.fromImpl(clazz); + if (null == cm) + throw new IllegalArgumentException( + "Unknown type for IsInstanceOf: " + clazz.getCanonicalName()); + + return Stream.of(cm.concreteInterfaces()) + .map(Class::getSimpleName) + .collect(Collectors.joining(",")); + } } diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlQueryBuilder.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlQueryBuilder.java index f4db3b067..e65adc437 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlQueryBuilder.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/PgsqlQueryBuilder.java @@ -4,6 +4,9 @@ */ package org.geoserver.cloud.backend.pgconfig.catalog.filter; +import static org.geoserver.cloud.backend.pgconfig.catalog.filter.CatalogInfoLiteralAdaptor.replaceCatalogInfoLiterals; +import static org.geoserver.cloud.backend.pgconfig.catalog.filter.PgsqlCatalogFilterSplitter.split; + import lombok.Getter; import lombok.RequiredArgsConstructor; @@ -20,7 +23,7 @@ @RequiredArgsConstructor public class PgsqlQueryBuilder { - private final Filter filter; + private final Filter origFilter; private final Set supportedPropertyNames; private @Getter Filter supportedFilter = Filter.INCLUDE; @@ -33,12 +36,17 @@ public class PgsqlQueryBuilder { private @Getter List literalTypes; public PgsqlQueryBuilder build() { - if (Filter.INCLUDE.equals(filter)) { + if (Filter.INCLUDE.equals(origFilter)) { return this; } - var splitter = PgsqlCatalogFilterSplitter.split(filter, supportedPropertyNames); + + Filter filter = replaceCatalogInfoLiterals(origFilter, supportedPropertyNames); + filter = simplify(filter); + + var splitter = split(filter, supportedPropertyNames); supportedFilter = adaptToSql(splitter.getFilterPre()); + unsupportedFilter = simplify(splitter.getFilterPost()); Result encodeResult = PgsqlFilterToSQL.evaluate(supportedFilter); @@ -49,7 +57,8 @@ public PgsqlQueryBuilder build() { } private Filter adaptToSql(Filter filterPre) { - Filter supported = ToPgsqlCompatibleFilterDuplicator.adapt(filterPre); + Filter supported = + ToPgsqlCompatibleFilterDuplicator.adapt(filterPre, supportedPropertyNames); return simplify(supported); } diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/ToPgsqlCompatibleFilterDuplicator.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/ToPgsqlCompatibleFilterDuplicator.java index 40a79b803..9e2d33f43 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/ToPgsqlCompatibleFilterDuplicator.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/filter/ToPgsqlCompatibleFilterDuplicator.java @@ -4,6 +4,9 @@ */ package org.geoserver.cloud.backend.pgconfig.catalog.filter; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; + import org.geotools.api.filter.BinaryComparisonOperator; import org.geotools.api.filter.Filter; import org.geotools.api.filter.MultiValuedFilter.MatchAction; @@ -22,6 +25,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; @@ -33,14 +37,18 @@ * * @since 1.4 */ +@RequiredArgsConstructor class ToPgsqlCompatibleFilterDuplicator extends DuplicatingFilterVisitor { + @NonNull private final Set supportedPropertyNames; + /** * @param supportedFilter Filter that's already been deemed as supported + * @param supportedPropertyNames * @return */ - public static Filter adapt(Filter supportedFilter) { - ToPgsqlCompatibleFilterDuplicator adaptor = new ToPgsqlCompatibleFilterDuplicator(); + public static Filter adapt(Filter supportedFilter, Set supportedPropertyNames) { + var adaptor = new ToPgsqlCompatibleFilterDuplicator(supportedPropertyNames); return (Filter) supportedFilter.accept(adaptor, null); } diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/CatalogInfoRowMapper.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/CatalogInfoRowMapper.java index a5c33866a..16d9fac98 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/CatalogInfoRowMapper.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/CatalogInfoRowMapper.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.RequiredArgsConstructor; import lombok.Setter; import lombok.extern.slf4j.Slf4j; @@ -27,6 +28,7 @@ import java.io.UncheckedIOException; import java.sql.ResultSet; import java.sql.SQLException; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -43,7 +45,7 @@ public final class CatalogInfoRowMapper { protected static final ObjectMapper objectMapper = PgsqlObjectMapper.newObjectMapper(); /** Lazily created by {@link #cache()} */ - private Map cache; + private Map, Map> cache; private @Setter Function> styleLoader; @@ -51,26 +53,35 @@ private CatalogInfoRowMapper() { // private constructor } - @SuppressWarnings("serial") - private Map cache() { + @SuppressWarnings("unchecked") + private Map cache(Class clazz) { if (cache == null) { - cache = - new LinkedHashMap<>() { - @Override - protected boolean removeEldestEntry(Map.Entry eldest) { - int size = size(); - boolean remove = size > 100; - if (remove) { - log.trace( - "RowMapper cache size: {}, removing eldest entry {}", - size, - eldest.getValue()); - } - return remove; - } - }; + cache = new HashMap<>(); + } + return (Map) cache.computeIfAbsent(clazz, c -> new LRUCache<>(100)); + } + + @SuppressWarnings({"serial", "java:S2160"}) + @RequiredArgsConstructor + private static class LRUCache extends LinkedHashMap { + final int maxCapacity; + + @Override + @SuppressWarnings("unchecked") + public V get(Object key) { + V v = super.get(key); + if (null != v && size() > 1) { + // by definition v is now the most recently used, so bubble it up to the top of the + // list. removeEldestEntry will hence always remove the least recently used. + super.putFirst((K) key, v); + } + return v; + } + + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return size() > maxCapacity; } - return cache; } protected T resolveCached( @@ -82,15 +93,16 @@ protected T resolveCached( protected T resolveCached( String id, Class clazz, Function loader) { if (null == id) return null; - CatalogInfo info = cache().get(id); + var infoCache = cache(clazz); + T info = infoCache.get(id); if (clazz.isInstance(info)) { log.trace("loaded from RowMapper cache: {}", info); } else { info = loader.apply(id); - cache.put(id, info); + infoCache.put(id, info); log.trace("RowMapper cached {}", info); } - return clazz.cast(info); + return info; } /** @@ -270,10 +282,6 @@ public ResourceInfo mapResource(ResultSet rs) { return resource; } - public ResourceInfo mapResource(String id, ResultSet rs) { - return resolveCached(id, ResourceInfo.class, rs, this::mapResource); - } - protected void setStore(ResourceInfo resource, ResultSet rs) { String storeId = resource.getStore().getId(); StoreInfo store = mapStore(storeId, rs); @@ -393,8 +401,9 @@ protected LayerInfo mapLayer(String id, ResultSet rs) { } private void setResource(LayerInfo layer, ResultSet rs) { - String resourceId = layer.getResource().getId(); - ResourceInfo resource = mapResource(resourceId, rs); + // ResourceInfos are not cached, they can only be mapped directly or when resolving a + // layerinfo. In the later, the relationship is 1:1 so caching them would be vane + ResourceInfo resource = mapResource(rs); layer.setResource(ModificationProxy.create(resource, ResourceInfo.class)); } diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/LoggingTemplate.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/LoggingTemplate.java index 6bfad9f4b..06d3e1e1e 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/LoggingTemplate.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/LoggingTemplate.java @@ -103,7 +103,11 @@ private void logAfter(long reqId, String sql, Duration ellapsed, DataAccessExcep ? "" : " (ERROR %s: %s)" .formatted(error.getClass().getSimpleName(), error.getMessage()); - log.debug("after request #{} ({}): '{}'{}", reqId, time, sql, errMsg); + if (error != null && log.isTraceEnabled()) { + log.trace("after request #{} ({}): '{}'{}", reqId, time, sql, errMsg, error); + } else { + log.debug("after request #{} ({}): '{}'{}", reqId, time, sql, errMsg); + } } private String stackTrace() { diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/PgsqlCatalogInfoRepository.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/PgsqlCatalogInfoRepository.java index 5ce48f07e..ba37262fd 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/PgsqlCatalogInfoRepository.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/PgsqlCatalogInfoRepository.java @@ -11,6 +11,8 @@ import lombok.extern.slf4j.Slf4j; import org.geoserver.catalog.CatalogInfo; +import org.geoserver.catalog.Info; +import org.geoserver.catalog.Predicates; import org.geoserver.catalog.impl.ClassMappings; import org.geoserver.catalog.plugin.CatalogInfoRepository; import org.geoserver.catalog.plugin.Patch; @@ -19,8 +21,10 @@ import org.geoserver.catalog.plugin.resolving.ResolvingFacade; import org.geoserver.cloud.backend.pgconfig.catalog.filter.PgsqlQueryBuilder; import org.geotools.api.filter.Filter; +import org.geotools.api.filter.FilterFactory; import org.geotools.api.filter.sort.SortBy; import org.geotools.api.filter.sort.SortOrder; +import org.geotools.factory.CommonFactoryFinder; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; @@ -46,10 +50,12 @@ public abstract class PgsqlCatalogInfoRepository extends ResolvingCatalogInfoRepository implements CatalogInfoRepository, ResolvingFacade { - protected final @NonNull LoggingTemplate template; - protected static final ObjectMapper infoMapper = PgsqlObjectMapper.newObjectMapper(); + protected static final FilterFactory FILTER_FACTORY = CommonFactoryFinder.getFilterFactory(); + + protected final @NonNull LoggingTemplate template; + private Set sortableProperties; /** @@ -150,13 +156,13 @@ public I update(@NonNull I value, @NonNull Patch patch) { @Override public Stream findAll(Query query) { - final Filter filter = query.getFilter(); + final Filter filter = applyTypeFilter(query.getFilter(), query.getType()); + final Class type = query.getType(); final PgsqlQueryBuilder qb = new PgsqlQueryBuilder(filter, sortableProperties()).build(); final Filter supportedFilter = qb.getSupportedFilter(); final Filter unsupportedFilter = qb.getUnsupportedFilter(); final String whereClause = qb.getWhereClause(); - final Class type = query.getType(); log.trace( "supported filter {} translated to {}, unsupported: {}", @@ -167,15 +173,11 @@ public Stream findAll(Query query) { final boolean filterFullySupported = Filter.INCLUDE.equals(unsupportedFilter); String sql = "SELECT * FROM %s WHERE TRUE".formatted(getQueryTable()); - sql = applyTypeFilter(sql, type); Object[] prepStatementParams = null; if (!Filter.INCLUDE.equals(supportedFilter)) { sql = "%s AND %s".formatted(sql, whereClause); - List literalValues = qb.getLiteralValues(); - if (!literalValues.isEmpty()) { - prepStatementParams = qb.getLiteralValues().toArray(); - } + prepStatementParams = prepareParams(qb); } sql = applySortOrder(sql, query.getSortBy()); @@ -212,6 +214,13 @@ protected Predicate toPredicate(Filter filter) { return info -> null != info && filter.evaluate(info); } + private Filter applyTypeFilter(Filter filter, Class type) { + if (!getContentType().equals(type)) { + filter = Predicates.and(Predicates.isInstanceOf(type), filter); + } + return filter; + } + private String applyTypeFilter(String sql, @NonNull Class type) { if (!getContentType().equals(type)) { String infoType = infoType(type); @@ -252,8 +261,8 @@ protected void checkCanSortBy(String property) { * @return {@code -1} if the {@code filter} is not fully supported */ @Override - public long count(Class of, final Filter filter) { - + public long count(Class of, Filter filter) { + filter = applyTypeFilter(filter, of); final PgsqlQueryBuilder qb = new PgsqlQueryBuilder(filter, sortableProperties()).build(); final Filter supportedFilter = qb.getSupportedFilter(); final Filter unsupportedFilter = qb.getUnsupportedFilter(); @@ -271,15 +280,10 @@ public long count(Class of, final Filter filter) { Object[] prepStatementParams = null; if (Filter.INCLUDE.equals(supportedFilter)) { sql = sql.formatted(getTable()); - sql = applyTypeFilter(sql, of); } else { sql = sql.formatted(getQueryTable()); - sql = applyTypeFilter(sql, of); sql = "%s AND %s".formatted(sql, whereClause); - List literalValues = qb.getLiteralValues(); - if (!literalValues.isEmpty()) { - prepStatementParams = qb.getLiteralValues().toArray(); - } + prepStatementParams = prepareParams(qb); } return template.queryForObject(sql, Long.class, prepStatementParams); } @@ -289,6 +293,18 @@ public long count(Class of, final Filter filter) { } } + @SuppressWarnings("java:S1168") + private Object[] prepareParams(PgsqlQueryBuilder qb) { + List literalValues = qb.getLiteralValues(); + if (literalValues.isEmpty()) return null; + return literalValues.stream().map(this::asPreparedValue).toArray(); + } + + private Object asPreparedValue(Object val) { + if (val instanceof Enum e) return e.name(); + return val; + } + @Override public Optional findById(@NonNull String id, Class clazz) { String query = diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/PgsqlLayerRepository.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/PgsqlLayerRepository.java index 482d5023c..31eb889f6 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/PgsqlLayerRepository.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/catalog/repository/PgsqlLayerRepository.java @@ -10,12 +10,12 @@ import org.geoserver.catalog.ResourceInfo; import org.geoserver.catalog.StyleInfo; import org.geoserver.catalog.plugin.CatalogInfoRepository.LayerRepository; +import org.geoserver.catalog.plugin.Query; +import org.geotools.api.filter.Filter; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.RowMapper; import java.util.Optional; -import java.util.Set; -import java.util.function.Predicate; import java.util.stream.Stream; /** @@ -65,22 +65,15 @@ public Optional findOneByName(@NonNull String possiblyPrefixedName) { return findOne(sql.formatted("name"), possiblyPrefixedName); } - // TODO: optimize @Override public Stream findAllByDefaultStyleOrStyles(@NonNull StyleInfo style) { - return findAll().filter(styleFilter(style)); - } - - private Predicate styleFilter(@NonNull StyleInfo style) { - return l -> { - if (matches(style, l.getDefaultStyle())) return true; - return Optional.ofNullable(l.getStyles()).orElse(Set.of()).stream() - .anyMatch(s -> matches(style, s)); - }; - } + var ff = FILTER_FACTORY; + Filter filter = + ff.or( + ff.equals(ff.property("defaultStyle.id"), ff.literal(style.getId())), + ff.equals(ff.property("styles.id"), ff.literal(style.getId()))); - private boolean matches(StyleInfo expected, StyleInfo actual) { - return actual != null && expected.getId().equals(actual.getId()); + return findAll(Query.valueOf(LayerInfo.class, filter)); } @Override diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/resource/PgsqlResourceStore.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/resource/PgsqlResourceStore.java index 487d6b33a..7b221dd50 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/resource/PgsqlResourceStore.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/backend/pgconfig/resource/PgsqlResourceStore.java @@ -456,11 +456,6 @@ public void close() { public PgsqlResource getParent(PgsqlResource resource) { if (ROOT_ID == resource.getId()) return null; String parentPath = resource.parentPath(); - try { - return (PgsqlResource) get(parentPath); - } catch (RuntimeException e) { - e.printStackTrace(); - throw e; - } + return (PgsqlResource) get(parentPath); } } diff --git a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/config/catalog/backend/pgconfig/PgsqlCatalogResourcesSynchronizer.java b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/config/catalog/backend/pgconfig/PgsqlCatalogResourcesSynchronizer.java index aa8bd84d0..f278e69ac 100644 --- a/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/config/catalog/backend/pgconfig/PgsqlCatalogResourcesSynchronizer.java +++ b/src/catalog/backends/pgconfig/src/main/java/org/geoserver/cloud/config/catalog/backend/pgconfig/PgsqlCatalogResourcesSynchronizer.java @@ -1,3 +1,7 @@ +/* + * (c) 2024 Open Source Geospatial Foundation - all rights reserved This code is licensed under the + * GPL 2.0 license, available at the root application directory. + */ package org.geoserver.cloud.config.catalog.backend.pgconfig; import lombok.extern.slf4j.Slf4j; @@ -27,6 +31,8 @@ /** * A {@link GeoServerConfigPersister} alike {@link CatalogListener} to synchronize {@link * ResourceStore} resources related to catalog changes. + * + * @since 1.8.1 */ @Slf4j(topic = "org.geoserver.cloud.config.catalog.backend.pgconfig") public class PgsqlCatalogResourcesSynchronizer implements CatalogListener, ExtensionPriority { diff --git a/src/catalog/backends/pgconfig/src/main/resources/db/pgsqlcatalog/migration/postgresql/V1_7__PublishedInfo_Multivalued_Id_Queryables.sql b/src/catalog/backends/pgconfig/src/main/resources/db/pgsqlcatalog/migration/postgresql/V1_7__PublishedInfo_Multivalued_Id_Queryables.sql new file mode 100644 index 000000000..a77edbcd5 --- /dev/null +++ b/src/catalog/backends/pgconfig/src/main/resources/db/pgsqlcatalog/migration/postgresql/V1_7__PublishedInfo_Multivalued_Id_Queryables.sql @@ -0,0 +1,85 @@ +/* + * Support indexed queries by layerinfo's styles.id, layergroupinfo's styles.id, and layergroupinfo's layers.id + * + * @since 1.8.1 + */ + +-- Add styles column to layerinfo and a GIN index to contain the style ids +-- when the populate_table_columns_from_jsonb trigger runs +ALTER TABLE layerinfo ADD COLUMN styles TEXT[]; +CREATE INDEX layerinfo_styles_gin_idx ON layerinfo USING GIN (styles); + +-- Add layers column to layergroupinfo and a GIN index to contain the style ids +-- when the populate_table_columns_from_jsonb trigger runs +ALTER TABLE layergroupinfo ADD COLUMN layers TEXT[]; +CREATE INDEX layergroupinfo_layers_gin_idx ON layergroupinfo USING GIN (layers); + +-- Add styles column to layergroupinfo and a GIN index to contain the layer ids +-- when the populate_table_columns_from_jsonb trigger runs +ALTER TABLE layergroupinfo ADD COLUMN styles TEXT[]; +CREATE INDEX layergroupinfo_styles_gin_idx ON layergroupinfo USING GIN (styles); + +-- Force running the populate_table_columns_from_jsonb trigger and update the new columns +UPDATE layerinfo SET info = info; +UPDATE layergroupinfo SET info = info; + +CREATE OR REPLACE VIEW layerinfos +AS + SELECT layer.id AS id, + layer."@type" AS "@type", + -- override layer.name with resource.name while it's coupled in the object model + -- layer.name AS name, + resource.name AS name, + resource.title AS title, + resource.enabled AS enabled, + resource.advertised AS advertised, + layer."defaultStyle" AS "defaultStyle.id", + layer."type" AS "type", + style.name AS "defaultStyle.name", + resource.id AS "resource.id", + resource.name AS "resource.name", + resource."store.workspace.name" || ':' || layer.name AS "prefixedName", + resource.enabled AS "resource.enabled", + resource.advertised AS "resource.advertised", + resource."SRS" AS "resource.SRS", + resource."store.id" AS "resource.store.id", + resource."store.name" AS "resource.store.name", + resource."store.enabled" AS "resource.store.enabled", + resource."store.type" AS "resource.store.type", + resource."store.workspace.id" AS "resource.store.workspace.id", + resource."store.workspace.name" AS "resource.store.workspace.name", + resource."namespace.id" AS "resource.namespace.id", + resource."namespace.prefix" AS "resource.namespace.prefix", + style.filename AS "defaultStyle.filename", + style.format AS "defaultStyle.format", + layer.info AS publishedinfo, + resource.resource AS resource, + resource.store AS store, + resource.workspace AS workspace, + resource.namespace AS namespace, + style.info AS "defaultStyle", + layer.styles AS "styles.id" + FROM layerinfo layer + INNER JOIN resourceinfos resource ON layer.resource = resource.id + LEFT OUTER JOIN styleinfo style ON layer."defaultStyle" = style.id; + +CREATE OR REPLACE VIEW layergroupinfos +AS + SELECT lg.id AS id, + lg."@type" AS "@type", + lg.name AS name, + lg.title AS title, + 'GROUP' AS "type", + concat_ws(':', workspace.name, lg.name) AS "prefixedName", + lg.mode AS mode, + lg.enabled AS enabled, + lg.advertised AS advertised, + lg.workspace AS "workspace.id", + workspace.name AS "workspace.name", + lg.info AS publishedinfo, + workspace.info AS workspace, + lg.layers AS "layers.id", + lg.styles AS "styles.id" + FROM layergroupinfo lg + LEFT OUTER JOIN workspaceinfo workspace + ON lg.workspace = workspace.id; diff --git a/src/catalog/plugin/src/main/java/org/geoserver/catalog/plugin/RepositoryCatalogFacadeImpl.java b/src/catalog/plugin/src/main/java/org/geoserver/catalog/plugin/RepositoryCatalogFacadeImpl.java index c5df9ba92..cab3e3eb1 100644 --- a/src/catalog/plugin/src/main/java/org/geoserver/catalog/plugin/RepositoryCatalogFacadeImpl.java +++ b/src/catalog/plugin/src/main/java/org/geoserver/catalog/plugin/RepositoryCatalogFacadeImpl.java @@ -31,13 +31,21 @@ import org.geoserver.catalog.plugin.CatalogInfoRepository.StoreRepository; import org.geoserver.catalog.plugin.CatalogInfoRepository.StyleRepository; import org.geoserver.catalog.plugin.CatalogInfoRepository.WorkspaceRepository; +import org.geoserver.function.IsInstanceOf; +import org.geotools.api.filter.And; +import org.geotools.api.filter.BinaryComparisonOperator; import org.geotools.api.filter.Filter; +import org.geotools.api.filter.Or; +import org.geotools.api.filter.expression.Expression; import org.geotools.api.filter.sort.SortBy; +import org.geotools.filter.visitor.SimplifyingFilterVisitor; import org.springframework.util.Assert; import java.lang.reflect.Proxy; import java.util.Comparator; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.function.Consumer; @@ -578,8 +586,11 @@ private void sync(Supplier> from, Consumer public int count(final Class of, final Filter filter) { long count; if (PublishedInfo.class.equals(of)) { - long layers = count(LayerInfo.class, filter); - long groups = count(LayerGroupInfo.class, filter); + Map, Filter> filters = splitOredInstanceOf(filter); + Filter layerFilter = filters.getOrDefault(LayerInfo.class, filter); + Filter lgFilter = filters.getOrDefault(LayerGroupInfo.class, filter); + long layers = count(LayerInfo.class, layerFilter); + long groups = count(LayerGroupInfo.class, lgFilter); count = layers + groups; } else { try { @@ -594,6 +605,42 @@ public int count(final Class of, final Filter filter) return count > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) count; } + Map, Filter> splitOredInstanceOf(Filter filter) { + filter = SimplifyingFilterVisitor.simplify(filter); + Map, Filter> split = new HashMap<>(); + if (filter instanceof Or or) { + for (Filter subFilter : or.getChildren()) { + IsInstanceOf instanceOf = findInstanceOf(subFilter); + if (instanceOf == null) return Map.of(); + List parameters = instanceOf.getParameters(); + if (parameters.isEmpty()) return Map.of(); + Class clazz = parameters.get(0).evaluate(null, Class.class); + split.put(clazz, subFilter); + } + } + return split; + } + + private IsInstanceOf findInstanceOf(Filter subFilter) { + if (subFilter instanceof And and) { + for (Filter f : and.getChildren()) { + var i = findInstanceOf(f); + if (i != null) return i; + } + } + if (subFilter instanceof BinaryComparisonOperator b) { + IsInstanceOf instanceOf = extractInstanceOf(b); + if (instanceOf != null) return instanceOf; + } + return null; + } + + private IsInstanceOf extractInstanceOf(BinaryComparisonOperator f) { + if (f.getExpression1() instanceof IsInstanceOf i) return i; + if (f.getExpression2() instanceof IsInstanceOf i) return i; + return null; + } + /** * This default implementation supports sorting against properties (could be nested) that are * either of a primitive type or implement {@link Comparable}. @@ -625,15 +672,25 @@ private void checkCanSort(final Class type, SortBy or } @Override + @SuppressWarnings("unchecked") public Stream query(Query query) { Stream stream; if (PublishedInfo.class.equals(query.getType())) { - Query lq = new Query<>(LayerInfo.class, query); - Query lgq = new Query<>(LayerGroupInfo.class, query); + final Filter filter = query.getFilter(); + final Map, Filter> filters = splitOredInstanceOf(filter); + final Filter layerFilter = filters.getOrDefault(LayerInfo.class, filter); + final Filter lgFilter = filters.getOrDefault(LayerGroupInfo.class, filter); + + var lq = new Query<>(LayerInfo.class, query).setFilter(layerFilter); + var lgq = new Query<>(LayerGroupInfo.class, query).setFilter(lgFilter); Stream layers = query(lq); Stream groups = query(lgq); - Comparator comparator = CatalogInfoLookup.toComparator(query); - stream = Stream.concat(layers, groups).sorted(comparator).map(query.getType()::cast); + stream = Stream.concat((Stream) layers, (Stream) groups); + if (!query.getSortBy().isEmpty()) { + Comparator comparator = CatalogInfoLookup.toComparator(query); + stream = stream.sorted(comparator); + } + stream = stream.map(query.getType()::cast); } else { try { checkCanSort(query);