Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial draft of MongoDB Document API support #2239

Closed
wants to merge 2 commits into from
Closed

Initial draft of MongoDB Document API support #2239

wants to merge 2 commits into from

Conversation

mp911de
Copy link
Contributor

@mp911de mp911de commented Jan 15, 2018

This pull request introduces Document API support for MongoDB (see #2215) as additional implementation, without changing DBObject support.

Morphia is entirely based on DBObject. Tests are a bit of a mixture of objects and Documents right now. Happy to dive into a discussion.

* @param <Q> concrete subtype
* @author Mark Paluch
*/
public abstract class AbstractFetchableMongodbQuery<K, Q extends AbstractFetchableMongodbQuery<K, Q>>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a split between some base functionality and the actual execution to enable different execution models (async, reactive) to be built on top of AbstractMongodbQuery.

…ngodbQuery.

Filter creation requires query execution (for joins/DBRef's) hence it's depending on the actual execution method. AbstractFetchableMongodbQuery is using imperative API so that's the best fit in there.
@Shredder121
Copy link
Member

Just wanted to mention, looks nice so far.
👀

So what would you recommend with the split of execution modes? How would you see the reactive mongodbquery work?

@mp911de
Copy link
Contributor Author

mp911de commented Apr 4, 2018

The main issue to tackle is query execution required for joins/relations. Since RxJava 1 is EOL, the only reactive driver is the Reactive Streams one. Maybe we could introduce something like continuations that are agnostic in regard to the underlying execution model.

This increases complexity in general but could be a nice approach to support both execution models from a single base class.

The idea is to introduce something like callback interfaces along the desired operation/criteria to query on a particular collection.

@odrotbohm
Copy link

odrotbohm commented Jun 6, 2018

Hey guys, any chance we can get some feedback on your plans going forward? The MongoDB Java driver team has indicated that it plans to remove the deprecated DBObject based APIs in some future version and we don't want to end up with the Spring Data Querydsl support ending up broken in a driver release.

I wouldn't mind getting the support for the new APIs introduced in a parallel type hierarchy so that we can move Spring Data over to the new APIs.

@Shredder121
Copy link
Member

I myself am not well-versed in the world of reactive programming, so maybe you, @mp911de can maintain the mongodb subproject once it's in the mainline? I don't know if it's something you are able/willing to do?

@odrotbohm
Copy link

We're not talking anything reactive here but the fact that the MongoDB Java driver switched from using DBObject to Document for its synchronous operations a while ago.

@mp911de
Copy link
Contributor Author

mp911de commented Jun 6, 2018

This pull request does not contain any reactive bits. However, there's a split between base functionality and the actual class which executes the query to not make assumptions about the execution model within basic query creation functionality.

@Shredder121
Copy link
Member

We're not talking anything reactive here

Oh, sorry. In that case I wouldn't mind migrating to Document exclusively, for Querydsl 5.
Unless you want both DBObject and Document support for now? Or both in Querydsl 4 and 5 respectively?

This pull request does not contain any reactive bits. However, there's a split between base functionality and the actual class which executes the query to not make assumptions about the execution model within basic query creation functionality.

EDIT: saw Mark's comment just now, yes I know about the split, but I'd rather you take a bit of responsibility, so you can get the mongodb submodule (and with assist also core) up to spec for later having reactive queries.

Then I think the process can be a bit more sped up, since I've been really busy lately, as you probably noticed.

@jwgmeligmeyling
Copy link
Member

@odrotbohm @mp911de Is this PR actually sufficient to port Spring Data over to the new API already? Or is this really still the initial draft?

Our current set of maintainers has little experience or ongoing work with MongoDB, so it is hard for me to review this properly. I am very willing to integrate it though if it helps you forward and helps the overall compatibility with DBObject being deprecated.

That said, what do you think about bumping the dependency versions of the MongoDB driver and/or Morphia (see #2029)? Which version is Spring Data currently targetting?

@mp911de
Copy link
Contributor Author

mp911de commented Oct 6, 2020

Let me have a look and potentially rebase the code. MongoDB 4.1 is btw. the most recent version. Spring Data MongoDB 3.1 (to be released in late October) is targeting the MongoDB 4.1 driver.

I will leave a comment with further details once I had a look.

@jwgmeligmeyling
Copy link
Member

Great, thanks! Lets finally get this out of the way after 2 years 😉

@mp911de
Copy link
Contributor Author

mp911de commented Oct 6, 2020

Meh! I've deleted my repository in the meantime and so this PR base got lost. I suggest creating a new PR from https://github.com/mp911de/querydsl/tree/issue/mongodb-document-api and closing this one. We fixed a few issues in Spring Data (we've inlined the Querydsl code) and the new PR would include also the fixes.

@mp911de
Copy link
Contributor Author

mp911de commented Nov 2, 2020

Closing in favor of #2689.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants