Skip to content

Commit

Permalink
Geogig/gwc integration: Handle null native crs and bounds concurrency
Browse files Browse the repository at this point in the history 
Make MinimalDiffBoundsConsumer thread safe as it may be called
by multiple threads from the PreOrderDiffWalk's ForkJoinPool.

Replace MinimalDiffBoundsConsumer tree name filter by a
PathFilteringDiffConsumer decorator.

Handle the case where the layer has no default CRS specified
by defaulting to the declared CRS.
  • Loading branch information
Gabriel Roldan committed Apr 17, 2017
1 parent 658b80e commit 299bb73
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 56 deletions.
14 changes: 9 additions & 5 deletions src/community/geogig/src/main/java/org/geogig/geoserver/gwc/MinimalDiffBounds.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,20 @@
import org.locationtech.geogig.model.Ref;
import org.locationtech.geogig.model.RevTree;
import org.locationtech.geogig.plumbing.ResolveTreeish;
import org.locationtech.geogig.plumbing.diff.PathFilteringDiffConsumer;
import org.locationtech.geogig.plumbing.diff.PreOrderDiffWalk;
import org.locationtech.geogig.plumbing.diff.PreOrderDiffWalk.Consumer;
import org.locationtech.geogig.repository.AbstractGeoGigOp;
import org.locationtech.geogig.storage.ObjectDatabase;

import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.vividsolutions.jts.geom.Geometry;

/**
* An operation that computes the "approximate minimal bounds" difference between two
* {@link RevTree trees}.
* An operation that computes the "approximate minimal bounds" difference between two {@link RevTree
* trees}.
* <p>
* The "approximate minimal bounds" is defined as the geometry union of the bounds of each
* individual difference, with the exception that when a tree node or bucket tree does not exist at
Expand Down Expand Up @@ -72,12 +75,13 @@ protected Geometry _call() {
ObjectDatabase rightSource = objectDatabase();

PreOrderDiffWalk visitor = new PreOrderDiffWalk(left, right, leftSource, rightSource);
MinimalDiffBoundsConsumer consumer = new MinimalDiffBoundsConsumer();
MinimalDiffBoundsConsumer boundsBuilder = new MinimalDiffBoundsConsumer();
Consumer consumer = boundsBuilder;
if (treeName != null) {
consumer.setTreeNameFilter(treeName);
consumer = new PathFilteringDiffConsumer(ImmutableList.of(treeName), boundsBuilder);
}
visitor.walk(consumer);
Geometry minimalBounds = consumer.buildGeometry();
Geometry minimalBounds = boundsBuilder.buildGeometry();
return minimalBounds;
}

Expand Down
61 changes: 25 additions & 36 deletions src/community/geogig/src/main/java/org/geogig/geoserver/gwc/MinimalDiffBoundsConsumer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
*/
package org.geogig.geoserver.gwc;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Strings.isNullOrEmpty;

import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

import org.eclipse.jdt.annotation.Nullable;
import org.geotools.geometry.jts.JTS;
Expand All @@ -18,8 +17,6 @@
import org.locationtech.geogig.plumbing.diff.PreOrderDiffWalk;
import org.locationtech.geogig.plumbing.diff.PreOrderDiffWalk.BucketIndex;

import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.vividsolutions.jts.geom.Envelope;
import com.vividsolutions.jts.geom.Geometry;
import com.vividsolutions.jts.geom.GeometryFactory;
Expand All @@ -38,25 +35,7 @@ class MinimalDiffBoundsConsumer implements PreOrderDiffWalk.Consumer {
*/
private List<Geometry> nonPoints = new LinkedList<>();

private Predicate<NodeRef> treeNodeFilter = Predicates.alwaysTrue();

private final Envelope envBuff = new Envelope();

public void setTreeNameFilter(final String treeName) {
checkArgument(!isNullOrEmpty(treeName), "treeName can't be null or empty");
this.treeNodeFilter = new Predicate<NodeRef>() {
private final String name = treeName;

/**
* @return true if node name is null, empty, or equal to {@code treeName}
*/
@Override
public boolean apply(NodeRef nodeRef) {
return nodeRef == null || NodeRef.ROOT.equals(nodeRef.getNode().getName())
|| name.equals(nodeRef.getNode().getName());
}
};
}
private final Lock lock = new ReentrantLock();

/**
* @return a single geometry product of unioning all the bounding boxes acquired while
Expand Down Expand Up @@ -85,9 +64,6 @@ public boolean feature(@Nullable final NodeRef left, @Nullable final NodeRef rig

@Override
public boolean tree(@Nullable final NodeRef left, @Nullable final NodeRef right) {
if (!treeNodeFilter.apply(left) || !treeNodeFilter.apply(right)) {
return false;
}
if (left == null) {
addEnv(right);
return false;
Expand All @@ -100,7 +76,8 @@ public boolean tree(@Nullable final NodeRef left, @Nullable final NodeRef right)

@Override
public boolean bucket(final NodeRef leftParent, final NodeRef rightParent,
final BucketIndex bucketIndex, @Nullable final Bucket left, @Nullable final Bucket right) {
final BucketIndex bucketIndex, @Nullable final Bucket left,
@Nullable final Bucket right) {
if (left == null) {
addEnv(right);
return false;
Expand All @@ -115,15 +92,21 @@ private void addEnv(@Nullable Bounded node) {
if (node == null) {
return;
}
Envelope env = envBuff;
env.setToNull();
node.expand(env);
if (env.isNull()) {
final Envelope env = node.bounds().orNull();
if (env == null || env.isNull()) {
return;
}
if (isPoint(env)) {
points.add(env.getMinX(), env.getMinY());
} else if (isOrthoLine(env)) {
lock.lock();
try {
points.add(env.getMinX(), env.getMinY());
} finally {
lock.unlock();
}
return;
}
Geometry geom;
if (isOrthoLine(env)) {
// handle the case where the envelope is given by an orthogonal line so we don't add a
// zero area polygon
double width = env.getWidth();
Expand All @@ -135,9 +118,15 @@ private void addEnv(@Nullable Bounded node) {
cs.add(env.getMinX(), env.getMinY());
cs.add(env.getMaxX(), env.getMinY());
}
nonPoints.add(GEOM_FACTORY.createLineString(cs));
geom = GEOM_FACTORY.createLineString(cs);
} else {
nonPoints.add(JTS.toGeometry(env, GEOM_FACTORY));
geom = JTS.toGeometry(env, GEOM_FACTORY);
}
lock.lock();
try {
nonPoints.add(geom);
} finally {
lock.unlock();
}
}

Expand Down
36 changes: 21 additions & 15 deletions src/community/geogig/src/main/java/org/geogig/geoserver/gwc/TruncateHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,35 +72,42 @@ public static void issueTruncateTasks(Context context, Optional<Ref> oldRef,
minimalBounds = geomBuildCommand.call();
sw.stop();
if (minimalBounds.isEmpty()) {
LOGGER.debug(String.format(
"Feature tree '%s' not affected by change %s...%s (took %s)",
layerTreeName, oldCommit, newCommit, sw));
LOGGER.debug(
String.format("Feature tree '%s' not affected by change %s...%s (took %s)",
layerTreeName, oldCommit, newCommit, sw));
return;
}
if (LOGGER.isDebugEnabled()) {
LOGGER.debug(String.format(
"Minimal bounds on layer '%s' computed in %s: %s", tileLayerName, sw,
formattedWKT(minimalBounds)));
LOGGER.debug(String.format("Minimal bounds on layer '%s' computed in %s: %s",
tileLayerName, sw, formattedWKT(minimalBounds)));
}
} catch (Exception e) {
sw.stop();
LOGGER.error(String.format(
"Error computing minimal bounds for %s...%s on layer '%s' after %s",
oldCommit, newCommit, tileLayerName, sw));
"Error computing minimal bounds for %s...%s on layer '%s' after %s", oldCommit,
newCommit, tileLayerName, sw));
throw Throwables.propagate(e);
}
final Set<String> gridSubsets = tileLayer.getGridSubsets();

LayerInfo layerInfo = tileLayer.getLayerInfo();
ResourceInfo resource = layerInfo.getResource();
final CoordinateReferenceSystem nativeCrs = resource.getNativeCRS();

final CoordinateReferenceSystem sourceCrs;
{
CoordinateReferenceSystem nativeCrs = resource.getNativeCRS();
if (nativeCrs == null) {
// no native CRS specified, layer must have been configured with an overriding one
sourceCrs = resource.getCRS();
} else {
sourceCrs = nativeCrs;
}
}
for (String gridsetId : gridSubsets) {
GridSubset gridSubset = tileLayer.getGridSubset(gridsetId);
final CoordinateReferenceSystem gridSetCrs = getGridsetCrs(gridSubset);

LOGGER.debug("Reprojecting geometry mask to gridset {}", gridsetId);
Geometry geomInGridsetCrs = transformToGridsetCrs(minimalBounds, nativeCrs, gridSetCrs);
Geometry geomInGridsetCrs = transformToGridsetCrs(minimalBounds, sourceCrs, gridSetCrs);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("geometry mask reprojected to gridset {}: {}", gridsetId,
formattedWKT(geomInGridsetCrs));
Expand Down Expand Up @@ -148,7 +155,7 @@ private static Geometry bufferAndSimplifyBySizeOfSmallerTile(Geometry geomInGrid
Geometry geometry = bufferOp.getResultGeometry(bufferRatio);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug(String.format(
"Geometry buffered by the size of a tile at zoom level %s (%s units): %s",
"Geometry buffered by the size of a tile at zoom level %s (%s units): %s",
zoomStop, bufferRatio, formattedWKT(geometry)));
}
TopologyPreservingSimplifier simplifier = new TopologyPreservingSimplifier(geometry);
Expand Down Expand Up @@ -217,9 +224,8 @@ private static void truncate(TileBreeder breeder, GeoServerTileLayer tileLayer,
rasterMask, mimeType, parameters);

if (LOGGER.isDebugEnabled()) {
LOGGER.debug(String.format(
"Truncating layer %s#%s#%s with geom mask %s", layerName, gridSetId,
mimeType.getFormat(), formattedWKT(geomInGridsetCrs)));
LOGGER.debug(String.format("Truncating layer %s#%s#%s with geom mask %s", layerName,
gridSetId, mimeType.getFormat(), formattedWKT(geomInGridsetCrs)));
}
try {
GWCTask[] tasks = breeder.createTasks(tileRange, TYPE.TRUNCATE, 1, false);
Expand Down

0 comments on commit 299bb73

Please sign in to comment.