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
Allow recursive profile group references #24327
Conversation
@@ -128,7 +130,7 @@ private boolean hasExplicit(Supplier<String[]> supplier, String propertyValue, S | |||
} | |||
List<String> reversed = new ArrayList<>(list); | |||
Collections.reverse(reversed); | |||
return Collections.unmodifiableList(reversed); | |||
return reversed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, we could do a stream/filter approach, but the unmodifiable list seemed not needed as the result is only used above and cannot be modified from the outside.
I'm not sure if just removing the current profile is enough. I think we'd end up with the same issue if we had a configuration like:
Maybe we should throw an exception in this case since it is an invalid configuration. |
@mbhave Good catch - I covered the additional case now. Which sort of exception do you want to be thrown here? |
I think an |
Update `Profiles` group expansion logic to fail if recursive references are found. See gh-24327
Thanks once again @dreis2211! I've merged this into master with a minor update to make the message catch duplicates in the expanded set as well as the stack. |
Reopening because I just realized that a user might want to have the same expansion in different groups:
|
After some more thought, I think it's best if we tolerate duplicates rather than throwing an exception. I've changed things again in commit bef5fe2. @mbhave @dreis2211 I'd appreciate a review if you have time. (I'll leave the issue open for now) |
Looks good to me, @philwebb |
Hi,
this PR fixes #24319 by removing the current profile from group profiles in order to avoid an endless loop when expanding/flattening the profile list.
Cheers,
Christoph