eclipse / jnosql

Eclipse JNoSQL is a framework which has the goal to help Java developers to create Jakarta EE applications with NoSQL.
Other
229 stars 72 forks source link

Improve Query Abstraction (issue/improvement) #263

Closed amoscatelli closed 1 year ago

amoscatelli commented 2 years ago

Hi there, I am trying to integrate JNoSql Document abstraction but I have some problem with the spec itself I believe.

I can/want to contribute on both of these requirement if we do agree on them.

I tried to subscribe to the dev mailing list but I was not accepted so far.

What do you thing about this ?

@otaviojava

otaviojava commented 2 years ago

Hey, @amoscatelli it sounds like a huge challenge. Sure, please, go ahead. Please, let me know if you need any help.

amoscatelli commented 2 years ago

Hi @otaviojava . Apart from the required changes I am willing to contribute with, do you already know any reason for this to be incompatible with other Document databases (for example OrangoDB?). I never worked with other document oriented databases apart from MongoDB

otaviojava commented 2 years ago

In documents databases, it should be fine. We have tests on our drivers, so it will be ok for us to test.

amoscatelli commented 2 years ago

I started working on the spec/abstraction

https://github.com/amoscatelli/nosql

Most of the work will be in the jakarta.nosql.column.criteria package, at first

amoscatelli commented 2 years ago

@otaviojava ok, I believe I finished the abstraction for the CriteriaQuery API. It is largely inspired by JPA, but it is indeed only a subset of those functionalities.

Tomorrow I'll finish adding comments to every interface, then I'll make a pull request and I'll start working on the eclipse implementation. At last, I will work on the MongoDB driver.

Please have a look and tell me if you like the direction I took. Also notice I will integrate a sort of metamodel generator in the eclipse implementation, scanning for @Entity and creating the metamodel on its own.

This is only the first part of what I am thinking of. CriteriaUpdate and CriteriaDelete will follow.

amoscatelli commented 2 years ago

I want to stress out tha fact that the Metamodel is a requirement to use the api.

otaviojava commented 2 years ago

@amoscatelli you did not send the PR to review.

amoscatelli commented 2 years ago

@otaviojava here you are, I was finishing the comments before making a pull request

otaviojava commented 2 years ago

@amoscatelli That is great progress. Are you going to create the implementation in the JNoSQL?

amoscatelli commented 2 years ago

Yes, I am going to. I wanted to know if you agreed before moving further. What I would like NOT to do, is to update every single document driver. I am going to take care of the MongoDB one, I hope the community will take care of the others (if any change is required indeed, I am not so sure atm).

otaviojava commented 2 years ago

@amoscatelli if it will implement MongoDB I believe that it would be better to start as an extension instead of the main API:

https://github.com/eclipse/jnosql-mapping-extension

otaviojava commented 2 years ago

Please, let me know what do you think.

amoscatelli commented 2 years ago

I am not sure I understand what you are saying. Didn't we agree that this abstraction is compatible with all Document dbs ? I added a method on the DocumentTemplate interface, is that wrong ? Also, I believe MongoDB drivers will need a change since it uses left associative strategy by default.

Anyway, if you want me to start as an extension is ok for me, please guide me on how to proceed and how to test the extension as I develop it.

What do I need to do next ?

otaviojava commented 2 years ago

@amoscatelli I'm thinking :thinking: In fork; https://github.com/eclipse/jnosql-mapping-extension 1) create a module call: criteria 2) move the whole API code there 3) Create a MongoDB module 4) Implement there

What do you think?

amoscatelli commented 2 years ago

@otaviojava Sorry, I still don't get it. Do you want me to fork https://github.com/eclipse/jnosql-mapping-extension ? What do you mean by module call ? Can you show me an example of what I am supposed to do ?

amoscatelli commented 2 years ago

in https://github.com/eclipse/jnosql-mapping-extension I can see only db specific packages. Where should I move the jakarta.nosql.criteria package ?

otaviojava commented 2 years ago

As the first step, yes. we can use it there to get mature and explore, even, to receive more feedback from the community, then, move it to the core API.

otaviojava commented 2 years ago

@amoscatelli sorry, I did check the code again. It makes sense to keep on the core as you did. As soon, you create it to MongoDB we can merge both. Please, let me know what you think. Again, sorry for that.

amoscatelli commented 2 years ago

Today I'll start working on it

otaviojava commented 2 years ago

@amoscatelli congrats on the progress, but in the end, it should return a DocumentQuery

Otherwise, it won't be compatible with what we have. You can use the DocumentCondition

E.g.: the (A and B) or (C and D)

DocumentCondition a = DocumentCondition.eq(Document.of("name", "Poliana"));
DocumentCondition b = DocumentCondition.eq(Document.of("name", "Otavio"));
DocumentCondition c = DocumentCondition.eq(Document.of("city", "São Paulo"));
DocumentCondition d = DocumentCondition.eq(Document.of("city", "Salvador"));
DocumentCondition andAB = DocumentCondition.and(a, b);
DocumentCondition andDC = DocumentCondition.and(c, d);
DocumentCondition finalCondition = DocumentCondition.or(andAB, andDC);

So, in this criteria API you can create this implementation:

 DocumentQuery query = new DocumentQuery() {
            @Override
            public long getLimit() {
                return 0;
            }

            @Override
            public long getSkip() {
                return 0;
            }

            @Override
            public String getDocumentCollection() {
                return null;
            }

            @Override
            public Optional<DocumentCondition> getCondition() {
                return Optional.empty();
            }

            @Override
            public List<Sort> getSorts() {
                return null;
            }

            @Override
            public List<String> getDocuments() {
                return null;
            }
        }
amoscatelli commented 2 years ago

Are you sure ? The new API will enable us to do things like this (I am finishing implementing this in the MongoDB driver) :

        CriteriaQuery<Person> criteriaQuery = new DefaultCriteriaQuery(Person.class);
        StringExpression<Person, Person> get = criteriaQuery.from().get(Person.NAME);
        CriteriaFunction<Person, Person, Person, Number> count = criteriaQuery.from().count();
        entityManager.executeQuery(
                criteriaQuery.select(
                        count
                ).where(
                        get.equal("Poliana")
                )
        );

Or for example I can even have a SUM operator

        CriteriaQuery<Person> criteriaQuery = new DefaultCriteriaQuery(Person.class);
        NumberExpression<Person, Person, Integer> get = criteriaQuery.from().get(Person.AGE);
        entityManager.executeQuery(
                criteriaQuery.select(
                        criteriaQuery.from().count(),
                        get.sum()
                ).where(
                        criteriaQuery.from().get(Person.NAME).equal("Poliana")
                )
        );

This is actually working, I am already finishing implementing the MongoDB driver.

Are you sure you want me to go back to the old API ? How can I expand the old API to do operations like these (count documents with predicates or do other aggregate operations) ? Please double check the API (I pushed a small update today)

otaviojava commented 2 years ago

@amoscatelli

The IDE is not to change the API, the API is perfect, only the implementation. It would amazing if the CriteriaQuery<Person> returns the DocumentQuerysomehow. The main goal is to have support for all the document types, not only for MongoDB.

Remember, the main point of the specification is to support more than one database. If the goal is only to support MongoDB, initially, it is fine if it goes to extension and stays there for while.

amoscatelli commented 2 years ago

@otaviojava I think you didn't understand

I am going to make an incomplete pull request for the communication layer implementation

We'll talk about this there

otaviojava commented 2 years ago

@amoscatelli what do you think if we do a call? It will make our life easier? What do you think?

amoscatelli commented 2 years ago

@otaviojava Sure it will make our life easier for this and the next functionalities (update criteria and delete criteria and I also have in mind something regarding polymoprhism).

Where would you prefer to make the call ? (telegram ? skype ? teams ?) I am available from next monday

In the mean time here is the pull request https://github.com/eclipse/jnosql-communication-driver/pull/178

otaviojava commented 2 years ago

@amoscatelli please, send me an e-mail: me@otaviojava.com

btw: you did a great job.

amoscatelli commented 2 years ago

@otaviojava thank you

I sent you an email, let me know if you receive it

otaviojava commented 2 years ago

Yeap, I saw yet. See you soon.

otaviojava commented 2 years ago

Hey @amoscatelli Sorry for the delay

I've created the MongoDB extension and the interface to extend the particular behavior on MongoDB:

https://github.com/eclipse/jnosql-mapping-extension/blob/master/mongodb-extension/src/main/java/org/eclipse/jnosql/mapping/mongodb/MongoDBTemplate.java

The next step would be to move your code to this project and open a PR. So, we can work together. Please, let me know what you think.

amoscatelli commented 2 years ago

I moved the code here :

https://github.com/eclipse/jnosql-mapping-extension/pull/60#issuecomment-1134897464

Let's continue there and move it !

otaviojava commented 1 year ago

We moved it to mapping-extensions