Support space in orderByFields separator

jkerr5 commented 5 years ago

ArcGIS Pro uses a comma plus a space , as a separator when multiple orderByFields are specified. Koop throws the following 500 error with requests like this though:

{"error":"Parse error on line 1:\n...rties->`domain` ASC,\n-----------------------^\nExpecting 'LITERAL', 'BRALITERAL', 'LPAR', 'NUMBER', 'STRING', 'SHARP', 'DOLLAR', 'AT', 'VALUE', 'COLON', 'NOT', 'IF', 'PLUS', 'STAR', 'QUESTION', 'CURRENT_TIMESTAMP', 'JAVASCRIPT', 'NEW', 'CAST', 'CONVERT', 'SUM', 'COUNT', 'MIN', 'MAX', 'AVG', 'FIRST', 'LAST', 'AGGR', 'ARRAY', 'REPLACE', 'DATEADD', 'DATEDIFF', 'INTERVAL', 'TRUE', 'FALSE', 'NSTRING', 'NULL', 'EXISTS', 'ARRAYLBRA', 'BRAQUESTION', 'CASE', 'TILDA', 'MINUS', 'ATLBRA', 'LCUR', got 'EOF'"}

The following request fails for example:

<feature layer URL>/query?f=json&outFields=%20domain%2cOBJECTID&where=1%20%3d%201&resultOffset=0&resultRecordCount=200&returnGeometry=false&orderByFields=%20domain%20ASC%2c%20OBJECTID%20ASC

jkerr5 commented 5 years ago

Here's a full trace for this error:

Trace: SyntaxError: Parse error on line 1:
...rties->`domain` ASC,
-----------------------^
Expecting 'LITERAL', 'BRALITERAL', 'LPAR', 'NUMBER', 'STRING', 'SHARP', 'DOLLAR', 'AT', 'VALUE', 'COLON', 'NOT', 'IF', 'PLUS', 'STAR', 'QUESTION', 'CURRENT_TIMESTAMP', 'JAVASCRIPT', 'NEW', 'CAST', 'CONVERT', 'SUM', 'COUNT', 'MIN', 'MAX', 'AVG', 'FIRST', 'LAST', 'AGGR', 'ARRAY', 'REPLACE', 'DATEADD', 'DATEDIFF', 'INTERVAL', 'TRUE', 'FALSE', 'NSTRING', 'NULL', 'EXISTS', 'ARRAYLBRA', 'BRAQUESTION', 'CASE', 'TILDA', 'MINUS', 'ATLBRA', 'LCUR', got 'EOF'
    at Parser.parser.parseError (/koop/node_modules/alasql/dist/alasql.fs.js:2211:8)
    at Parser.parse (/koop/node_modules/alasql/dist/alasql.fs.js:2085:22)
    at Function.alasql.parse (/koop/node_modules/alasql/dist/alasql.fs.js:4425:22)
    at Function.alasql.dexec (/koop/node_modules/alasql/dist/alasql.fs.js:4618:19)
    at Function.alasql.exec (/koop/node_modules/alasql/dist/alasql.fs.js:4595:17)
    at alasql (/koop/node_modules/alasql/dist/alasql.fs.js:138:16)
    at standardQuery (/koop/node_modules/winnow/dist/executeQuery.js:33:18)
    at Object.Winnow.query (/koop/node_modules/winnow/dist/index.js:33:17)
    at query (/koop/node_modules/featureserver/dist/query.js:38:56)
    at execQuery (/koop/node_modules/featureserver/dist/route.js:51:16)
    at Object.route (/koop/node_modules/featureserver/dist/route.js:40:14)
    at /koop/node_modules/koop-output-geoservices/index.js:16:24
    at /koop/node_modules/koop/dist/models/index.js:15:9
    at mq.providerGetData.then.data (/koop/marklogic.js:45:8)
    at <anonymous>
    at execQuery (/koop/node_modules/featureserver/dist/route.js:53:52)
    at Object.route (/koop/node_modules/featureserver/dist/route.js:40:14)
    at /koop/node_modules/koop-output-geoservices/index.js:16:24
    at /koop/node_modules/koop/dist/models/index.js:15:9
    at mq.providerGetData.then.data (/koop/marklogic.js:45:8)
    at <anonymous>

rgwozdz commented 5 years ago

@jkerr5 thanks for reporting this. I'd be happy to to take this on, unless you already have a PR in progress.

jkerr5 commented 5 years ago

I haven't started on anything yet so an update from your side would be great!

rgwozdz commented 5 years ago

@jkerr5 I am having trouble reproducing this. Can you let me know which version of winnow and alasql you are experiencing this with?

jkerr5 commented 5 years ago

Winnow version 1.16.3 Alasql version 0.4.11

rgwozdz commented 5 years ago

Hmm, ok. I didn't get the same error as you. But I did find something else amiss with orderByFields and have PR https://github.com/koopjs/winnow/pull/99 to address it. Maybe its related to what you are seeing.

rgwozdz commented 5 years ago

@jkerr5 please try upgrading to latest winnow (1.16.4) and let me know if this solves the issue.

jkerr5 commented 5 years ago

Yes, this looks resolved now. Thanks!

On Thu, Sep 13, 2018 at 7:47 PM Rich Gwozdz notifications@github.com wrote:

@jkerr5 https://github.com/jkerr5 please try upgrading to latest winnow (1.16.4) and let me know if this solves the issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/koopjs/FeatureServer/issues/116#issuecomment-421093274, or mute the thread https://github.com/notifications/unsubscribe-auth/AAP8AhJ0lswaejeBdG4-zMnQZjb0eDb_ks5uapoYgaJpZM4WYGrz .

koopjs / FeatureServer

Support space in orderByFields separator #116