Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closes #1354 - fix(config-server/config-server-ui) Support enums in Collections in Highlighting #1355

Merged

Conversation

aaronweissler
Copy link
Member

@aaronweissler aaronweissler commented Mar 22, 2022

This change is Reviewable

heiko-holz
heiko-holz previously approved these changes Mar 22, 2022
Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aaronweissler)


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/yamlhighlighter/HighlightRulesMapController.java, line 109 at r1 (raw file):

            } else if (currentClass.equals(GenericActionSettings.class) && (field.getName()
                    .equals("value") || field.getName().equals("valueBody"))) {

do we maybe want to support equalsIgnoreCase in case someone writes valuebody, ValueBody or other forms?


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/yamlhighlighter/HighlightRulesMapController.java, line 139 at r1 (raw file):

    private List<String> extractEnumValues(Class<?> currentEnum) {
        List<String> enumValues = new ArrayList<>();

you could also use the Arrays.stream functions:

List<String> extractedEnums =  Arrays.stream(currentEnum.getFields()).map(field -> field.getName()).collect(Collectors.toList());

Code quote:

extractEnum

Copy link
Member Author

@aaronweissler aaronweissler left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aaronweissler)


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/yamlhighlighter/HighlightRulesMapController.java, line 109 at r1 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

do we maybe want to support equalsIgnoreCase in case someone writes valuebody, ValueBody or other forms?

The String "valueBody" here comes from the name of the field in the java class GenericActionSettings itself, so it will always be "valueBody", unless someone else refactors that code (which, related but not really your question, as of now would silently break the highlighter, but I don't know if it is worth it to stop that by for example checking that this if clause is reached twice, since it is rather unlikely to be changed?)


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/yamlhighlighter/HighlightRulesMapController.java, line 139 at r1 (raw file):

Previously, heiko-holz (Heiko Holz) wrote…

you could also use the Arrays.stream functions:

List<String> extractedEnums =  Arrays.stream(currentEnum.getFields()).map(field -> field.getName()).collect(Collectors.toList());

That's a good idea, I'll change it :)

Copy link
Contributor

@heiko-holz heiko-holz left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aaronweissler)


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/yamlhighlighter/HighlightRulesMapController.java, line 109 at r1 (raw file):

Previously, aaronweissler wrote…

The String "valueBody" here comes from the name of the field in the java class GenericActionSettings itself, so it will always be "valueBody", unless someone else refactors that code (which, related but not really your question, as of now would silently break the highlighter, but I don't know if it is worth it to stop that by for example checking that this if clause is reached twice, since it is rather unlikely to be changed?)

okay, then equals sounds more appropriate

@heiko-holz heiko-holz merged commit 7bf4688 into inspectIT:master Mar 23, 2022
@aaronweissler aaronweissler deleted the fix/collections-enum-highlighting branch June 7, 2022 12:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants