jaystack / odata-v4-mongodb

This server component lets you process OData requests over a MongoDB database
30 stars 24 forks source link

Fix for Equals and notEquals Expressions. Added Support for toLower and toUpper Functions. #19

Open jordanparrott96 opened 3 years ago

jordanparrott96 commented 3 years ago

Hi,

I stumbled across your package and found it useful for oData to MongoDB conversions. My main problem with it however arose from the lack of toUpper and toLower function support.

From this pull request: https://github.com/jaystack/odata-v4-mongodb/pull/11. As the pull request has been lying dormant I added in support for these methods (and associated tests) and as our database is case sensitive, I effectively pass them straight through. Happy to discuss an alternative for these functions. I also ran into an issue where the context identifier was being deleted after a method so it couldn't be used in any other operation. The only case I saw that this was needed was for the contains method. Again happy to discuss this.

I updated the test expression numbers to the newest documentation I could find while I was at it. https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#_Toc31361028

Uzlopak commented 3 years ago

Unfortunately jaysoft is kind of dead. They don't merge anything. I will probably fork this for our company. Kind of interested on more changes if you provide them.

aaghevli commented 3 years ago

Yeah same. If there is a fork, it would be nice to keep it somewhat combined and possibly archive this. I am not using the package much anymore, but find it to be nice and useful, so can volunteer some time to reviews.

Assuming nobody objects, I can merge this this week.

jordanparrott96 commented 3 years ago

Thank you! I am not quite sure why the need for the $eq is now necessary in the tests, but this looks good to me.

The $eq was to account for problems that arose with the use of $not and other functions which effectively wrap the query. As

{displayName: {$eq: 'name'} } is equivalent to {displayName: 'name' }

When a function like 'not' was used the original program would produce the following (which is an error in mongo),

{displayName: {$not : 'name'}}

With the implemented change (this works as intentional in mongo):

{displayName: {$not: {$eq: 'name'}}}

So the program now produces {displayName: {$eq: 'name'} } instead of {displayName: 'name' }. So I had to change the tests to acknowledge this change.

jordanparrott96 commented 3 years ago

Yeah same. If there is a fork, it would be nice to keep it somewhat combined and possibly archive this. I am not using the package much anymore, but find it to be nice and useful, so can volunteer some time to reviews.

I'm more than happy to work on a forked repo of this or base a package on this, if this is indeed not being maintained anymore. I'll need to implement more features of oData into a mongoDB like conversion going forward anyway.

aaghevli commented 3 years ago

Another option is to open up contributions to this repo. I am not sure what the best path forwards is, but I am open to help.

jordanparrott96 commented 3 years ago

Alright, I'll continue development on my fork in the meantime (as I still require a couple features going forward). If the decision is made to open this repo up for contributions I'd be open to help review as well.

aaghevli commented 3 years ago

Oh no. I actually do not have write access to this. I thought I did since I am not sure why I would be getting notifications otherwise. Let me know if you do fork it permanently since I am not sure jaystack will be really responsive.

Uzlopak commented 3 years ago

Does somebody of you have a solution for $count?

I think that it actually start to stream all results when doing a count :/

I have a test dataset with 5 mio documents, which are only streamed to return the number of documents.