Skip to content

Process profiles in filter phase#2548

Open
MikeEdgar wants to merge 1 commit into
smallrye:mainfrom
MikeEdgar:quarkus-53479
Open

Process profiles in filter phase#2548
MikeEdgar wants to merge 1 commit into
smallrye:mainfrom
MikeEdgar:quarkus-53479

Conversation

@MikeEdgar
Copy link
Copy Markdown
Member

@MikeEdgar MikeEdgar commented Apr 24, 2026

For quarkusio/quarkus#53479

This change moves the handling of operation profiles to the filter phase, allowing operations contributed to the model from a static file or OASModelReader to be included/excluded according to any given configurations.

@MikeEdgar MikeEdgar added this to the 4.3.2 milestone Apr 24, 2026
@MikeEdgar MikeEdgar marked this pull request as ready for review April 24, 2026 14:05
Signed-off-by: Michael Edgar <michael@xlate.io>
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Member

@phillip-kruger phillip-kruger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MikeEdgar
Copy link
Copy Markdown
Member Author

@Azquelt I don't recall if you were curious to look at this one. I'll be looking to do a release in the next day or two, so please let me know if you'd like some additional time to go over it.

Comment on lines +100 to +113
public static boolean includedProfile(Extensible<?> extensible, Set<String> included, Set<String> excluded) {
Set<String> profiles = getProfiles(extensible);

if (!excluded.isEmpty()) {
return excluded.stream().noneMatch(profiles::contains);
}

if (included.isEmpty()) {
return true;
}

return included.stream().anyMatch(profiles::contains);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit sketchy. Can the user specify both and include list and an exclude list?

It's odd that the include list is ignored completely if the exclude list is specified.

I realise this logic has not changed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I agree on that. Maybe something like this would be better (plus updating the test). Both include and exclude could be specified.

public static boolean includedProfile(Extensible<?> extensible, Set<String> included, Set<String> excluded) {
    Set<String> profiles = getProfiles(extensible);

    if (profiles.isEmpty()) {
        // Include an extensible without any profiles attached when none
        // are explicitly included via configuration.
        return included.isEmpty();
    }

    if (included.stream().anyMatch(profiles::contains)) {
        return true;
    }

    if (excluded.stream().anyMatch(profiles::contains)) {
        // Exclude the extensible if it has configured exclusions and also
        // does not have any configured inclusions. This also means that
        // inclusion overrides exclusion if a profile is configured for both.
        return false;
    }

    return included.isEmpty();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants