Skip to content

Conversation

dsyer
Copy link
Member

@dsyer dsyer commented Nov 5, 2018

@Bean methods are usually public, but there is one class in Spring
Data that breaks this convention, and Spring Boot happens to
extend it. The normal reflective use of @Configuration isn't affected
but if you wanted to build a functional bean registration from the
Spring Boot code you couldn't without the public accessor.

See spring-projects/spring-data-relational#100 for
a change in Spring Data that makes this redundant, but might not be
available till after Lovelace.

See spring-projects/spring-data-relational#100 for
a change in Spring Data that makes this redundant, but might not be
available till after Lovelace.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 5, 2018
@wilkinsona
Copy link
Member

That's pretty unpleasant. Can we back port the changes into the next Lovelace SR instead please?

@wilkinsona wilkinsona added status: on-hold We can't start working on this issue yet and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 5, 2018
@dsyer
Copy link
Member Author

dsyer commented Nov 5, 2018

We could, and that might still happen. The Spring Data team are concerned that a Lovelace SR can't change the visibility without breaking existing apps (an app that extended the Lovelace.RELEASE class and overrode but didn't change the access on those methods).

@wilkinsona
Copy link
Member

Is there somewhere where we can track that decision being made?

@dsyer
Copy link
Member Author

dsyer commented Nov 6, 2018

https://jira.spring.io/browse/DATAJDBC-288 (looks like it is already merged into Lovelace, but not released yet). We should probably wait until the next Lovelace SR before closing this because @odrotbohm had an opinion that seemed to suggest it would have to wait for M.

@schauder
Copy link
Contributor

schauder commented Nov 6, 2018

DATAJDBC-288 is NOT merged in Lovelace, only in Moore. (the "Lovelace" in the ticket is just the Sprint).

@dsyer
Copy link
Member Author

dsyer commented Nov 6, 2018

Well that’s confusing. Oh well, looks like we need this in Boot now, till Moore.

@odrotbohm
Copy link
Member

I guess the first question to be answered is: in what Boot version is this needed? If it's 2.1 then I guess the fix in Boot is less likely to be breaking than a backport of the fix in SD JDBC as with the latter in use without Boot, it's very likely someone has extended the class and we'd break binary compatibility. I guess Boot auto-configurations are much less likely to be extended by user code. Boot 2.2 could remove those overrides again as it'll be requiring Moore anyway, right?

@wilkinsona
Copy link
Member

Boot's sub-class has the benefit of being package-private so it's really unlikely that anyone has sub-classed it. However, that also means that I don't see the benefit of making the @Bean methods public. Can you enlighten me please, @dsyer?

@dsyer
Copy link
Member Author

dsyer commented Nov 6, 2018

I want to make an ApplicationInitializer and call the @Bean methods. I can do that in the same package if the methods are public (or package private I guess, but public is the convention everywhere else).

@wilkinsona
Copy link
Member

I get it now. Thank you. I still dislike having to do this in Boot to work around what I consider to be a design bug and inconsistency in Spring Data, but I guess we don't have much choice without holding up the functional bean registration efforts and I dislike that even more.

@dsyer
Copy link
Member Author

dsyer commented Nov 6, 2018

I'll try to make a diary entry for a PR to reverse it in 2.2

@wilkinsona wilkinsona removed the status: on-hold We can't start working on this issue yet label Nov 6, 2018
@wilkinsona wilkinsona added this to the 2.1.x milestone Nov 6, 2018
@wilkinsona wilkinsona added the type: task A general task label Nov 6, 2018
@snicoll snicoll changed the title Make @Bean methods in Spring Data JDBC public Temporarily make @Bean methods in Spring Data JDBC public Nov 16, 2018
@snicoll snicoll self-assigned this Nov 16, 2018
@snicoll snicoll modified the milestones: 2.1.x, 2.1.1 Nov 16, 2018
@snicoll snicoll closed this in b6aff10 Nov 16, 2018
snicoll added a commit that referenced this pull request Nov 16, 2018
* pr/15097:
  Temporarily make @bean methods in Spring Data JDBC public
@snicoll
Copy link
Member

snicoll commented Nov 16, 2018

@dsyer I've created #15197 to upgrade to Moore's first milestone and added a note that this commit should be reverted.

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

Successfully merging this pull request may close these issues.

6 participants