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

MongoTemplate doFind for ad-hoc query method visibility as protected. #4498

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aleixmorgadas
Copy link

Need:
The method that's being called when using method-like queries has default package visibility, compared to other methods with protected visibility.

In order to extend MongoTemplate without placing the new class in the org.springframework.data.mongodb.core package, I added the protected as method visibility.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 12, 2023
@mp911de
Copy link
Member

mp911de commented Sep 14, 2023

This is by design as we do not want to expose the method. It has a very large number of arguments and exposing it removes our desire to freely change the method.

Let's take a step back: Can you elaborate on what you're trying to achieve?

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label Sep 14, 2023
@aleixmorgadas
Copy link
Author

Sure!

We had to do some work around add filters to the query due to our system being multitenant and we wanted to avoid to add ifs everywhere to verify that the fetched data belongs to a tenant.

We extended MongoTemplate and added some annotations to allow that behaviour. The result is this OSS library.

https://github.com/aleixmorgadas/spring-boot-data-mongo-multitenant

You can see that we added the specific class into org.springframework.data.mongodb.core in order to extend the method as a work around.

The library isn't the best but it works.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 14, 2023
@mp911de mp911de added the for: team-attention An issue we need to discuss as a team to make progress label Sep 15, 2023
@christophstrobl
Copy link
Member

@aleixmorgadas thanks for providing additional information - In the light of this PR I think it would make sense to revisit #2067 and maybe even spring-projects/spring-data-commons#766.

@aleixmorgadas
Copy link
Author

aleixmorgadas commented Sep 15, 2023

I'm happy to help here.

Right now, I took the approach of extending the behaviour via the @MultiTenant annotation at the Entity level. I'm considering do it at the Repository level instead.

I think it helps making the Repository more obvious by being:

interface UserRepository extends MultiTenantCrudRepository<User, ObjectId> {
    String tenantField() {
        return "tenant";
    }
}

Instead of

interface UserRepository extends CrudRepository<User, ObjectId> {
}

@MultiTenant
@Document
class User {
    @Id 
    ObjectId id;

    @MultiTenantField
    String tenant;
}

@oscarbrookssynacor
Copy link

Hi there,
This very simple change is currently blocking multiple long standing, and I would argue, highly impactful issues, some of which have been opened and asked about for over 10 years.
spring-projects/spring-data-commons#766
spring-projects/spring-security#2629
spring-projects/spring-security#6736

All of these ultimately are blocked by this change. In light of that I would like to request that this be given a higher priority.

@mp911de
Copy link
Member

mp911de commented Feb 29, 2024

There's some tension in exposing internal methods because we have over time a lot of changes to internals. If your application would rely on these methods, these would break quite often. Instead it would make rather sense to introduce query post-processing hooks with a well-defined API.

@oscarbrookssynacor
Copy link

oscarbrookssynacor commented Feb 29, 2024

Hi, thanks for the quick reply, I understand your position, my goal is to simply try and facilitate getting the ball moving on those tickets I listed, I am not partial to the way this ticket gets resolved.
The option you are suggesting, what would it take to get that accomplished? I am fairly well versed in spring internals, but I have not worked with data-mongodb before.

@oscarbrookssynacor
Copy link

To add, it seems like there are no hard blocker to accomplishing any of the features on any of those tickets, and it seems like there is simply a lack of cross-communication between the projects, perhaps it would help to get all the respective folks involved in one conversation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention An issue we need to discuss as a team to make progress status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants