Skip to content

Commit

Permalink
Merge pull request #536 from bserdar/535
Browse files Browse the repository at this point in the history 
Fix #535: Set matchCount correctly
  • Loading branch information
@dcrissman
dcrissman committed Dec 7, 2015
2 parents e05fd77 + 4acd4e2 commit 6418ac5
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 21 deletions.
21 changes: 17 additions & 4 deletions crud/src/main/java/com/redhat/lightblue/mediator/CompositeFindImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ public CRUDFindResponse find(OperationContext ctx,
QueryPlan searchQPlan=null;
QueryPlanNode searchQPlanRoot=null;
SortAndLimit sortAndLimit=new SortAndLimit(root,null,null,null);
// Need to keep the matchCount, because the root node evaluator will only return the documents in the range
CRUDFindResponse rootResponse=null;

if(minimalTree.size()>1) {
// The query depends on several entities. so, we query first, and then retrieve
QueryPlanChooser qpChooser=new QueryPlanChooser(root,
Expand All @@ -181,14 +184,19 @@ public CRUDFindResponse find(OperationContext ctx,
QueryPlanNodeExecutor exec=node.getProperty(QueryPlanNodeExecutor.class);
if(node.getMetadata().getParent()==null) {
searchQPlanRoot=node;
if(node.getSources().length>0) {
boolean hasSources=node.getSources().length>0;
if(hasSources) {
sortAndLimit=new SortAndLimit(root,req.getSort(),req.getFrom(),req.getTo());
} else {
if (req.getTo() != null && req.getFrom() != null) {
exec.setRange(req.getFrom(), req.getTo());
}
}
exec.execute(ctx,req.getSort());
if(hasSources) {
exec.execute(ctx,req.getSort());
} else {
rootResponse=exec.execute(ctx, req.getSort());
}
} else {
exec.execute(ctx,null);
}
Expand All @@ -214,6 +222,7 @@ public CRUDFindResponse find(OperationContext ctx,
// Now execute rest of the retrieval plan
QueryPlanNode[] nodeOrdering=retrievalQPlan.getBreadthFirstNodeOrdering();


List<ResultDoc> rootDocs=null;
for(int i=0;i<nodeOrdering.length;i++) {
if(nodeOrdering[i].getMetadata().getParent()==null) {
Expand All @@ -233,6 +242,10 @@ public CRUDFindResponse find(OperationContext ctx,
}
LOGGER.debug("Retrieving {} docs",filteredDocs.size());
nodeOrdering[i].getProperty(QueryPlanNodeExecutor.class).setDocs(filteredDocs);
if(rootResponse==null) {
rootResponse=new CRUDFindResponse();
rootResponse.setSize(filteredDocs.size());
}
rootDocs=sortAndLimit.process(filteredDocs);
} else {
LOGGER.debug("Performing search for retrieval");
Expand All @@ -241,7 +254,7 @@ public CRUDFindResponse find(OperationContext ctx,
if (req.getTo() != null && req.getFrom() != null) {
exec.setRange(req.getFrom(), req.getTo());
}
exec.execute(ctx,req.getSort());
rootResponse=exec.execute(ctx,req.getSort());
rootDocs=exec.getDocs();
}
} else {
Expand All @@ -262,7 +275,7 @@ public CRUDFindResponse find(OperationContext ctx,

CRUDFindResponse response=new CRUDFindResponse();

response.setSize(resultDocuments.size());
response.setSize(rootResponse.getSize());
ctx.setDocuments(projectResults(ctx,resultDocuments,req.getProjection()));
ctx.addErrors(errors);

Expand Down
27 changes: 13 additions & 14 deletions crud/src/main/java/com/redhat/lightblue/mediator/QueryPlanNodeExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ private boolean isParentOfThis(QueryPlanNode otherNode) {
}


public void execute(OperationContext ctx,
Sort sort) {
public CRUDFindResponse execute(OperationContext ctx,
Sort sort) {
LOGGER.debug("execute {}: start",node.getName());

CRUDFindRequest findRequest=new CRUDFindRequest();
Expand All @@ -169,17 +169,14 @@ public void execute(OperationContext ctx,
findRequest.setSort(resolvedReference.getReferenceField().getSort());
}

if(!sources.isEmpty()) {
findRequest.setFrom(fromIndex);
findRequest.setTo(toIndex);
}
LOGGER.debug("execute {}: findRequest.query={}, projection={}, sort={}", node.getName(),
findRequest.getQuery(),findRequest.getProjection(),findRequest.getSort());

CRUDFindResponse response;
if(sources.isEmpty()) {
findRequest.setFrom(fromIndex);
findRequest.setTo(toIndex);
execute(ctx,findRequest,null);
response=execute(ctx,findRequest,null);
} else {
// We will evaluate this node for every possible combination of parent docs
// Some of those parent docs are docs that include the docs of this node.
Expand Down Expand Up @@ -217,7 +214,7 @@ public void execute(OperationContext ctx,
source.docs.size());
tuples.add(list);
}

response=new CRUDFindResponse();
// Iterate n-tuples
for(Iterator<List<DocReference>> tupleItr=tuples.tuples();tupleItr.hasNext();) {
List<DocReference> tuple=tupleItr.next();
Expand All @@ -238,7 +235,7 @@ public void execute(OperationContext ctx,
}
}

execute(ctx,findRequest,tuple);
response.setSize(response.getSize()+execute(ctx,findRequest,tuple).getSize());
}
// Once all documents are collected, if there are any reversed relationships (i.e. parent node
// has child documents), we associate them here
Expand All @@ -251,7 +248,8 @@ public void execute(OperationContext ctx,
}
}
}
}
}
return response;
}

public List<ResultDoc> getDocs() {
Expand All @@ -266,15 +264,15 @@ public QueryPlanNode getNode() {
return node;
}

private void execute(OperationContext ctx,
CRUDFindRequest findRequest,
List<DocReference> parents) {
private CRUDFindResponse execute(OperationContext ctx,
CRUDFindRequest findRequest,
List<DocReference> parents) {
OperationContext nodeCtx=ctx.getDerivedOperationContext(node.getMetadata().getName(),findRequest);
LOGGER.debug("execute {}: entity={}, findRequest.query={}, projection={}, sort={}", node.getName(),
nodeCtx.getEntityName(),
findRequest.getQuery(),findRequest.getProjection(),findRequest.getSort());
// note the response is not used, but find method changes the supplied context.
finder.find(nodeCtx,findRequest);
CRUDFindResponse response=finder.find(nodeCtx,findRequest);
LOGGER.debug("execute {}: storing {} documents", node.getName(),nodeCtx.getDocuments().size());

for(DocCtx doc:nodeCtx.getDocuments()) {
Expand All @@ -295,5 +293,6 @@ private void execute(OperationContext ctx,
LOGGER.debug("Adding {}",id);
docs.add(resultDoc);
}
return response;
}
}
18 changes: 17 additions & 1 deletion crud/src/test/java/com/redhat/lightblue/mediator/CompositeFinderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ public void retrieveAandBonly_manyB() throws Exception {
fr.setEntityVersion(new EntityVersion("A","1.0.0"));
Response response=mediator.find(fr);
Assert.assertEquals(2,response.getEntityData().size());
Assert.assertEquals(2,response.getMatchCount());
Assert.assertEquals("MANYB1",response.getEntityData().get(0).get("_id").asText());
Assert.assertEquals(2,response.getEntityData().get(0).get("nonid_b").size());
Assert.assertEquals("MANYB2",response.getEntityData().get(1).get("_id").asText());
Expand All @@ -267,7 +268,22 @@ public void retrieveAandBonly_manyB_range() throws Exception {
Assert.assertEquals(1,response.getEntityData().size());
Assert.assertEquals("MANYB1",response.getEntityData().get(0).get("_id").asText());
Assert.assertEquals(2,response.getEntityData().get(0).get("nonid_b").size());
}
}

@Test
public void retrieveAandBonly_manyB_range_incorrect_count() throws Exception {
FindRequest fr=new FindRequest();
fr.setQuery(query("{'field':'_id','op':'$in','values':['MANYB1','MANYB2']}"));
fr.setProjection(projection("[{'field':'*','recursive':1},{'field':'nonid_b'}]"));
fr.setSort(sort("{'_id':'$asc'}"));
fr.setFrom(0l);
fr.setTo(0l);
fr.setEntityVersion(new EntityVersion("A","1.0.0"));
Response response=mediator.find(fr);
Assert.assertEquals(1,response.getEntityData().size());
Assert.assertEquals(2,response.getMatchCount());
}


@Test
public void retrieveAandConly_CFirst() throws Exception {
Expand Down
5 changes: 3 additions & 2 deletions crud/src/test/java/com/redhat/lightblue/mediator/TestCrudController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ public CRUDFindResponse find(CRUDOperationContext ctx,
}
}
}
CRUDFindResponse ret=new CRUDFindResponse();
ret.setSize(output.size());

int f=from==null?0:from.intValue();
int t=to==null?output.size():(to.intValue()+1);
if(t<f||f>=output.size()) {
Expand All @@ -115,8 +118,6 @@ public CRUDFindResponse find(CRUDOperationContext ctx,
output=output.subList(f,t);
}
ctx.setDocuments(output);
CRUDFindResponse ret=new CRUDFindResponse();
ret.setSize(output.size());
return ret;
}

Expand Down

0 comments on commit 6418ac5

Please sign in to comment.