When using the client syntax (so using a query like {{db.something.find(...)}}) with a ill formed JSON query, we have an obscure error about using {{db}} instead of a valid JSON, whereas the issue is not about the db part.
We should try to improve the parser so that it reports an understandable error in the following cases:
* the command used is not supported (command as in {{find}})
* the JSON query is not valid (but it does not make the db part invalid)
Some more thoughts that might provide some useful guidance. Note that I didn't play with it so it's just rough ideas for now.
The {{Operation()}} rule is one of the interesting ones. Basically, we have the current behavior right now: either you have exactly the right command and the right format or it considers that it will be not be a {{db.xxx.yyy()}} command at all.
So to give a few examples of what could be done:
- the parser keeps the state pushed to the builder, so the idea is to push as much information as possible in the builder, so that it is able to properly make decisions and provide useful error reports
- for instance, we could change the db() rule to be:
{code}
@SuppressNode
public Rule Db() {
return Sequence( ZeroOrMore( WhiteSpace() ), "db ", Separator(), builder.setCliQuery( true ) );
}
{code}
allowing the builder to know we are dealing with a cliQuery.
- then I would add a last choice to the Operation() rule with an operation composed of any alphabetical characters so that later we could report it as an unsupported command
- I also think each Operation() (find for instance) should be built differently e.g. we should set the operation type as soon as we find the right command but have a flag saying the operation is complete/valid at the end of the command parsing
so basically:
{code}
Sequence( Find(), builder.setOperationValid( true ) ), // <--- we say the command is valid instead of setting the operation type
{code}
and:
{code}
public Rule Find() {
return Sequence(
Separator(),
"find ",
"( ",
builder.setOperation( Operation.FIND ), // <----- this is new
JsonObject(), builder.setCriteria( match() ),
Optional( Sequence( ", ", JsonObject(), builder.setProjection( match() ) ) ),
") "
);
}
{code}
- we should also check if we can make the CriteriaOnlyFindQuery() a bit more restrictive (but we should check that MongoDB doesn't accept all sorts of weird syntaxes for this JSON)
Then the idea is to use all this information we collect to improve {{MongoDBQueryDescriptorBuilder#build()}} to make educated decisions about what this query is and return proper error message if the query is not valid. |
|
