Skip to content

Commit fc0ace8

Browse files
bvulajbvulaj
authored
Merge pull request #782 from bserdar/781
Fix #781: Do not get results if there is an error
2 parents bb38898 + f0c5c6a commit fc0ace8

File tree

3 files changed

+87
-63
lines changed

3 files changed

+87
-63
lines changed

‎crud/src/main/java/com/redhat/lightblue/mediator/Mediator.java‎

Lines changed: 68 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -424,40 +424,45 @@ private QueryExpression rewriteUpdateQueryForCompositeSearch(CompositeMetadata m
424424
CompositeFindImpl finder = new CompositeFindImpl(md);
425425
finder.setParallelism(9);
426426
CRUDFindResponse response = finder.find(findCtx, freq.getCRUDFindRequest());
427-
DocumentStream<DocCtx> docStream = findCtx.getDocumentStream();
428-
429-
// Now write a query
430-
List<QueryExpression> orq = new ArrayList<>();
431-
for (;docStream.hasNext();) {
432-
DocCtx doc=docStream.next();
433-
if(!doc.hasErrors()) {
434-
DocId id = docIdx.getDocId(doc);
435-
List<QueryExpression> idList = new ArrayList<>(identityFields.length);
436-
for (int ix = 0; ix < identityFields.length; ix++) {
437-
if (!identityFields[ix].equals(PredefinedFields.OBJECTTYPE_PATH)) {
438-
Object value = id.getValue(ix);
439-
idList.add(new ValueComparisonExpression(identityFields[ix],
440-
BinaryComparisonOperator._eq,
441-
new Value(value)));
427+
if(findCtx.hasErrors()) {
428+
ctx.addErrors(findCtx.getErrors());
429+
} else {
430+
DocumentStream<DocCtx> docStream = findCtx.getDocumentStream();
431+
432+
// Now write a query
433+
List<QueryExpression> orq = new ArrayList<>();
434+
for (;docStream.hasNext();) {
435+
DocCtx doc=docStream.next();
436+
if(!doc.hasErrors()) {
437+
DocId id = docIdx.getDocId(doc);
438+
List<QueryExpression> idList = new ArrayList<>(identityFields.length);
439+
for (int ix = 0; ix < identityFields.length; ix++) {
440+
if (!identityFields[ix].equals(PredefinedFields.OBJECTTYPE_PATH)) {
441+
Object value = id.getValue(ix);
442+
idList.add(new ValueComparisonExpression(identityFields[ix],
443+
BinaryComparisonOperator._eq,
444+
new Value(value)));
445+
}
442446
}
447+
QueryExpression idq;
448+
if (idList.size() == 1) {
449+
idq = idList.get(0);
450+
} else {
451+
idq = new NaryLogicalExpression(NaryLogicalOperator._and, idList);
452+
}
453+
orq.add(idq);
443454
}
444-
QueryExpression idq;
445-
if (idList.size() == 1) {
446-
idq = idList.get(0);
447-
} else {
448-
idq = new NaryLogicalExpression(NaryLogicalOperator._and, idList);
449-
}
450-
orq.add(idq);
455+
}
456+
docStream.close();
457+
if (orq.isEmpty()) {
458+
return null;
459+
} else if (orq.size() == 1) {
460+
return orq.get(0);
461+
} else {
462+
return new NaryLogicalExpression(NaryLogicalOperator._or, orq);
451463
}
452464
}
453-
docStream.close();
454-
if (orq.isEmpty()) {
455-
return null;
456-
} else if (orq.size() == 1) {
457-
return orq.get(0);
458-
} else {
459-
return new NaryLogicalExpression(NaryLogicalOperator._or, orq);
460-
}
465+
return null;
461466
}
462467

463468
/**
@@ -499,25 +504,25 @@ public Response find(FindRequest req) {
499504
CRUDFindResponse result = finder.find(ctx, req.getCRUDFindRequest());
500505
ctx.measure.end("finder.find");
501506

502-
ctx.measure.begin("postProcessFound");
503-
DocumentStream<DocCtx> docStream=ctx.getDocumentStream();
504-
List<ResultMetadata> rmd=new ArrayList<>();
505-
response.setEntityData(factory.getNodeFactory().arrayNode());
506-
for(;docStream.hasNext();) {
507-
DocCtx doc=docStream.next();
508-
if(!doc.hasErrors()) {
509-
response.addEntityData(doc.getOutputDocument().getRoot());
510-
rmd.add(doc.getResultMetadata());
507+
if(!ctx.hasErrors()) {
508+
ctx.measure.begin("postProcessFound");
509+
DocumentStream<DocCtx> docStream=ctx.getDocumentStream();
510+
List<ResultMetadata> rmd=new ArrayList<>();
511+
response.setEntityData(factory.getNodeFactory().arrayNode());
512+
for(;docStream.hasNext();) {
513+
DocCtx doc=docStream.next();
514+
if(!doc.hasErrors()) {
515+
response.addEntityData(doc.getOutputDocument().getRoot());
516+
rmd.add(doc.getResultMetadata());
511517
} else {
512-
DataError error=doc.getDataError();
513-
if(error!=null)
514-
response.getDataErrors().add(error);
518+
DataError error=doc.getDataError();
519+
if(error!=null)
520+
response.getDataErrors().add(error);
521+
}
515522
}
516-
}
517-
docStream.close();
518-
response.setResultMetadata(rmd);
519-
ctx.measure.end("postProcessFound");
520-
if (!ctx.hasErrors()) {
523+
docStream.close();
524+
response.setResultMetadata(rmd);
525+
ctx.measure.end("postProcessFound");
521526
ctx.setStatus(OperationStatus.COMPLETE);
522527
} else {
523528
ctx.setStatus(OperationStatus.ERROR);
@@ -793,23 +798,25 @@ private List<DataError> setResponseResults(OperationContext ctx,
793798
int t=to==null?Integer.MAX_VALUE:to.intValue();
794799
int ix=0;
795800
DocumentStream<DocCtx> docStream=ctx.getDocumentStream();
796-
List<ResultMetadata> rmd=new ArrayList<>();
797-
for(;docStream.hasNext();) {
798-
DocCtx doc=docStream.next();
799-
if(!doc.hasErrors()) {
800-
if(ix>=f&&ix<=t) {
801-
response.addEntityData(doc.getOutputDocument().getRoot());
802-
rmd.add(doc.getResultMetadata());
801+
if(docStream!=null) {
802+
List<ResultMetadata> rmd=new ArrayList<>();
803+
for(;docStream.hasNext();) {
804+
DocCtx doc=docStream.next();
805+
if(!doc.hasErrors()) {
806+
if(ix>=f&&ix<=t) {
807+
response.addEntityData(doc.getOutputDocument().getRoot());
808+
rmd.add(doc.getResultMetadata());
809+
}
810+
ix++;
811+
} else {
812+
DataError error=doc.getDataError();
813+
if(error!=null)
814+
dataErrors.add(error);
803815
}
804-
ix++;
805-
} else {
806-
DataError error=doc.getDataError();
807-
if(error!=null)
808-
dataErrors.add(error);
809816
}
817+
docStream.close();
818+
response.setResultMetadata(rmd);
810819
}
811-
docStream.close();
812-
response.setResultMetadata(rmd);
813820
return dataErrors;
814821
}
815822
}

‎crud/src/test/java/com/redhat/lightblue/mediator/CompositeFinderTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -773,14 +773,14 @@ public void assoc_errors_propagate() throws Exception {
773773
errorEntity = "U";
774774
Response response = mediator.find(fr);
775775
System.out.println(response.getEntityData());
776-
Assert.assertEquals(0, response.getEntityData().size());
776+
Assert.assertTrue(response.getEntityData()==null||response.getEntityData().size()==0);
777777
Assert.assertEquals(1, response.getErrors().size());
778778

779779
findError = Error.get("NoAccess", "blah");
780780
errorEntity = "L";
781781
response = mediator.find(fr);
782782
System.out.println(response.getEntityData());
783-
Assert.assertEquals(0, response.getEntityData().size());
783+
Assert.assertTrue(response.getEntityData()==null||response.getEntityData().size()==0);
784784
Assert.assertEquals(1, response.getErrors().size());
785785

786786
}

‎crud/src/test/java/com/redhat/lightblue/mediator/MediatorTest.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import com.redhat.lightblue.query.ValueComparisonExpression;
5454
import com.redhat.lightblue.util.JsonDoc;
5555
import com.redhat.lightblue.util.Path;
56+
import com.redhat.lightblue.util.Error;
5657

5758
public class MediatorTest extends AbstractMediatorTest {
5859

@@ -383,6 +384,22 @@ public void findResultMdTest() throws Exception {
383384
}
384385
}
385386

387+
@Test
388+
public void queryTimeoutTest() throws Exception {
389+
FindRequest req = new FindRequest();
390+
req.setEntityVersion(new EntityVersion("test", "1.0"));
391+
392+
mdManager.md.getAccess().getFind().setRoles("anyone");
393+
mockCrudController.findResponse = new CRUDFindResponse();
394+
mockCrudController.findCb=ctx->{
395+
ctx.addError(Error.get(CrudConstants.ERR_DATASOURCE_TIMEOUT,"timeout"));
396+
};
397+
Response response = mediator.find(req);
398+
Assert.assertEquals(OperationStatus.ERROR, response.getStatus());
399+
Assert.assertEquals(1,response.getErrors().size());
400+
Assert.assertEquals(CrudConstants.ERR_DATASOURCE_TIMEOUT,response.getErrors().get(0).getErrorCode());
401+
}
402+
386403
@Test
387404
public void findQueryFieldRoleTest() throws Exception {
388405
FindRequest req = new FindRequest();

0 commit comments

Comments
 (0)