Skip to content

Commit

Permalink
Remove special implementation for searching features by tags
Browse files Browse the repository at this point in the history 
- Treat tag-searches like any other property search (that fixes the issue that tag searches were possible without checking for the existence of the necessary index on the DB table)
- Remove special tags handling in SearchForFeatures QR
- Remove obsolete classes TagsQuery & TagList from connector protocol (increases protocol version to 0.8.0)

Signed-off-by: Benjamin Rögner <[email protected]>
  • Loading branch information
@roegi
roegi committed Sep 26, 2024
1 parent a9c4ffb commit 76cd7e4
Show file tree
Hide file tree
Showing 11 changed files with 126 additions and 277 deletions.
50 changes: 26 additions & 24 deletions xyz-hub-service/src/main/java/com/here/xyz/hub/rest/ApiParam.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@
package com.here.xyz.hub.rest;

import static com.here.xyz.events.ContextAwareEvent.SpaceContext.DEFAULT;
import static com.here.xyz.hub.rest.ApiParam.Query.F_PREFIX;

import com.amazonaws.util.StringUtils;
import com.here.xyz.events.ContextAwareEvent;
import com.here.xyz.events.PropertiesQuery;
import com.here.xyz.events.PropertyQuery;
import com.here.xyz.events.PropertyQuery.QueryOperation;
import com.here.xyz.events.PropertyQueryList;
import com.here.xyz.events.TagsQuery;
import com.here.xyz.models.geojson.coordinates.PointCoordinates;
import com.here.xyz.models.geojson.implementation.Point;
import io.vertx.ext.web.RoutingContext;
Expand All @@ -46,6 +46,12 @@
import java.util.stream.Stream;

public class ApiParam {
private static final Map<String, String> SEARCH_KEY_REPLACEMENTS = Map.of(
"f.id", "id",
"f.createdAt", "properties.@ns:com:here:xyz.createdAt",
"f.updatedAt", "properties.@ns:com:here:xyz.updatedAt",
"f.tags", "properties.@ns:com:here:xyz.tags"
);

private static Object getConvertedValue(String rawValue) {
// Boolean
Expand Down Expand Up @@ -82,19 +88,13 @@ private static Object getConvertedValue(String rawValue) {
}

public static String getConvertedKey(String rawKey) {
if (rawKey.startsWith("p.")) {
if (rawKey.startsWith("p."))
return rawKey.replaceFirst("p.", "properties.");
}
Map<String, String> keyReplacements = new HashMap<String, String>() {{
put("f.id", "id");
put("f.createdAt", "properties.@ns:com:here:xyz.createdAt");
put("f.updatedAt", "properties.@ns:com:here:xyz.updatedAt");
}};

String replacement = keyReplacements.get(rawKey);
String replacement = SEARCH_KEY_REPLACEMENTS.get(rawKey);

/** Allow root property search f.foo */
if(replacement == null && rawKey.startsWith("f."))
//Allow root property search by using f.<key>
if (replacement == null && rawKey.startsWith(F_PREFIX))
return rawKey.substring(2);

return replacement;
Expand Down Expand Up @@ -302,13 +302,6 @@ public static boolean getBoolean(RoutingContext context, String param, boolean a
return alt;
}

/**
* Returns the parsed tags parameter
*/
static TagsQuery getTags(RoutingContext context) {
return TagsQuery.fromQueryParameter(queryParam(Query.TAGS, context));
}

public static List<String> getSelection(RoutingContext context) {
if (Query.getString(context, Query.SELECTION, null) == null) {
return null;
Expand All @@ -334,7 +327,7 @@ public static List<String> getSort(RoutingContext context) {

List<String> sort = new ArrayList<>();
for (String s : Query.queryParam(Query.SORT, context))
if (s.startsWith("p.") || s.startsWith("f."))
if (s.startsWith("p.") || s.startsWith(F_PREFIX))
sort.add( s.replaceFirst("^p\\.", "properties.") );

return sort;
Expand Down Expand Up @@ -428,13 +421,13 @@ static PropertyQuery getPropertyQuery(String query, String key, boolean multiVal
}

public static PropertiesQuery parsePropertiesQuery(String query, String property, boolean spaceProperties) {
if (query == null || query.length() == 0) {
if (query == null || query.length() == 0)
return null;
}

PropertyQueryList pql = new PropertyQueryList();
Stream.of(query.split("&"))
.filter(k -> k.startsWith("p.") || k.startsWith("f.") || spaceProperties)
.map(queryParam -> queryParam.startsWith("tags=") ? transformLegacyTags(queryParam) : queryParam)
.filter(queryParam -> queryParam.startsWith("p.") || queryParam.startsWith(F_PREFIX) || spaceProperties)
.forEach(keyValuePair -> {
PropertyQuery propertyQuery = new PropertyQuery();

Expand Down Expand Up @@ -490,12 +483,21 @@ public static PropertiesQuery parsePropertiesQuery(String query, String property
PropertiesQuery pq = new PropertiesQuery();
pq.add(pql);

if (pq.stream().flatMap(List::stream).mapToLong(l -> l.getValues().size()).sum() == 0) {
if (pq.stream().flatMap(List::stream).mapToLong(l -> l.getValues().size()).sum() == 0)
return null;
}

return pq;
}

private static String transformLegacyTags(String legacyTagsQuery) {
String[] tagQueryParts = legacyTagsQuery.split("=");
if (tagQueryParts.length != 2)
return legacyTagsQuery;
String tags = tagQueryParts[1];

return F_PREFIX + "tags" + "=cs=" + tags;
}

static Map<String, Object> getAdditionalParams(RoutingContext context, String type) throws Exception{
Map<String, Object> clusteringParams = context.get(type);

Expand Down
2 changes: 0 additions & 2 deletions xyz-hub-service/src/main/java/com/here/xyz/hub/rest/FeatureApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.here.xyz.events.ContextAwareEvent.SpaceContext;
import com.here.xyz.events.GetFeaturesByIdEvent;
import com.here.xyz.events.ModifyFeaturesEvent;
import com.here.xyz.events.TagsQuery;
import com.here.xyz.hub.rest.ApiParam.Path;
import com.here.xyz.hub.rest.ApiParam.Query;
import com.here.xyz.hub.task.FeatureTask.ConditionalOperation;
Expand Down Expand Up @@ -180,7 +179,6 @@ private void deleteFeature(final RoutingContext context) {
private void deleteFeatures(final RoutingContext context) {
try {
final Set<String> featureIds = new HashSet<>(Query.queryParam(Query.FEATURE_ID, context));
final TagsQuery tags = Query.getTags(context);
final String accept = context.request().getHeader(ACCEPT);
final ApiResponseType responseType = APPLICATION_GEO_JSON.equals(accept) || APPLICATION_JSON.equals(accept)
? ApiResponseType.FEATURE_COLLECTION : ApiResponseType.EMPTY;
Expand Down
6 changes: 1 addition & 5 deletions xyz-hub-service/src/main/java/com/here/xyz/hub/rest/FeatureQueryApi.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ private void searchForFeatures(final RoutingContext context) {
final String author = Query.getString(context, Query.AUTHOR, null);

final SearchForFeaturesEvent event = new SearchForFeaturesEvent();
event.withTags(Query.getTags(context))
.withPropertiesQuery(propertiesQuery)
event.withPropertiesQuery(propertiesQuery)
.withLimit(getLimit(context))
.withRef(getRef(context))
.withForce2D(force2D)
Expand Down Expand Up @@ -220,7 +219,6 @@ public void getFeaturesBySpatial(final RoutingContext context) {
.withRadius(Query.getRadius(context))
.withH3Index(Query.getH3Index(context))
.withLimit(getLimit(context))
.withTags(Query.getTags(context))
.withClip(Query.getBoolean(context, Query.CLIP, false))
.withPropertiesQuery(Query.getPropertiesQuery(context))
.withSelection(Query.getSelection(context))
Expand Down Expand Up @@ -267,7 +265,6 @@ private void getFeaturesByBBox(final RoutingContext context) {
.withClusteringParams(Query.getAdditionalParams(context,Query.CLUSTERING))
.withTweakType(Query.getString(context, Query.TWEAKS, null))
.withTweakParams(Query.getAdditionalParams(context, Query.TWEAKS))
.withTags(Query.getTags(context))
.withPropertiesQuery(Query.getPropertiesQuery(context))
.withLimit(getLimit(context))
.withSelection(Query.getSelection(context))
Expand Down Expand Up @@ -328,7 +325,6 @@ else if( acceptTypeSuffix != null )
.withTweakType(Query.getString(context, Query.TWEAKS, null))
.withTweakParams(Query.getAdditionalParams(context, Query.TWEAKS))
.withLimit(getLimit(context, ("viz".equals(optimMode) ? HARD_LIMIT : DEFAULT_FEATURE_LIMIT)))
.withTags(Query.getTags(context))
.withPropertiesQuery(Query.getPropertiesQuery(context))
.withSelection(Query.getSelection(context))
.withForce2D(force2D)
Expand Down
5 changes: 4 additions & 1 deletion xyz-hub-test/src/test/java/com/here/xyz/hub/rest/ReadFeatureApiIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,18 @@ public void testIterateSpaceWithEtag() {
}

@Test
public void testEmptyParams() {
public void testEmptyParamsForIterate() {
given().
accept(APPLICATION_GEO_JSON).
headers(getAuthHeaders(AuthProfile.ACCESS_OWNER_1_ADMIN)).
when().
get(getSpacesPath() + "/x-psql-test/iterate?limit=5&tags=").
then().
statusCode(OK.code());
}

@Test
public void testEmptyParamsForSearch() {
given().
accept(APPLICATION_GEO_JSON).
headers(getAuthHeaders(AuthProfile.ACCESS_OWNER_1_ADMIN)).
Expand Down
2 changes: 1 addition & 1 deletion xyz-models/src/main/java/com/here/xyz/Payload.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
@JsonIgnoreProperties(ignoreUnknown = true)
public class Payload implements Typed {

public static final String VERSION = "0.7.0";
public static final String VERSION = "0.8.0";

public static InputStream prepareInputStream(InputStream input) throws IOException {
if (!input.markSupported())
Expand Down
20 changes: 0 additions & 20 deletions xyz-models/src/main/java/com/here/xyz/events/SearchForFeaturesEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonTypeName(value = "SearchForFeaturesEvent")
public class SearchForFeaturesEvent<T extends SearchForFeaturesEvent> extends SelectiveEvent<T> {

private static final long DEFAULT_LIMIT = 1_000L;
private static final long MAX_LIMIT = 100_000L;

Expand All @@ -40,34 +39,15 @@ public void setLimit(long limit) {
this.limit = Math.max(1L, Math.min(limit, MAX_LIMIT));
}

public void setFreeLimit(long limit) {
this.limit = limit;
}

@SuppressWarnings("unused")
public T withLimit(long limit) {
setLimit(limit);
//noinspection unchecked
return (T) this;
}

private TagsQuery tags;
private PropertiesQuery propertiesQuery;

public TagsQuery getTags() {
return this.tags;
}

public void setTags(TagsQuery tags) {
this.tags = tags;
}

@SuppressWarnings("unused")
public T withTags(TagsQuery tags) {
setTags(tags);
return (T)this;
}

@SuppressWarnings("unused")
public PropertiesQuery getPropertiesQuery() {
return this.propertiesQuery;
Expand Down
39 changes: 0 additions & 39 deletions xyz-models/src/main/java/com/here/xyz/events/TagList.java
View file

This file was deleted.

77 changes: 0 additions & 77 deletions xyz-models/src/main/java/com/here/xyz/events/TagsQuery.java
View file

This file was deleted.

Loading

0 comments on commit 76cd7e4

Please sign in to comment.