As a user, I want to know why my query syntax is invalid

[tloubrieu-jpl]

@tloubrieu-jpl commented on Wed May 05 2021

Motivation

...so that I can update my query (q param) to make it work

Additional Details

Acceptance Criteria

Given deployed API server
When I perform request q=ops:Data_File_Info.ops:file_size gte 138172
Then I expect an explicit error message like "Unkown operator gte", status 400

To be completed

Engineering Details

I believe this can be easily added by using messages in the ParseCancellationException and throwing the exception all the way through the api controllers. Actually this requires a bit a research to understand how a springboot controller method can returm multiple type (products or error).

---

@jordanpadams commented on Fri May 07 2021

@tloubrieu-jpl so was this closed per NASA-PDS/registry-api-service#15 or is this a follow-on?

---

@tloubrieu-jpl commented on Mon May 10 2021

@jordanpadams this is a follow on

---

@al-niessner commented on Tue May 18 2021

@jordanpadams @tloubrieu-jpl

I do not think that the lexer/parser tool (ANTLR) gives enough visibility to do what is being requested. For instance, not( a eq b ) will fail but it is because of a token failure in ANTLR that gives odd messages like token not FIELD or illegal character cannot form token. There is no way to reverse it to say it should have been not ( a eq b ). With gte as in example, ANTLR fails with cryptic at best information but does not relinquish control to add a message that would allow one to detect that it was an illegal operator.

If this desire is required, then you will probably need to replace ANTLR.

---

@jordanpadams commented on Wed May 19 2021

+1 thanks @al-niessner . we will need to maybe table this for a future build to investigate how we can accomplish this solution

---

@tloubrieu-jpl commented on Wed May 19 2021

@al-niessner @jordanpadams what if we have the message illegal token and the name of the token 'not(' in the message. That would not be too bad to help the user to figure a space is needed ?

For now we have nothing as error messages so I would like to initiate that behavior of the API where we can return error messages. We can discuss that at the next breakout.

---

@al-niessner commented on Wed Jul 21 2021

Here are some simple examples from the unit testing of the lexer/parser:

erroneous statement: ( a eq b
erroneous statement: a eq b )
erroneous statement: not( a eq b )
line 1:0 no viable alternative at input 'not('
erroneous statement: a eq b and c eq d and
erroneous statement: ( a eq b and c eq d and )
line 1:24 no viable alternative at input ')'
erroneous statement: ( a eq b and c eq d or e eq f )
erroneous statement: ( a amd b )
line 1:4 no viable alternative at input 'aamd'

Lets go through each of these:

( a eq b generates an exception in the parser but produces not error message. The exception is reached end of line. The obvious correction is to add an } at the end.

a eq b ) generates an exception in the parser (same exception as above) but produces no error message. Despite the exceptions being the same, the solutions are different in that this requires '(' be prepended.

not( a eq b ) generates an exception in the parser (same as previous exceptions) but also generates the error message line 1:0 no viable alternative at input 'not('. The message indicates that character in question is 'n' (character 0) when in fact it is a missing space between not and (..

Skipping a few to ' ( a amd b ) generates the same exception with the error message complaining 'line 1:4 no viable alternative at input 'aamd'. In fact, the real error is that "amd" should be "and".

The point is that knowing the lexer/parser and the syntax being used, it is possible to reverse the meaning. However doing this in code is going to require building a very complex parser to figure out why the lexer/parser exception occurred. Cannot even really use the extensions to the lexer/parser to help because the "operator" calls are never called in the last example because the lexer/parser treats a amd as a FIELD not a FIELD and OPERATOR (capitalization is to reflect the language syntax)..

---

@jordanpadams commented on Fri Jul 23 2021

@al-niessner thanks for investigating this. looking at the bugs and features that are creeping up in this repo, I think we have bigger fish to fry at the moment. I am going to defer this to a future sprint/build.

[tloubrieu-jpl]

closed by mistake