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

TestcontainersLifecycleBeanPostProcessor does not work correctly with scoped beans #35786

Closed
eddumelendez opened this issue Jun 8, 2023 · 13 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@eddumelendez
Copy link
Contributor

eddumelendez commented Jun 8, 2023

I’ve been playing around with several examples using Spring Boot Testcontainers at Development time and last night I noticed those are not working as expected with @RestartScope. IIRC this example used to work in the past, download the image, start the container and so on. I also tried something similar to what’s mentioned in the docs but the image is not downloaded at all in this example if @ServiceConnection is removed. It works as expected without @RestartScope

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 8, 2023
@eddumelendez eddumelendez changed the title Contributing Dynamic Properties at Development Time are not working with @RestartScope Contributing Dynamic Properties at Development Time is not working with @RestartScope Jun 8, 2023
@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 16, 2023
@philwebb philwebb added this to the 3.1.x milestone Jun 16, 2023
@nedimAT
Copy link

nedimAT commented Jun 23, 2023

Hello @philwebb, i would like to fix this. Please assign it to me.

@philwebb
Copy link
Member

Thanks @nedimAT, I've assigned the issue to you. I've no idea if it's an easy or difficult fix so feel free to comment here if you don't get anywhere.

@nedimAT
Copy link

nedimAT commented Jun 27, 2023

Hello @eddumelendez, for me the rabbitmq example works as expected. Removing the @ServiceConnection Annotation means you have to setup the properties yourself, see docs dynamic-properties
The Pulsar example i can not get to run on my machine, caused by this bug
Can you provide further information about your issue and probably some logs? Your logs should contain some insights about what testcontainers is up to, similar to the following tc.rabbitmq:3.11-alpine : Pulling docker image: rabbitmq:3.11-alpine. Please be patient; this may take some time but only needs to be done once.

@eddumelendez
Copy link
Contributor Author

eddumelendez commented Jun 27, 2023

I have another example. See https://github.com/eddumelendez/spring-boot-sqs-testcontainers-reusable-mode

In summary, @RestartScope works with existing supported ServiceConnections but not when used along with DynamicProperties. Not close at the computer right now but in those scenarios is like Testcontainers is ignored and not picked up. You should be able to reproduce with this last example. If needed I can attach logs later.

@nedimAT
Copy link

nedimAT commented Jun 27, 2023

Thanks, with that example i can reproduce it. I will start looking into it.

@nedimAT
Copy link

nedimAT commented Jul 11, 2023

Hello @philwebb could you provide me with some hint where i should start. What i know so far is, that @RestartScope on its own is not enough for the Container to be started. One needs additionally @Scope or @ServiceConnection, but then the Container will restart with the Application. I searched now for some time in the DefaultListableBeanFactory for clues but i think i am stuck.

@eddumelendez
Copy link
Contributor Author

eddumelendez commented Jul 15, 2023

I did another check and wonder if just changing the following lines

beanNames.addAll(List.of(this.beanFactory.getBeanNamesForType(ContainerState.class, false, false)));
beanNames.addAll(List.of(this.beanFactory.getBeanNamesForType(Startable.class, false, false)));

to

beanNames.addAll(List.of(this.beanFactory.getBeanNamesForType(ContainerState.class, true, false)));
beanNames.addAll(List.of(this.beanFactory.getBeanNamesForType(Startable.class, true, false)));

due to the scope is not singleton anymore but restart.

@philwebb philwebb changed the title Contributing Dynamic Properties at Development Time is not working with @RestartScope Contributing Dynamic Properties at Development Time is not working with @RestartScope Nov 7, 2023
@Wzy19930507
Copy link
Contributor

Wzy19930507 commented Jan 22, 2024

I have another example. See https://github.com/eddumelendez/spring-boot-sqs-testcontainers-reusable-mode

Hi, @eddumelendez, @philwebb
this example is DynamicPropertyRegistry at Development Time is not working with @RestartScope.
DynamicPropertyRegistry need init property before SqsAsyncClient.

But when add @RestartScope, scope change to restart and skip code DefaultListableBeanFactory.preInstantiateSingletons
, so DynamicPropertyRegistry init property after SqsAsyncClient.

Is it reasonable to add a tip to the documentation?

@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.2.x May 20, 2024
@philwebb philwebb self-assigned this Jun 26, 2024
@philwebb philwebb changed the title Contributing Dynamic Properties at Development Time is not working with @RestartScope TestcontainersLifecycleBeanPostProcessor does not work correctly with scoped beans Jun 26, 2024
@philwebb philwebb modified the milestones: 3.2.x, 3.2.8 Jun 26, 2024
@philwebb
Copy link
Member

Thanks for the hint @eddumelendez. Hopefully fixed in the next snapshot.

philwebb added a commit that referenced this issue Jun 26, 2024
@eddumelendez
Copy link
Contributor Author

Thanks @philwebb ! It's working :)

@shawnweeks
Copy link

I've been following this ticket since before it got renamed and with the latest 3.3.2-SNAPSHOT The combination of @RestartScope and DynamicPropertyRegistry does work on startup but all the properties get unset after dev-tools reloads the app. Previously the properties didn't get set at all so it's definitely an improvement but I'm not sure this fully fixes the issue.

@scottfrederick
Copy link
Contributor

@shawnweeks Does the discussion in #35200 and the documentation changes made in this related commit help with your use case?

@shawnweeks
Copy link

@scottfrederick They don't appear to. This issue was originally about Contributing Dynamic Properties at Development Time is not working with @RestartScope which is what still doesn't seem to be working. This issue seems to resolve part of the original issue but dynamic properties still don't fully work with RestartScope. Don't mind starting another ticket just don't want it to get closed as duplicate since this ticket appeared to be trying to resolve the same issue.

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

No branches or pull requests

8 participants