Skip to content

Commit

Permalink
Fixes #50: Handle sorting of documents on multiple attributes (#51)
Browse files Browse the repository at this point in the history
* fix: sorting on multiple attributes

* style: reformat code

* refactor: extract mongo query parsing methods into MongoQueryParser

* refactor: extract postgres query parsing methods into PostgresQueryParser

* test: parseOrderBys in PostgresQueryParser

* test(integration): extend testOffsetAndLimitOrderBy with multiple orderBys
  • Loading branch information
GurtejSohi authored Apr 28, 2021
1 parent 7ccdf64 commit 3f51099
Show file tree
Hide file tree
Showing 8 changed files with 571 additions and 519 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,9 @@ public void testOffsetAndLimitOrderBy(String dataStoreName) throws IOException {
query.setLimit(2);
query.setOffset(1);
query.addOrderBy(new OrderBy("_id", true));
query.addOrderBy(new OrderBy("foo1", true));
query.addOrderBy(new OrderBy("foo2", true));
query.addOrderBy(new OrderBy("foo3", true));

Iterator<Document> results = collection.search(query);
List<Document> documents = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.annotations.VisibleForTesting;
import com.mongodb.BasicDBObject;
import com.mongodb.MongoBulkWriteException;
import com.mongodb.MongoCommandException;
Expand All @@ -26,7 +25,6 @@
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
Expand All @@ -48,7 +46,6 @@
import org.hypertrace.core.documentstore.Filter;
import org.hypertrace.core.documentstore.JSONDocument;
import org.hypertrace.core.documentstore.Key;
import org.hypertrace.core.documentstore.OrderBy;
import org.hypertrace.core.documentstore.Query;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -160,7 +157,7 @@ private BulkWriteResult bulkUpdateImpl(List<BulkUpdateRequest> bulkUpdateRequest
Map<String, Object> conditionMap =
bulkUpdateRequest.getFilter() == null
? new HashMap<>()
: parseQuery(bulkUpdateRequest.getFilter());
: MongoQueryParser.parseFilter(bulkUpdateRequest.getFilter());
conditionMap.put(ID_KEY, key.toString());
BasicDBObject conditionObject = new BasicDBObject(conditionMap);

Expand All @@ -185,7 +182,7 @@ public org.hypertrace.core.documentstore.UpdateResult update(
Key key, Document document, Filter condition) throws IOException {
try {
Map<String, Object> conditionMap =
condition == null ? new HashMap<>() : parseQuery(condition);
condition == null ? new HashMap<>() : MongoQueryParser.parseFilter(condition);
conditionMap.put(ID_KEY, key.toString());
BasicDBObject conditionObject = new BasicDBObject(conditionMap);
UpdateOptions options = new UpdateOptions().upsert(false);
Expand Down Expand Up @@ -331,7 +328,7 @@ public Iterator<Document> search(Query query) {

// If there is a filter in the query, parse it fully.
if (query.getFilter() != null) {
map = parseQuery(query.getFilter());
map = MongoQueryParser.parseFilter(query.getFilter());
}

Bson projection = new BasicDBObject();
Expand Down Expand Up @@ -361,9 +358,8 @@ public Iterator<Document> search(Query query) {
}

if (!query.getOrderBys().isEmpty()) {
Map<String, Object> orderbyMap = new HashMap<>();
parseOrderByQuery(query.getOrderBys(), orderbyMap);
BasicDBObject orderBy = new BasicDBObject(orderbyMap);
BasicDBObject orderBy =
new BasicDBObject(MongoQueryParser.parseOrderBys(query.getOrderBys()));
cursor.sort(orderBy);
}

Expand Down Expand Up @@ -409,91 +405,6 @@ public boolean deleteAll() {
return true;
}

private void parseOrderByQuery(List<OrderBy> orderBys, Map<String, Object> orderbyMap) {
for (OrderBy orderBy : orderBys) {
orderbyMap.put(orderBy.getField(), (orderBy.isAsc() ? 1 : -1));
}
}

@VisibleForTesting
Map<String, Object> parseQuery(Filter filter) {
if (filter.isComposite()) {
Filter.Op op = filter.getOp();
switch (op) {
case OR:
case AND:
{
List<Map<String, Object>> childMapList =
Arrays.stream(filter.getChildFilters())
.map(this::parseQuery)
.filter(map -> !map.isEmpty())
.collect(Collectors.toList());
if (!childMapList.isEmpty()) {
return Map.of("$" + op.name().toLowerCase(), childMapList);
} else {
return Collections.emptyMap();
}
}
default:
throw new UnsupportedOperationException(
String.format("Boolean operation:%s not supported", op));
}
} else {
Filter.Op op = filter.getOp();
Object value = filter.getValue();
Map<String, Object> map = new HashMap<>();
switch (op) {
case EQ:
map.put(filter.getFieldName(), value);
break;
case LIKE:
// Case insensitive regex search
map.put(
filter.getFieldName(), new BasicDBObject("$regex", value).append("$options", "i"));
break;
case NOT_IN:
map.put(filter.getFieldName(), new BasicDBObject("$nin", value));
break;
case IN:
map.put(filter.getFieldName(), new BasicDBObject("$in", value));
break;
case CONTAINS:
map.put(filter.getFieldName(), new BasicDBObject("$elemMatch", filter.getValue()));
break;
case GT:
map.put(filter.getFieldName(), new BasicDBObject("$gt", value));
break;
case LT:
map.put(filter.getFieldName(), new BasicDBObject("$lt", value));
break;
case GTE:
map.put(filter.getFieldName(), new BasicDBObject("$gte", value));
break;
case LTE:
map.put(filter.getFieldName(), new BasicDBObject("$lte", value));
break;
case EXISTS:
map.put(filter.getFieldName(), new BasicDBObject("$exists", true));
break;
case NOT_EXISTS:
map.put(filter.getFieldName(), new BasicDBObject("$exists", false));
break;
case NEQ:
// $ne operator in Mongo also returns the results, where the key does not exist in the
// document. This is as per semantics of EQ vs NEQ. So, if you need documents where
// key exists, consumer needs to add additional filter.
// https://github.com/hypertrace/document-store/pull/20#discussion_r547101520
map.put(filter.getFieldName(), new BasicDBObject("$ne", value));
break;
case AND:
case OR:
default:
throw new UnsupportedOperationException(UNSUPPORTED_QUERY_OPERATION);
}
return map;
}
}

@Override
public long count() {
return collection.countDocuments();
Expand All @@ -505,7 +416,7 @@ public long total(Query query) {

// If there is a filter in the query, parse it fully.
if (query.getFilter() != null) {
map = parseQuery(query.getFilter());
map = MongoQueryParser.parseFilter(query.getFilter());
}

return collection.countDocuments(new BasicDBObject(map));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package org.hypertrace.core.documentstore.mongo;

import static org.hypertrace.core.documentstore.Collection.UNSUPPORTED_QUERY_OPERATION;

import com.mongodb.BasicDBObject;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.hypertrace.core.documentstore.Filter;
import org.hypertrace.core.documentstore.OrderBy;

class MongoQueryParser {

static Map<String, Object> parseFilter(Filter filter) {
if (filter.isComposite()) {
Filter.Op op = filter.getOp();
switch (op) {
case OR:
case AND:
{
List<Map<String, Object>> childMapList =
Arrays.stream(filter.getChildFilters())
.map(MongoQueryParser::parseFilter)
.filter(map -> !map.isEmpty())
.collect(Collectors.toList());
if (!childMapList.isEmpty()) {
return Map.of("$" + op.name().toLowerCase(), childMapList);
} else {
return Collections.emptyMap();
}
}
default:
throw new UnsupportedOperationException(
String.format("Boolean operation:%s not supported", op));
}
} else {
Filter.Op op = filter.getOp();
Object value = filter.getValue();
Map<String, Object> map = new HashMap<>();
switch (op) {
case EQ:
map.put(filter.getFieldName(), value);
break;
case LIKE:
// Case insensitive regex search
map.put(
filter.getFieldName(), new BasicDBObject("$regex", value).append("$options", "i"));
break;
case NOT_IN:
map.put(filter.getFieldName(), new BasicDBObject("$nin", value));
break;
case IN:
map.put(filter.getFieldName(), new BasicDBObject("$in", value));
break;
case CONTAINS:
map.put(filter.getFieldName(), new BasicDBObject("$elemMatch", filter.getValue()));
break;
case GT:
map.put(filter.getFieldName(), new BasicDBObject("$gt", value));
break;
case LT:
map.put(filter.getFieldName(), new BasicDBObject("$lt", value));
break;
case GTE:
map.put(filter.getFieldName(), new BasicDBObject("$gte", value));
break;
case LTE:
map.put(filter.getFieldName(), new BasicDBObject("$lte", value));
break;
case EXISTS:
map.put(filter.getFieldName(), new BasicDBObject("$exists", true));
break;
case NOT_EXISTS:
map.put(filter.getFieldName(), new BasicDBObject("$exists", false));
break;
case NEQ:
// $ne operator in Mongo also returns the results, where the key does not exist in the
// document. This is as per semantics of EQ vs NEQ. So, if you need documents where
// key exists, consumer needs to add additional filter.
// https://github.com/hypertrace/document-store/pull/20#discussion_r547101520
map.put(filter.getFieldName(), new BasicDBObject("$ne", value));
break;
case AND:
case OR:
default:
throw new UnsupportedOperationException(UNSUPPORTED_QUERY_OPERATION);
}
return map;
}
}

static LinkedHashMap<String, Object> parseOrderBys(List<OrderBy> orderBys) {
LinkedHashMap<String, Object> orderByMap = new LinkedHashMap<>();
for (OrderBy orderBy : orderBys) {
orderByMap.put(orderBy.getField(), (orderBy.isAsc() ? 1 : -1));
}
return orderByMap;
}
}
Loading

0 comments on commit 3f51099

Please sign in to comment.