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

Profiles set with @ActiveProfiles are not visible to EnvironmentPostProcessor since 2.5.7 #28739

Closed
eftche opened this issue Nov 18, 2021 · 12 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@eftche
Copy link

eftche commented Nov 18, 2021

After upgrading 2.5.6 to 2.5.7 profiles set with @ActiveProfiles annotation on the test class are not visible to EnvironmentPostProcessor anymore both through environment and application references.
Still investigating but probably related to #28530.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 18, 2021
@mbhave
Copy link
Contributor

mbhave commented Nov 18, 2021

@fedortche It is most likely related to #28530. Can you provide a minimal sample that we can run to reproduce the issue?

@mbhave mbhave added the status: waiting-for-feedback We need additional information before we can continue label Nov 18, 2021
@eftche
Copy link
Author

eftche commented Nov 22, 2021

@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 Nov 22, 2021
@mbhave
Copy link
Contributor

mbhave commented Nov 23, 2021

Thanks @fedortche. The call to setActiveProfiles() which added profiles from @ActiveProfiles to the environment was only added to prevent a potential side-effect that could be caused by doGetActiveProfiles(). Since we switched to a different Environment type, this side-effect is no longer present and the call to setActiveProfiles() is no longer necessary. Furthermore, the change in #28530 now aligns test code with production code. Active profiles set via spring.profiles.active in the main app will not be available to any EnvironmentPostProcessor ordered before ConfigDataEnvironmentPostProcessor.

If you need to access the profiles from @ActiveProfiles in the EnvironmentPostProcessor, you can do so by getting spring.profiles.active. I don't think we should change anything here. Flagging for team-attention to see what the rest of the team thinks.

@mbhave mbhave added the for: team-attention An issue we'd like other members of the team to review label Nov 23, 2021
@eftche
Copy link
Author

eftche commented Nov 24, 2021

Yes thanks @mbhave, while looking into this I realized that profiles set previously with spring.profiles.active would have similar issue prior to 2.5.7 too. But I'm not sure if that is a correct way.

  • First, those active profiles are known prior to ConfigDataEnvironmentPostProcessor and ideally they should be set to environment prior to any environment post-processors executing.
  • Then, profiles set by annotation in this example actually would not be visible to post processor in spring.profiles.active property, in my test example it would be spring.profiles.active[0] property, so it's not really aligned with production code anyway
  • Also, let's say I have 2 post processors. First one sets second active profile using environment.addActiveProfile("bar"). Second post-processor now has two profiles, one returned by environment.getActiveProfiles(), and another one set in spring.profiles.active[0] property. It's very inconsistent that way, each post-processor would have to check active profiles, check spring.profiles.active, and check spring.profiles.active[*], and then somehow merge them in the right order. It's quite a mess.

@JeromeSimmonds
Copy link

JeromeSimmonds commented Nov 24, 2021

Hi, the following used to work with Spring Boot 2.5.6 (accessing the properties from src/test/resources/application-test.properties)

@SpringBootTest
@ActiveProfiles("test")
@AutoConfigureMockMvc
class SomeClassIT {

    @Autowired
    private MockMvc mockMvc;

    @Value("${info.app.name}")
    private String appName;

    @Test
    public void testEndpointInfo() throws Exception {
        mockMvc.perform(get("/info")).andDo(print())
            .andExpect(status().isOk())
            .andExpect(jsonPath("$.build.name").value(appName))
    }
}

but ignores the test file after upgrading to 2.6.0, most likely related to the current issue.
What do we need to change to get the properties from src/test/resources/application-test.properties override the default ones again?

@wilkinsona
Copy link
Member

wilkinsona commented Nov 29, 2021

@JeromeSimmonds Per what @mbhave has described above, you should be able to set the spring.profiles.active property in place of @ActiveProfiles, for example through @SpringBootTest:

@SpringBootTest(properties = "spring.profiles.active=test")

@wilkinsona
Copy link
Member

Active profiles set via spring.profiles.active in the main app will not be available to any EnvironmentPostProcessor ordered before ConfigDataEnvironmentPostProcessor.

I don't think @ActiveProfiles has to be equivalent to spring.profiles.active. It feels like it should really be an implementation detail to me. The javadoc describes it as:

a class-level annotation that is used to declare which active bean definition profiles should be used when loading an ApplicationContext for test classes.

In Spring Boot, loading config data is part of "loading an ApplicationContext for test class" so it feels to me like we should get @ActiveProfiles working again if we can.

@JeromeSimmonds
Copy link

@wilkinsona it doesn't work, src/test/resources/application-test.properties is still ignored.

@mbhave
Copy link
Contributor

mbhave commented Nov 30, 2021

@JeromeSimmonds Your issue sounds slightly different. The reason I suggested spring.profiles.active here was because of active profiles from @ActiveProfiles no longer being set on the Environment before ConfigDataEnvironmentPostProcessor is called. @ActiveProfiles should still work as before unless you're trying to get them from an EnvironmentPostProcessor that's ordered before ConfigDataEnvironmentPostProcessor. Please create a new issue with a small sample that we can run to reproduce the issue.

@mbhave
Copy link
Contributor

mbhave commented Nov 30, 2021

I don't think @activeprofiles has to be equivalent to spring.profiles.active. It feels like it should really be an implementation detail to me.

@wilkinsona I agree. However, I'm not sure how we can make them available to an EnvironmentPostProcessor that runs before ConfigDataEnvironmentPostProcessor otherwise, without breaking #28530.

In Spring Boot, loading config data is part of "loading an ApplicationContext for test class" so it feels to me like we should get @activeprofiles working again if we can.

@ActiveProfiles do work. They are not available using env.getActiveProfiles() in an EnvironmentPostProcessor that's ordered before ConfigDataEnvironmentPostProcessor . Flagging it so that we can discuss it on the team call.

@mbhave mbhave added for: team-meeting An issue we'd like to discuss as a team to make progress and removed for: team-attention An issue we'd like other members of the team to review labels Nov 30, 2021
@wilkinsona wilkinsona removed status: feedback-provided Feedback has been provided for: team-meeting An issue we'd like to discuss as a team to make progress labels Dec 3, 2021
@mbhave
Copy link
Contributor

mbhave commented Dec 3, 2021

First, those active profiles are known prior to ConfigDataEnvironmentPostProcessor and ideally they should be set to environment prior to any environment post-processors executing.

The ConfigDataEnvironmentPostProcessor is responsible for collecting and merging active profiles from all sources in the right order. Setting them before ConfigDataEnvironmentPostProcessor leads to undesired ordering as seen in #28530.

Then, profiles set by annotation in this example actually would not be visible to post processor in spring.profiles.active property, in my test example it would be spring.profiles.active[0] property, so it's not really aligned with production code anyway

Yes, the indexed values were added to support #19556. One way to get them without using spring.profiles.active[0] is by using the Binder API Binder.get(env).bind("spring.profiles.active", Bindable.listOf(String.class));

Also, let's say I have 2 post processors. First one sets second active profile using environment.addActiveProfile("bar"). Second post-processor now has two profiles, one returned by environment.getActiveProfiles(), and another one set in spring.profiles.active[0] property. It's very inconsistent that way, each post-processor would have to check active profiles, check spring.profiles.active, and check spring.profiles.active[*], and then somehow merge them in the right order. It's quite a mess.

In this situation, if @ActiveProfiles were set on the Environment before the first post processor ran, they would have less precedence than the ones set by the post processor. It is very likely that profiles from @ActiveProfiles should take precedence, which was the regression reported it in #28530. I agree it can turn out to be messy but there isn't really a good way to preserve ordering if we set @ActiveProfiles on the environment directly.

We discussed it on the team call and decided to leave things as they are, given that there is a workaround to access @ActiveProfiles in an EnvironmentPostProcessor.

@mbhave mbhave closed this as completed Dec 3, 2021
@mbhave mbhave removed their assignment Dec 3, 2021
@mbhave mbhave added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 3, 2021
@lbreuss
Copy link

lbreuss commented Oct 2, 2023

And just in case, make sure, that the profiles you list in @ActiveProfile list individual profiles, not the comma separated spring.profiles.active property value.

@ActiveProfile({"test", "foo", "bar"})

instead of @ActiveProfile({"test,foo,bar"})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

6 participants