From 502fc2132ebf49a4b06e7b7b4d94e06c74fd42b7 Mon Sep 17 00:00:00 2001 From: Jody Garnett Date: Fri, 16 Feb 2024 14:08:06 -0800 Subject: [PATCH] [GEOS-11306] Java 17 does not support GetFeature lazy JDBC count(*) --- .../java/org/geoserver/wfs/GetFeature.java | 66 ++++++------ .../request/FeatureCollectionResponse.java | 100 +++++++++++++++--- 2 files changed, 114 insertions(+), 52 deletions(-) diff --git a/src/wfs/src/main/java/org/geoserver/wfs/GetFeature.java b/src/wfs/src/main/java/org/geoserver/wfs/GetFeature.java index 675ed8f4bb9..1d34cd8fa33 100644 --- a/src/wfs/src/main/java/org/geoserver/wfs/GetFeature.java +++ b/src/wfs/src/main/java/org/geoserver/wfs/GetFeature.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -26,7 +27,6 @@ import net.opengis.wfs20.QueryType; import net.opengis.wfs20.ResultTypeType; import net.opengis.wfs20.StoredQueryType; -import org.apache.commons.lang3.SystemUtils; import org.geoserver.catalog.AttributeTypeInfo; import org.geoserver.catalog.Catalog; import org.geoserver.catalog.FeatureTypeInfo; @@ -120,8 +120,6 @@ import org.geotools.util.factory.Hints; import org.geotools.xsd.Encoder; import org.locationtech.jts.geom.Polygon; -import org.springframework.cglib.proxy.Enhancer; -import org.springframework.cglib.proxy.LazyLoader; import org.xml.sax.helpers.NamespaceSupport; /** @@ -305,7 +303,7 @@ public FeatureCollectionResponse run(GetFeatureRequest request) throws WFSExcept boolean isNumberMatchedSkipped = false; int count = 0; // should probably be long - BigInteger totalCount = BigInteger.ZERO; + Supplier totalCount = () -> BigInteger.ZERO; // offset into result set in which to return features int totalOffset = request.getStartIndex() != null ? request.getStartIndex().intValue() : -1; @@ -682,7 +680,20 @@ private void validateQueryAliases(GetFeatureRequest request, Query query) { } } - private BigInteger updateTotalCount( + /** + * Total count represents the total count of the features matched for this query in cases where + * the client has limited the result set size, so we compute it lazily. + * + * @param maxFeatures + * @param isNumberMatchedSkipped + * @param count + * @param totalOffset + * @param calculateSize + * @param totalCountExecutors + * @return Lazy calculation of total count, or {@code null} if isNumberMatchedSkipped + * @throws IOException + */ + private Supplier updateTotalCount( int maxFeatures, boolean isNumberMatchedSkipped, int count, @@ -690,42 +701,27 @@ private BigInteger updateTotalCount( boolean calculateSize, List totalCountExecutors) throws IOException { - BigInteger totalCount; - // total count represents the total count of the features matched for this query in - // cases/ where the client has limited the result set size, so we compute it lazily if (isNumberMatchedSkipped) { - totalCount = BigInteger.valueOf(-1); + return () -> BigInteger.valueOf(-1); } else if (count < maxFeatures && calculateSize && totalOffset == 0) { // optimization: if count < max features then total count == count // can't use this optimization for v2 - totalCount = BigInteger.valueOf(count); + return () -> BigInteger.valueOf(count); } else if (isPreComputed(totalCountExecutors)) { long total = getTotalCount(totalCountExecutors); - totalCount = BigInteger.valueOf(total); + return () -> BigInteger.valueOf(total); } else { - if (SystemUtils.IS_JAVA_11 || SystemUtils.IS_JAVA_1_8) { - // ok, in this case we're forced to run the queries to discover the actual total - // count. We do so lazily, not all output formats need it, leveraging the fact that - // BigInteger is not final to wrap it in a lazy loading proxy - Enhancer enhancer = new Enhancer(); - enhancer.setSuperclass(BigInteger.class); - enhancer.setCallback( - (LazyLoader) - () -> { - long totalCount1 = getTotalCount(totalCountExecutors); - return BigInteger.valueOf(totalCount1); - }); - totalCount = - (BigInteger) - enhancer.create(new Class[] {String.class}, new Object[] {"0"}); - } else { - // Spring asm does not work with newer versions of Java, skip lazy loading for now - // we might want to revisit with a different approach later, e..g, when upgrading - // Spring and its internal ASM library - totalCount = BigInteger.valueOf(getTotalCount(totalCountExecutors)); - } + return () -> { + try { + long totalCount1 = getTotalCount(totalCountExecutors); + return BigInteger.valueOf(totalCount1); + } catch (IOException ioException) { + throw new RuntimeException( + "Lazy total count unavailable " + ioException.getMessage(), + ioException); + } + }; } - return totalCount; } private void collectPropertyNames( @@ -1020,14 +1016,14 @@ protected FeatureCollectionResponse buildResults( int offset, int maxFeatures, int count, - BigInteger total, + Supplier total, List> results, String lockId, boolean getFeatureById) { FeatureCollectionResponse result = request.createResponse(); result.setNumberOfFeatures(BigInteger.valueOf(count)); - result.setTotalNumberOfFeatures(total); + result.setLazyTotalNumberOfFeatures(total); result.setTimeStamp(Calendar.getInstance()); result.setLockId(lockId); result.getFeature().addAll(results); diff --git a/src/wfs/src/main/java/org/geoserver/wfs/request/FeatureCollectionResponse.java b/src/wfs/src/main/java/org/geoserver/wfs/request/FeatureCollectionResponse.java index 64dc910a479..434e06624f0 100644 --- a/src/wfs/src/main/java/org/geoserver/wfs/request/FeatureCollectionResponse.java +++ b/src/wfs/src/main/java/org/geoserver/wfs/request/FeatureCollectionResponse.java @@ -8,6 +8,7 @@ import java.math.BigInteger; import java.util.Calendar; import java.util.List; +import java.util.function.Supplier; import net.opengis.wfs.FeatureCollectionType; import net.opengis.wfs.WfsFactory; import net.opengis.wfs20.Wfs20Factory; @@ -22,7 +23,14 @@ */ public abstract class FeatureCollectionResponse extends RequestObject { - private boolean getFeatureById = false; + protected boolean getFeatureById = false; + + /** + * It can be expensive to determine the total number of features up front, by using a supplier + * we can defer calculation until the end of the request (when the value may already have been + * established). + */ + protected Supplier lazyTotalNumberOfFeatures = null; public static FeatureCollectionResponse adapt(Object adaptee) { if (adaptee instanceof FeatureCollectionType) { @@ -53,15 +61,66 @@ public void setTimeStamp(Calendar timeStamp) { eSet(adaptee, "timeStamp", timeStamp); } + /** + * Factory method creating a new FeatureCollectionResponse. + * + * @return feature collection response + */ public abstract FeatureCollectionResponse create(); + /** + * Number of features included in this response. Number reflect the number of features included, + * which may be less than {@link #getTotalNumberOfFeatures()} when paging through more content + * than can be obtained in a single request. + * + * @return number of features included in this response. + */ public abstract BigInteger getNumberOfFeatures(); + /** + * Number of features included in response. + * + * @param n Number of features included in response + */ public abstract void setNumberOfFeatures(BigInteger n); - public abstract BigInteger getTotalNumberOfFeatures(); + /** + * Used to calculate total number of features on demand (only if needed). This allows formats + * that do not need the total to avoid calculating this expensive result, it also may be that + * some data stores can better estimate this total is obtained after traversing results. + * + * @param totalNumberOfFeatures Delayed calculation of total number of featuers. + */ + public void setLazyTotalNumberOfFeatures(Supplier totalNumberOfFeatures) { + this.lazyTotalNumberOfFeatures = totalNumberOfFeatures; + } - public abstract void setTotalNumberOfFeatures(BigInteger n); + /** + * Total number of features hits matched, or {@code null} for "unknown". Total is used as a + * guide when paging through more content than can be obtained in a single request. + * + *

This value is set by calling {@link #setLazyTotalNumberOfFeatures(Supplier)} (deferred + * value), or {@link #setTotalNumberOfFeatures(BigInteger)}. + * + * @return total number of features available, or null for "unknown". + */ + public BigInteger getTotalNumberOfFeatures() { + if (lazyTotalNumberOfFeatures != null) { + return lazyTotalNumberOfFeatures.get(); + } else { + return null; + } + } + + /** + * Total number of Features hits matched, which may be greater than the number included in an + * individual result. + * + * @param totalHits total number of feature hits matched, or {@code null} for "unknown". + */ + public void setTotalNumberOfFeatures(final BigInteger totalHits) { + lazyTotalNumberOfFeatures = () -> totalHits; + } public abstract void setPrevious(String previous); @@ -90,8 +149,8 @@ public boolean isGetFeatureById() { return getFeatureById; } + /** FeatureCollection response adapted from {@link net.opengis.wfs20.FeatureCollectionType}. */ public static class WFS11 extends FeatureCollectionResponse { - BigInteger totalNumberOfFeatures; public WFS11(EObject adaptee) { super(adaptee); @@ -113,16 +172,6 @@ public void setNumberOfFeatures(BigInteger n) { eSet(adaptee, "numberOfFeatures", n); } - @Override - public BigInteger getTotalNumberOfFeatures() { - return totalNumberOfFeatures; - } - - @Override - public void setTotalNumberOfFeatures(BigInteger n) { - this.totalNumberOfFeatures = n; - } - @Override public String getPrevious() { // noop @@ -177,6 +226,7 @@ public Object unadapt(Class target) { } } + /** FeatureCollectionResponse from {@link net.opengis.wfs20.FeatureCollectionType}. */ public static class WFS20 extends FeatureCollectionResponse { public WFS20(EObject adaptee) { super(adaptee); @@ -200,14 +250,29 @@ public void setNumberOfFeatures(BigInteger n) { @Override public BigInteger getTotalNumberOfFeatures() { - BigInteger result = eGet(adaptee, "numberMatched", BigInteger.class); - if (result != null && result.signum() < 0) return null; - return result; + if (lazyTotalNumberOfFeatures != null) { + return lazyTotalNumberOfFeatures.get(); + } else { + BigInteger result = eGet(adaptee, "numberMatched", BigInteger.class); + if (result != null && result.signum() < 0) { + // indicates "unknown" + return null; + } + return result; + } } @Override public void setTotalNumberOfFeatures(BigInteger n) { eSet(adaptee, "numberMatched", n); + this.lazyTotalNumberOfFeatures = + () -> { + BigInteger result = eGet(adaptee, "numberMatched", BigInteger.class); + if (result != null && result.signum() < 0) { + return null; // indicates "unknown" + } + return result; + }; } @Override @@ -245,6 +310,7 @@ public void setFeatures(List features) { @SuppressWarnings("unchecked") // EMF model without generics public Object unadapt(Class target) { if (target.equals(net.opengis.wfs20.FeatureCollectionType.class)) { + eSet(adaptee, "numberMatched", getTotalNumberOfFeatures()); return adaptee; } else if (target.equals(FeatureCollectionType.class)) { net.opengis.wfs20.FeatureCollectionType source =