thingdom/node-neo4j

resultDataContents

ekkis opened this issue · 8 comments

ekkis commented

in looking at my distribution's lib-new/GraphDatabase.js, bottom of the cypher method, I see:

236             formats.push(format = lean ? 'row' : 'rest');
237             _results.push({
238               statement: query,
239               parameters: params || {},
240               resultDataContents: [format]
241             });

I tried looking at the sources here on github but I don't read coffee easily and couldn't even find the code, but in essence, line 236 seems to me wants to be format == so it's a comparison, not an assignment. also line 240 should likely be: resultDataContents: formats (or perhaps I'm wrong because I see formats is defined outside the function that generates the statements, so it seems to be a collector whose values are tossed...

in any case "rest" is not a valid value for the endpoint, "graph" is and I don't see a way to pass that into the code. is this borked or how do I do this?

q = { 
    statement: "match (n) return n",
    resultDataContents: ["graph"],
    includeStats: false
};  
q = JSON.stringify({statements: [q]});
db.cypher(q, function(ls) { ... });

Hey @ekkis,

CoffeeScript is definitely terse in cases like this, but it's correct.

In JS, the order of operations is that a ? b : c is evaluated before x = ..., so format = lean ? 'row' : 'rest' is correctly assigning format to either 'row' (if the lean option was true) or 'rest'.

And then resultDataContents should not be formats (plural), because formats is the array of formats corresponding to the batch of (potentially multiple) queries in this request.

Finally, 'rest' is a valid value for the endpoint. It's how you get back node and relationship metadata beyond just property data, e.g. labels and native IDs.

http://neo4j.com/docs/2.3.0/rest-api-transactional.html#rest-api-execute-statements-in-an-open-transaction-in-rest-format-for-the-return

To your question at the end, node-neo4j indeed provides no API today to return data in 'graph' format. If you have this as a feature request, would you mind sharing your use case a bit? The 'rest' format does give you all the data you need, as far as I've ever been able to tell.

Hope that helps. Cheers.

ekkis commented

aseemk, thank you so much for the quick reply. yes, of course, it's embarrassing that I wouldn't have understood the = vs == from the start, and you're right, of course... "rest" is a valid value (I learned something new today - though the docs are not very good on what the valid list of values are).

as for my use case, I'm porting an app from using the linkurious library (which uses the "graph" format returns) to using node-neo4j. I would prefer not to have to rewrite everything but just receive the data as it expects. I haven't yet taken the time to analyze the result structure of "rest" but it is different from that of "graph". I've forked and cloned your project (wasn't sure, branch v2 or thingdom/v2?) with the intention of patching it to allow returning this format but I'm unsure: do I patch the coffee source in lib-new, or the .js source? I tried modifying both and running npm build to see which produced which but they seem to be independent of each other

No problem at all.

Got it. Thanks for explaining.

Here are the instructions for hacking on this library:

https://github.com/thingdom/node-neo4j/blob/v2/CONTRIBUTING.md#development

npm run build (note you need the run in there; different than npm build) overwrites the .js with the .coffee, so if you're editing the .js directly, don't run npm run build. (Modifying the .coffee is recommended for this reason, if you're comfortable doing so.)

Good luck, and let me know if I can help anymore! I'm going to go ahead close this issue, but feel free to keep posting any q's here. Cheers.

Btw, the v2 branch in this thingdom/node-neo4j repo is the right branch! I really need to finish it and merge it to master. Time is just hard to find these days. =)

ekkis commented

ok, I figured out how to get what I want: support for the 'graph' formatted results the API supports. it was a small patch and is lint clean, however, I've never written coffee and am not smart enough to implement the test. I added the test but cannot make it pass. help? here's the pull request: #207

ekkis commented

I'm not sure that overloading the lean attribute was the right thing to do. that attribute doesn't appear in the API and it would have been better if resultDataContents had just been supported directly. I didn't want to have both and removing lean would probably have been an IBC, ergo my choice, but do please feel free to modify it as you will

ekkis commented

and yes, I know about time being short... but it would be great (for me) if you could do a release to npm with the patch soon. I'm reading about npm link (thank you for pointing that out in your docs) so I can get by in the meantime

Awesome, @ekkis! Glad you figured this out for your needs, and thanks for opening the PR. Let's move the discussion on your approach to there.