Highlight enums with String values correctly

[aaronweissler]

Closes #1436

---

This change is 

[heiko-holz]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aaronweissler)

---

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

    private List<String> extractEnumValues(Class<?> currentEnum) {
        List<String> allFields = Arrays.stream(currentEnum.getDeclaredFields()).map(Field::getName).collect(Collectors.toList());
        if (allFields.contains("values")){

The field values is specific to the TransportProtocol.

In case of the TransportProtocol it would also be sufficient to just map to the toString method instead of the getName.

Assuming that the other Enums do not override the toString method to express something different, we can use

 private List<String> extractEnumValues(Class<?> currentEnum) {
        return Arrays.stream(currentEnum.getEnumConstants()).map(Object::toString).collect(Collectors.toList());
    }
``

(in order to make the test pass, the order of the expected values for `TransportProtocol` need to be adjusted in `HighlightRulesMapControllerIntTest`

Alternatively, we can also add the edge case of the `TransportProcotol` directly into the `extractEnumValues` method:

private List extractEnumValues(Class currentEnum) { Class> clazz = (Class>) currentEnum;
if (currentEnum.equals(TransportProtocol.class)) {
return Arrays.stream(clazz.getEnumConstants()).map(Enum::toString).collect(Collectors.toList());
} else {
return Arrays.stream(clazz.getEnumConstants()).map(Enum::name).collect(Collectors.toList());
}
}

[codecov]

Codecov Report

Merging #1488 (b962b19) into master (945412d) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Current head b962b19 differs from pull request most recent head 80490dd. Consider uploading reports for the commit 80490dd to get more accurate results

@@             Coverage Diff              @@
##             master    #1488      +/-   ##
============================================
+ Coverage     79.26%   79.27%   +0.01%     
  Complexity     2254     2254              
============================================
  Files           231      230       -1     
  Lines          7305     7304       -1     
  Branches        867      867              
============================================
  Hits           5790     5790              
+ Misses         1157     1156       -1     
  Partials        358      358              

[aaronweissler]

Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @heiko-holz)

---

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

Previously, heiko-holz (Heiko Holz) wrote…

The field values is specific to the TransportProtocol.

In case of the TransportProtocol it would also be sufficient to just map to the toString method instead of the getName.

Assuming that the other Enums do not override the toString method to express something different, we can use

 private List<String> extractEnumValues(Class<?> currentEnum) {
        return Arrays.stream(currentEnum.getEnumConstants()).map(Object::toString).collect(Collectors.toList());
    }
``

(in order to make the test pass, the order of the expected values for `TransportProtocol` need to be adjusted in `HighlightRulesMapControllerIntTest`

Alternatively, we can also add the edge case of the `TransportProcotol` directly into the `extractEnumValues` method:

private List extractEnumValues(Class currentEnum) { Class> clazz = (Class>) currentEnum;
if (currentEnum.equals(TransportProtocol.class)) {
return Arrays.stream(clazz.getEnumConstants()).map(Enum::toString).collect(Collectors.toList());
} else {
return Arrays.stream(clazz.getEnumConstants()).map(Enum::name).collect(Collectors.toList());
}
}

Yes, exactly this problem/limitation I would have mentioned when turning it from draft to reviewable, that right now it assumes that any other Enums using this functionality would also save their values in a values field.

Simply using toString seems like a really good idea to me though, since it should be correct for any Enums used in this context, since the values for configurations are also defined as Strings.
And I like it more than defining an exception specifically for TransportProtocol, since it would just work for new Enums working the same way without additional code changes.

[heiko-holz]

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: 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 170 at r1 (raw file):

Previously, aaronweissler wrote…

Yes, exactly this problem/limitation I would have mentioned when turning it from draft to reviewable, that right now it assumes that any other Enums using this functionality would also save their values in a values field.

Simply using toString seems like a really good idea to me though, since it should be correct for any Enums used in this context, since the values for configurations are also defined as Strings.
And I like it more than defining an exception specifically for TransportProtocol, since it would just work for new Enums working the same way without additional code changes.

Yep, I also like the implementation with toString