Skip to content

Commit

Permalink
Merge pull request #476 from groldan/fix/repo_catalogfacade_query_pub…
Browse files Browse the repository at this point in the history 
…lishedinfos

Fix RepositoryCatalogFacadeImpl's query of PublishedInfo.class
  • Loading branch information
@groldan
groldan authored May 23, 2024
2 parents ea8a373 + 1f86862 commit 6109c70
Show file tree
Hide file tree
Showing 3 changed files with 197 additions and 31 deletions.
138 changes: 110 additions & 28 deletions ...atalog/plugin/src/main/java/org/geoserver/catalog/plugin/RepositoryCatalogFacadeImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
*/
package org.geoserver.catalog.plugin;

import com.google.common.collect.Iterators;
import com.google.common.collect.Streams;

import org.geoserver.catalog.Catalog;
import org.geoserver.catalog.CatalogCapabilities;
import org.geoserver.catalog.CatalogException;
Expand All @@ -15,6 +18,7 @@
import org.geoserver.catalog.LockingCatalogFacade;
import org.geoserver.catalog.MapInfo;
import org.geoserver.catalog.NamespaceInfo;
import org.geoserver.catalog.Predicates;
import org.geoserver.catalog.PublishedInfo;
import org.geoserver.catalog.ResourceInfo;
import org.geoserver.catalog.StoreInfo;
Expand Down Expand Up @@ -42,6 +46,7 @@
import java.lang.reflect.Proxy;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -322,7 +327,7 @@ public List<LayerGroupInfo> getLayerGroups() {

@Override
public List<LayerGroupInfo> getLayerGroupsByWorkspace(WorkspaceInfo workspace) {
// Question: do we need to support ANY_WORKSPACE? see "todo" comment in DefaultCatalogFacade
// Question: do we need to support ANY_WORKSPACE? see comment in DefaultCatalogFacade

WorkspaceInfo ws;
if (workspace == null) {
Expand Down Expand Up @@ -581,8 +586,9 @@ private <T extends CatalogInfo> void sync(Supplier<Stream<T>> from, Consumer<T>
}

@Override
public <T extends CatalogInfo> int count(final Class<T> of, final Filter filter) {
public <T extends CatalogInfo> int count(final Class<T> of, Filter filter) {
long count;
filter = SimplifyingFilterVisitor.simplify(filter);
if (PublishedInfo.class.equals(of)) {
Map<Class<?>, Filter> filters = splitOredInstanceOf(filter);
Filter layerFilter = filters.getOrDefault(LayerInfo.class, filter);
Expand All @@ -594,6 +600,7 @@ public <T extends CatalogInfo> int count(final Class<T> of, final Filter filter)
try {
count = repository(of).count(of, filter);
} catch (RuntimeException e) {
e.printStackTrace();
throw new CatalogException(
"Error obtaining count of %s with filter %s"
.formatted(of.getSimpleName(), filter),
Expand All @@ -604,7 +611,6 @@ public <T extends CatalogInfo> int count(final Class<T> of, final Filter filter)
}

Map<Class<?>, Filter> splitOredInstanceOf(Filter filter) {
filter = SimplifyingFilterVisitor.simplify(filter);
Map<Class<?>, Filter> split = new HashMap<>();
if (filter instanceof Or or) {
for (Filter subFilter : or.getChildren()) {
Expand Down Expand Up @@ -669,34 +675,110 @@ private <T extends CatalogInfo> void checkCanSort(final Class<T> type, SortBy or
}

@Override
@SuppressWarnings("unchecked")
public <T extends CatalogInfo> Stream<T> query(Query<T> query) {
Stream<T> stream;
if (PublishedInfo.class.equals(query.getType())) {
final Filter filter = query.getFilter();
final Map<Class<?>, 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<LayerInfo> layers = query(lq);
Stream<LayerGroupInfo> groups = query(lgq);
stream = Stream.concat((Stream<T>) layers, (Stream<T>) groups);
if (!query.getSortBy().isEmpty()) {
Comparator<CatalogInfo> comparator = CatalogInfoLookup.toComparator(query);
stream = stream.sorted(comparator);
}
stream = stream.map(query.getType()::cast);
} else {
try {
checkCanSort(query);
stream = repository(query.getType()).findAll(query);
} catch (RuntimeException e) {
throw new CatalogException("Error obtaining stream: %s".formatted(query), e);
if (Filter.EXCLUDE.equals(query.getFilter())) {
return Stream.empty();
}
final Class<T> type = query.getType();
if (PublishedInfo.class.equals(type)) {
return queryPublishedInfo(query).map(query.getType()::cast);
}
try {
checkCanSort(query);
return repository(query.getType()).findAll(query);
} catch (RuntimeException e) {
throw new CatalogException("Error obtaining stream: %s".formatted(query), e);
}
}

/**
* Queries {@link LayerRepository} and {@link LayerGroupRepository}, and returns a merge-sorted
* stream.
*
* <p>If the query does not specify a sort order, the {@code id} property is used to provide
* predictable order for the merge-sort algorithm
*
* <p>When the returned stream is closed, it'll close the two underlying streams
*/
protected Stream<PublishedInfo> queryPublishedInfo(Query<?> query) {
final Filter filter = SimplifyingFilterVisitor.simplify(query.getFilter());
final Map<Class<?>, Filter> filters = splitOredInstanceOf(filter);
final Filter layerFilter = filters.getOrDefault(LayerInfo.class, filter);
final Filter lgFilter = filters.getOrDefault(LayerGroupInfo.class, filter);

var layerQuery = new Query<>(LayerInfo.class, query).setFilter(layerFilter);
var lgQuery = new Query<>(LayerGroupInfo.class, query).setFilter(lgFilter);

if (query.getSortBy().isEmpty()) {
// enforce predictable order
List<SortBy> sortBy = List.of(Predicates.sortBy("id", true));
layerQuery.setSortBy(sortBy);
lgQuery.setSortBy(sortBy);
}
final Integer offset = query.getOffset();
final Integer limit = query.getCount();
final boolean applyOffsetLimit = shallApplyOffsetLimit(offset, limit, layerQuery, lgQuery);

Stream<LayerInfo> layers = Stream.empty();
Stream<LayerGroupInfo> groups = Stream.empty();
try {
layers = query(layerQuery);
groups = query(lgQuery);

// merge-sort the two streams without additional memory allocation, they're guaranteed
// to be sorted

Iterator<PublishedInfo> layersit = layers.map(PublishedInfo.class::cast).iterator();
Iterator<PublishedInfo> lgit = groups.map(PublishedInfo.class::cast).iterator();
Comparator<PublishedInfo> comparator = CatalogInfoLookup.toComparator(query);

var stream = Streams.stream(Iterators.mergeSorted(List.of(layersit, lgit), comparator));
if (applyOffsetLimit) {
if (offset != null) stream = stream.skip(offset);
if (limit != null) stream = stream.limit(limit);
}
stream = closing(stream, layers, groups);
return stream;
} catch (RuntimeException e) {
layers.close();
groups.close();
throw e;
}
}

protected boolean shallApplyOffsetLimit(
final Integer offset,
final Integer limit,
Query<LayerInfo> layerQuery,
Query<LayerGroupInfo> lgQuery) {
if (null == offset && null == limit) {
return false;
}
// bad luck, but try to discard the one(s) that don't match any first for a chance to
// avoid in-process offset/limit filtering
int lgCount = count(LayerGroupInfo.class, lgQuery.getFilter());
if (0 == lgCount) {
lgQuery.setFilter(Filter.EXCLUDE);
return false;
}
int lcount = count(LayerInfo.class, layerQuery.getFilter());
if (0 == lcount) {
layerQuery.setFilter(Filter.EXCLUDE);
return false;
}
return stream;
// neither is zero, we gotta do in-process offset/limit to preserve the sort order
layerQuery.setOffset(null);
layerQuery.setCount(null);
lgQuery.setOffset(null);
lgQuery.setCount(null);
return true;
}

private <T> Stream<T> closing(Stream<T> stream, Stream<?>... closeables) {
return stream.onClose(
() -> {
for (var s : closeables) s.close();
});
}

@Override
Expand Down
11 changes: 8 additions & 3 deletions src/catalog/plugin/src/test/java/org/geoserver/catalog/CatalogTestData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.geotools.coverage.grid.io.OverviewPolicy;
import org.geotools.process.factory.AnnotationDrivenProcessFactory;
import org.geotools.util.Converters;
import org.springframework.lang.Nullable;

import java.io.Serializable;
import java.util.HashMap;
Expand Down Expand Up @@ -356,6 +357,12 @@ public CatalogTestData createConfigObjects() {
return this;
}

public LayerGroupInfo createLayerGroup(
String name, @Nullable WorkspaceInfo ws, PublishedInfo layer) {
StyleInfo style = layer instanceof LayerInfo l ? l.getDefaultStyle() : null;
return createLayerGroup(null, ws, name, layer, style);
}

public LayerGroupInfo createLayerGroup(
String id, WorkspaceInfo workspace, String name, PublishedInfo layer, StyleInfo style) {
// not using factory cause SecuredCatalog would return SecuredLayerGroupInfo which has no id
Expand All @@ -371,10 +378,8 @@ public LayerGroupInfo createLayerGroup(
}

public LayerInfo createLayer(ResourceInfo resource, StyleInfo defaultStyle) {

String id = resource.getName() + "-layer-id";
String title = resource.getName() + " title";
return createLayer(id, resource, title, true, defaultStyle);
return createLayer(null, resource, title, true, defaultStyle);
}

public LayerInfo createLayer(
Expand Down
79 changes: 79 additions & 0 deletions src/catalog/plugin/src/test/java/org/geoserver/catalog/plugin/CatalogConformanceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,13 +293,21 @@ protected LayerInfo addLayer() {
LayerInfo layer = data.layerFeatureTypeA;
layer.setResource(addFeatureType());
layer.setDefaultStyle(addStyle());
return addLayer(layer);
}

private LayerInfo addLayer(LayerInfo layer) {
return add(layer, catalog::add, catalog::getLayer);
}

protected LayerGroupInfo addLayerGroup() {
LayerGroupInfo lg = data.layerGroup1;
lg.getLayers().clear();
lg.getLayers().add(addLayer());
return addLayerGroup(lg);
}

private LayerGroupInfo addLayerGroup(LayerGroupInfo lg) {
return add(lg, catalog::add, catalog::getLayerGroup);
}

Expand Down Expand Up @@ -3723,4 +3731,75 @@ void testCountIdFilter() {
assertEquals(
0, catalog.count(CoverageStoreInfo.class, equal("id", data.dataStoreB.getId())));
}

@Test
void testListPublishedInfos() {
addNamespace();
addDataStore();
addStyle();

PublishedInfo l1 = addNewLayer("A-l");
PublishedInfo g1 = addNewLayerGroup("B-lg", l1);
PublishedInfo l2 = addNewLayer("C-l");
PublishedInfo g2 = addNewLayerGroup("D-lg", l2);
PublishedInfo l3 = addNewLayer("E-l");
PublishedInfo g3 = addNewLayerGroup("F-lg", l3);
PublishedInfo l4 = addNewLayer("G-l");
PublishedInfo g4 = addNewLayerGroup("H-lg", l4);
PublishedInfo l5 = addNewLayer("I-l");
PublishedInfo g5 = addNewLayerGroup("J-lg", l5);
assertEquals(5, catalog.getLayers().size());
assertEquals(5, catalog.getLayerGroups().size());

testListPublishedInfos(
acceptAll(), null, null, List.of(l1, g1, l2, g2, l3, g3, l4, g4, l5, g5));
testListPublishedInfos(acceptAll(), 3, 4, List.of(g2, l3, g3, l4));
testListPublishedInfos(acceptAll(), 8, 4, List.of(l5, g5));

Filter filter = Predicates.isInstanceOf(LayerInfo.class);
testListPublishedInfos(filter, null, null, List.of(l1, l2, l3, l4, l5));
testListPublishedInfos(filter, 0, 2, List.of(l1, l2));
testListPublishedInfos(filter, 3, 5, List.of(l4, l5));

filter = Predicates.isInstanceOf(LayerGroupInfo.class);
testListPublishedInfos(filter, null, null, List.of(g1, g2, g3, g4, g5));
testListPublishedInfos(filter, 0, 2, List.of(g1, g2));
testListPublishedInfos(filter, 3, 10, List.of(g4, g5));
}

// sorts by name
private void testListPublishedInfos(
Filter filter, Integer offset, Integer limit, List<PublishedInfo> expected) {

List<PublishedInfo> result = listPublishedInfos(filter, offset, limit);
assertEquals(expected.size(), result.size());
List<String> expectedNames = expected.stream().map(PublishedInfo::getName).toList();
List<String> actualNames = result.stream().map(PublishedInfo::getName).toList();
assertEquals(expectedNames, actualNames);

if (null == offset && null == limit) {
assertEquals(expected.size(), catalog.count(PublishedInfo.class, filter));
}
}

private List<PublishedInfo> listPublishedInfos(Filter filter, Integer offset, Integer limit) {
List<PublishedInfo> result;
SortBy nameOrder = Predicates.sortBy("name", true);
try (var it = catalog.list(PublishedInfo.class, filter, offset, limit, nameOrder)) {
result = Lists.newArrayList(it);
}
return result;
}

LayerInfo addNewLayer(String name) {
FeatureTypeInfo ft = data.createFeatureType(name);
ft = add(ft, catalog::add, catalog::getFeatureType);
LayerInfo layer = data.createLayer(ft, data.style1);
return addLayer(layer);
}

LayerGroupInfo addNewLayerGroup(String name, PublishedInfo layer) {
LayerGroupInfo lg = data.createLayerGroup(name, null, layer);
return addLayerGroup(lg);
}
}

0 comments on commit 6109c70

Please sign in to comment.