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

Support AND operator in @Profile annotation [SPR-12458] #17063

Closed
spring-projects-issues opened this issue Nov 20, 2014 · 28 comments
Closed

Support AND operator in @Profile annotation [SPR-12458] #17063

spring-projects-issues opened this issue Nov 20, 2014 · 28 comments
Assignees
Labels
has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Nov 20, 2014

Artem opened SPR-12458 and commented

As of v4.1.2 @Profile annotation does not support AND operator, being OR by definition.

This significantly limits it's applicability in our project since we have profiles: PROD, TEST, CONFIG1, CONFIG2, etc. And we want to have beans autowired based in PROD && CONFIG1 style to be able to setup config1 in production and TEST && CONFIG1 to setup same profile for test.

The good example would be config being MQENABLED profile. Which in case of production will be set on @Configuration class with RabbitTemplate bean configuration, but for test we want to have our own implementation of AmqpTemplate (instead of RabbitTemplate).


Affects: 4.1.2

Issue Links:

18 votes, 30 watchers

@spring-projects-issues
Copy link
Collaborator Author

Vladimir Rozhkov commented

+1 for this. Had to write simple condition matcher to get job done:

public class AndProfilesCondition implements Condition {

    public static final String VALUE = "value";
    public static final String DEFAULT_PROFILE = "default";

    @Override
    public boolean matches(final ConditionContext context, final AnnotatedTypeMetadata metadata) {
        if (context.getEnvironment() == null) {
            return true;
        }
        MultiValueMap<String, Object> attrs = metadata.getAllAnnotationAttributes(Profile.class.getName());
        if (attrs == null) {
            return true;
        }
        String[] activeProfiles = context.getEnvironment().getActiveProfiles();
        String[] definedProfiles = (String[]) attrs.getFirst(VALUE);
        Set<String> allowedProfiles = new HashSet<>(1);
        Set<String> restrictedProfiles = new HashSet<>(1);
        for (String nextDefinedProfile : definedProfiles) {
            if (!nextDefinedProfile.isEmpty() && nextDefinedProfile.charAt(0) == '!') {
                restrictedProfiles.add(nextDefinedProfile.substring(1, nextDefinedProfile.length()));
                continue;
            }
            allowedProfiles.add(nextDefinedProfile);
        }

        for (String nextActiveProfile : activeProfiles) {
            if (DEFAULT_PROFILE.equals(nextActiveProfile) && allowedProfiles.isEmpty()) {
                continue;
            }
            if (!allowedProfiles.contains(nextActiveProfile) || restrictedProfiles.contains(nextActiveProfile) || allowedProfiles.size() != activeProfiles.length) {
                return false;
            }
        }
        return true;
    }

}

It will be nice to have expressions support in @Profile annotation.

@spring-projects-issues
Copy link
Collaborator Author

Stephen Bellchester commented

We are developing a spring boot application to run in WebSphere, with our test suite running in Jetty using @WebIntegrationTest.
We would like to be able to specify some beans (eg transaction managers) to be different between environments that run in Websphere, and those running on Jetty.
As the set of environments that are not running on WebSphere is the smaller and more static of the two, the neatest way to do this would be something like the following, which is currently not possible.

@Bean
@Profile({"!lcl && !ci"})
public JtaTransactionManager transactionManager() {
    return new WebSphereUowTransactionManager();
}

@spring-projects-issues
Copy link
Collaborator Author

Martin Wegner commented

I would use the acceptsProfiles() method for the condition implementation:

public class ProfileAndCondition implements Condition {

  @Override
  public boolean matches(final ConditionContext context, final AnnotatedTypeMetadata metadata) {
    final Environment contextEnvironment = context.getEnvironment();

    if (contextEnvironment == null) {
      return true;
    }

    final MultiValueMap<String, Object> allProfileAnnotations = metadata
        .getAllAnnotationAttributes(Profile.class.getName());

    if (allProfileAnnotations == null) {
      return true;
    }

    final List<?> allProfileAnnotationValues = allProfileAnnotations.get("value");

    if (allProfileAnnotationValues == null) {
      return true;
    }

    for (final Object value : allProfileAnnotationValues) {
      final String[] neededProfiles = (String[]) value;

      for (final String neededProfile : neededProfiles) {

        if (!contextEnvironment.acceptsProfiles(neededProfile)) {
          return false;
        }

      }

    }

    return true;
  }

}

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Indeed, implementing a custom condition along those lines is what I would recommend at this point.

As for 4.3, it's primarily a design question: How to express 'and' semantics in @Profile itself? Custom parsing of expressions would go a bit far... It might be more appropriate to introduce an enum on the annotation, indicating 'and' vs 'or' semantics for multiple profiles specified. In combination with making @Profile repeatable, this would deliver quite an amount of expressiveness already. Everything beyond that I would rather leave to custom programmatic condition implementations.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

FWIW, @Profile can already be repeated when declared as a meta-annotation on multiple composed annotations (even though it's not technically a repeatable annotation).

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

In the meantime, I'll rather leave this up to 5.0, along with other condition-related fine tuning there.

@spring-projects-issues
Copy link
Collaborator Author

Jared Jacobs commented

Spring Boot assumes AND semantics are desired when negated profiles are combined with non-negated profiles:

If both negated and non-negated profiles are specified for a single document, at least one non-negated profile must match and no negated profiles may match.

Spring Boot Reference Guide 24.6.3

I wish @Profile would behave the same way. This is the only situation where I’ve ever needed AND semantics.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Stéphane Nicoll, assigning this to you for further consideration... in particular in alignment with Boot 2.0.

@spring-projects-issues
Copy link
Collaborator Author

DJ Kulkarni commented

We have a similar requirement and our solution was to have a @Repeatable(Profiles.class) @Profile which otherwise has the same implementation as that of org.springframework.context.annotation.Profile

 
Then @Profiles does an AND while the default behavior for @Profile remains OR .
 
So now we can do-

@Profile("p1")
@Profile({"p2","p3")
@Bean

Which translates to-
p1 AND (p2 OR p3)

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

That sounds like an elegant custom solution, DJ Kulkarni.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Indeed... Let's consider rolling a variant of this into 5.1.

@spring-projects-issues
Copy link
Collaborator Author

Eric Deandrea commented

Let me see if our company rules allow us to contribute our solution back (DJ works with me...).

@spring-projects-issues
Copy link
Collaborator Author

Phil Webb commented

See also spring-projects/spring-boot#12469 in case we want to align solutions.

@spring-projects-issues
Copy link
Collaborator Author

Phil Webb commented

FWIW the multiple @Profile approach won't be much use for spring-projects/spring-boot#12469. That may or may not sway the design but I'd personally prefer it if the same syntax could be used in the @Profile annotation and the application.properties file.

I was thinking something like a simple + might do the trick. e.g:

@Profile("p1", "p2+p3")

Of course, that could break back compatibility if an existing profile happened to have a + in it.

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

FWIW, I don't like the repeatable approach because its semantic isn't as explicit as a profile expression. I wonder how likely + is going to be used in a profile name.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

We had similar discussions for tag support in JUnit 5.

The difference is that we luckily decided to introduce a set of reserved characters prior to releasing JUnit Platform 1.0. That enabled us to introduce a tag expression language at a later date.

! is already a reserved character for profiles in Spring.

So maybe it wouldn't be so bad to introduce + or & (my preference) as an additional reserved character.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

We could check for "p2+p3" as one active profile, and if not check for "p2" and "p3" as individually active in combination, having '+' with a special meaning but not as a strictly reserved character. That would preserve compatibility with existing arrangements where people might have mimicked an 'and' operator through such plus-named profiles.

@spring-projects-issues
Copy link
Collaborator Author

Taylor S. Wicksell commented

Like Sam, I also prefer & over \+, as \+ typically indicates the OR operator.

However, I do hope that this expression syntax stays isolated to the @Profile annotation. Would it be reasonable to add methods to Environment along the lines of acceptsAllProfiles and acceptsAnyProfiles? This way future integrations may exclusively use the AND or OR semantics depending on what makes the most sense. Had those existed prior to Boot 2.0's release, I believe a strong case could've been made for using acceptsAllProfiles in the case of yaml document loading.

@spring-projects-issues
Copy link
Collaborator Author

Jamie Cramb commented

Will this be supported for XML config as well?

Right now we have to do the following:

<beans profile="profileA">
    <beans profile="profileB">
        <!-- Stuff for when profile A & B are active -->
    </beans>
<beans>

<beans profile="profileC">
    <beans profile="!profileD">
        <!-- Stuff for when profile C is active but profile D is not -->
    </beans>
<beans>

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

Yes

@spring-projects-issues
Copy link
Collaborator Author

David Harrigan commented

At the risk of hijacking this thread, would it also be possible to add OR support, something like

@Profile("p1|p2")

Presently, in Spring Boot 2, for a custom Spring Logback configuration, we are unable to support allowing multiple configurations that are logically OR'ed, e.g., if we want to output console log output for Profile A or Profile B, we find we can't do that with the current support in Spring, i.e., this is what we have:

<root level="INFO">
    <springProfile name="local">
        <appender-ref ref="CONSOLE" />
    </springProfile>
    <springProfile name="!local">
        <appender-ref ref="LOGSTASH" />
    </springProfile>
    <appender-ref ref="FILE" />
</root>

If we have a @Profile("test"), then output would go to the LOGSTASH appender. You may ask why not have springProfile called "test" and have a appender-ref for that, but that won't work, since for every other environment apart from local and test, we do want the LOGSTASH appender to be used. So in the absence of defining a spring.profiles.active for each deployment and each environment, we have a catch-all that switches on the logstash appender since it's appropriate to do so as the software is not running locally or under test.

Ideally, we would love to do something like:

<root level="INFO">
    <springProfile name="local|test">
        <appender-ref ref="CONSOLE" />
    </springProfile>
    <springProfile name="!local&!test">
        <appender-ref ref="LOGSTASH" />
    </springProfile>
    <appender-ref ref="FILE" />
</root>

The implementation of doing the ANDing/ORing is done isn't much of a concern for us, simply having the ability to do it would be awesome. I hope that explains it clearly for our requirement.

Thank you.

-=david=-

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

 However, I do hope that this expression syntax stays isolated to the @Profile annotation

Taylor S. Wicksell can you please expand why? The handling of the ! operator is handled in acceptProfiles at the moment so it's likely that any format we come up with should be parsed by a similar method on the Environment. At the moment the ProfileCondition is trivial as it relies on the current method and I think that's fair.

I am expecting the sub-documents loading feature in YAML to use a format of some sort and I expect it to be the same as the one you could use in @Profile. Am I missing something?
 

@spring-projects-issues
Copy link
Collaborator Author

Taylor S. Wicksell commented

@snicoll I'm just thinking the presence of more complicated boolean operators in the context of YAML loading makes the properties file unintelligible. While I'm happy to use that in @Profile (typically on a single bean), I worry about YAML files like: 

spring.profiles.active: p1,p2
&mdash;
spring.profiles: p1 | !p2

someProp: v1

&mdash;
spring.profiles: !p2 & p1

someProp: v2

What is the value of someProp in the above? The more profiles you add the worse this can be. And you have to remember that the last matching set of profiles wins in terms of the property being set. 

What I do hope to see from Boot is: 

spring.profiles.active: p1,p2
&mdash;
spring.profiles.any: p1,p2
someProp: v1

&mdash;
spring.profiles.all: p1,p2
someProp: v2

Less flexible to be sure, but I think a bit more readable when you have to scan a document full of these and try to reason about what properties are going to be loaded for a given set of profiles. Regardless of your implementation with @Profile I'm hoping Boot opts for a lesser version of that functionality for the sake of a simpler usability story with regards to the YAML feature. 

@spring-projects-issues
Copy link
Collaborator Author

Phil Webb commented

I've had a crack at solving this in my branch here. I know that Juergen Hoeller commented that "Custom parsing of expressions would go a bit far..." but I think it might be the cleanest way to support it in the end. I've tried to keep the parser really simple. It supports & | and ! for local and, or and not expressions. There's no support operator precedence, so you can't do "A & B | C" but you can use parenthesis to make more complex expressions. e.g. "(A & B) | C". The parser itself is only 150 lines or so.

I've also hidden the parser behind a Profiles interface so people can plug in their own parsing if they wish.

The nice thing about the parser approach is we automatically some consistency. You can use it in an annotation (@Profile("A & B")) or in XML <beans profiles="A & B">. With a small change to Spring Boot we can support the same syntax in application.yml.

Taylor S. Wicksell, I know you were originally looking for some different properties to keep the YAML sane but I think I actually the expression syntax personally. I also think that spring.profiles.any and spring.profiles.all might be too restrictive. With the expression you can do things like this:

&mdash;
spring.profiles: production & (us-east | us-west)
...
&mdash;
spring.profiles: production & (eu-central | eu-west)
...
&mdash;
spring.profiles: !producton
...

What do people think? Is it adding too much complexity? Does it go far enough?

@spring-projects-issues
Copy link
Collaborator Author

David Harrigan commented

Thank you Phil, I like it :)

@spring-projects-issues
Copy link
Collaborator Author

Eric Deandrea commented

I like it! Something we've been waiting for now for a long time! Nice job!

 

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

Phil Webb thank you very much, I like it and this addresses a significant amount of my concerns and raise one I hadn't identified so far.

I've polished your initial submission in e168c35 and improved the support in fa83bcbf.

& is a reserved keyword in XML so the profile expression there is quite weird

<beans xmlns="http://www.springframework.org/schema/beans"
	   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
	   xsi:schemaLocation="http://www.springframework.org/schema/beans
		http://www.springframework.org/schema/beans/spring-beans-3.1.xsd"
	   profile="prod&amp;dev">

	<bean id="foo" class="java.lang.String"/>
</beans>

Another element I hadn't noticed so far is that the XML parser uses spaces, commas or semi-colons to separate profiles (i.e. profile="prod,dev"> means that either prod or dev should be active). This is a problem when you use an expression the way it is described at the moment as the space is interpreted as a separate profile, so prod &amp; dev is parsed as prod or & or dev.

To avoid the espace of the logical AND operator, I've renamed it to + (as already discussed above). As for the XML parsing issue, I've updated the XSD to mention that expressions must not have spaces in them.

We had this plan of checking if a profile had the same name as an expression for backward compatibility reason. Looking at the current arrangement, that would probably increase the complexity a lot. However, not being able to parse profiles the same way as before (with only ! handling) can be problematic for those willing to keep this simple arrangement for the time being.

I'll do one more pass with Juergen and report here.

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

So we've decided to not support profile expressions in XML, leaving acceptProfiles(String...) as it is in a deprecated fashion. This means the format as originaly proposed will be available in a 5.1 snapshot in half an hour or so. 

Thanks a lot for the help on this Phil!

@spring-projects-issues spring-projects-issues added in: core Issues in core modules (aop, beans, core, context, expression) has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 5.1 RC1 milestone Jan 11, 2019
sbrannen added a commit that referenced this issue Apr 25, 2023
Environment.acceptsProfiles(String...) was deprecated in 5.1 in
conjunction with gh-17063 which introduced a new
acceptsProfiles(Profiles) method to replace it. The deprecated method
only supports OR semantics; whereas, the new method supports profile
expressions. Thus, the goal was to encourage people to use the more
powerful profile expressions instead of the limited OR support with
profile names.

However, there are use cases where it is difficult (if not impossible)
to provide a Profiles instance, and there are use cases where it is
simply preferable to provide profile expressions directly as strings.

To address these issues, this commit introduces a new matchesProfiles()
method in Environment that accepts a var-args list of profile
expressions.

Closes gh-30206
sbrannen added a commit that referenced this issue Apr 25, 2023
Environment.acceptsProfiles(String...) was deprecated in 5.1 in
conjunction with gh-17063 which introduced a new
acceptsProfiles(Profiles) method to replace it. The deprecated method
only supports OR semantics; whereas, the new method supports profile
expressions. Thus, the goal was to encourage people to use the more
powerful profile expressions instead of the limited OR support with
profile names.

However, there are use cases where it is difficult (if not impossible)
to provide a Profiles instance, and there are use cases where it is
simply preferable to provide profile expressions directly as strings.

To address these issues, this commit introduces a new matchesProfiles()
method in Environment that accepts a var-args list of profile
expressions.

See gh-30206
Closes gh-30226
mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
Environment.acceptsProfiles(String...) was deprecated in 5.1 in
conjunction with spring-projectsgh-17063 which introduced a new
acceptsProfiles(Profiles) method to replace it. The deprecated method
only supports OR semantics; whereas, the new method supports profile
expressions. Thus, the goal was to encourage people to use the more
powerful profile expressions instead of the limited OR support with
profile names.

However, there are use cases where it is difficult (if not impossible)
to provide a Profiles instance, and there are use cases where it is
simply preferable to provide profile expressions directly as strings.

To address these issues, this commit introduces a new matchesProfiles()
method in Environment that accepts a var-args list of profile
expressions.

Closes spring-projectsgh-30206
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants