Closes #1290 - Add an endpoint to the Configuration Server to obtain a configuration documentation #1314
Conversation
Actually get Configuration based on Mapping instead of based on Mapping's Attributes
Throw JsonProcessingException in ConfigParser to be able to handle it better in ConfigurationController.
There was a problem hiding this comment.
Reviewable status: 0 of 8 files reviewed, 1 unresolved discussion (waiting on @MariusBrill)
components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java, line 90 at r1 (raw file):
String mappingName = "name"; AgentMapping agentMapping = AgentMapping.builder().build();
I could not mock these two easily, because they are final classes due to their Value Annotation and mockito only supports mocking final classes when using mockito-inline instead (https://javadoc.io/doc/org.mockito/mockito-core/2.23.0/org/mockito/Mockito.html#Mocking_Final) and I did not want to add that for something rather small like this, but of course I could if you think it would be worth it :)
There was a problem hiding this comment.
Reviewable status: 0 of 8 files reviewed, 1 unresolved discussion (waiting on @aaronweissler and @MariusBrill)
components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java, line 90 at r1 (raw file):
Previously, aaronweissler wrote…
I could not mock these two easily, because they are final classes due to their
ValueAnnotation and mockito only supports mocking final classes when using mockito-inline instead (https://javadoc.io/doc/org.mockito/mockito-core/2.23.0/org/mockito/Mockito.html#Mocking_Final) and I did not want to add that for something rather small like this, but of course I could if you think it would be worth it :)
We agreed to not use mockito-inline as as it does not work with IBM JDK8.
There was a problem hiding this comment.
Reviewable status: 0 of 8 files reviewed, all discussions resolved (waiting on @MariusBrill)
components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java, line 90 at r1 (raw file):
Previously, heiko-holz (Heiko Holz) wrote…
We agreed to not use
mockito-inlineas as it does not work with IBM JDK8.
Okay, thank you! Then that question is settled.
There was a problem hiding this comment.
Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aaronweissler)
components/inspectit-ocelot-configdocsgenerator/build.gradle, line 33 at r1 (raw file):
project(':inspectit-ocelot-config'), 'ch.qos.logback:logback-classic:0.9.26', 'org.apache.commons:commons-lang3:3.12.0',
Just a reminder to update THIRD-PARTY-LICENCES.txt :)
components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java, line 97 at r1 (raw file):
/** * Fetches the configuration given an AgentMapping the configuration should be for.
This sentence is not quite right - Maybe something like "Takes an instance of AgentMapping and returns the befitting instance of AgentConfiguration" ?
components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java, line 101 at r1 (raw file):
* @param agentMapping AgentMapping for which the configuration should be returned. * * @return The configuration for this AgentMapping or null if no configuration for that mapping found.
Here you could also add that the first matching instance of AgentConfiguration is returned in case there are more than one matching instances.
components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java, line 81 at r1 (raw file):
} }
Maybe we could here add a third test case for the Situation that a JsonProcessingException is thrown?
You can provoke an exception in configDocsGenerator.generateConfigDocs with a when-thenThrow method in Mockito
aaronweissler
force-pushed
the
feature/config-server-config-doc-endpoint
branch
from
f12127c to
2531144
Compare
Codecov Report
@@ Coverage Diff @@
## master #1314 +/- ##
============================================
- Coverage 83.00% 80.80% -2.20%
- Complexity 1736 2026 +290
============================================
Files 174 204 +30
Lines 5366 6460 +1094
Branches 650 769 +119
============================================
+ Hits 4454 5220 +766
- Misses 675 950 +275
- Partials 237 290 +53
|
There was a problem hiding this comment.
Reviewable status: 6 of 9 files reviewed, 4 unresolved discussions (waiting on @MariusBrill)
components/inspectit-ocelot-configdocsgenerator/build.gradle, line 33 at r1 (raw file):
Previously, MariusBrill (Marius Brill) wrote…
Just a reminder to update THIRD-PARTY-LICENCES.txt :)
Done.
components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java, line 97 at r1 (raw file):
Previously, MariusBrill (Marius Brill) wrote…
This sentence is not quite right - Maybe something like "Takes an instance of AgentMapping and returns the befitting instance of AgentConfiguration" ?
Done.
components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java, line 101 at r1 (raw file):
Previously, MariusBrill (Marius Brill) wrote…
Here you could also add that the first matching instance of AgentConfiguration is returned in case there are more than one matching instances.
Done.
As talked about, there can only be one configuration per Mapping.
components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java, line 81 at r1 (raw file):
Previously, MariusBrill (Marius Brill) wrote…
Maybe we could here add a third test case for the Situation that a JsonProcessingException is thrown?
You can provoke an exception in configDocsGenerator.generateConfigDocs with a when-thenThrow method in Mockito
Done.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @aaronweissler)
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @aaronweissler)
This change is