techtask/Separation-of-credentials-from-endpoints #35
Conversation
| companion object { | ||
| val LOGGER: Logger = LoggerFactory.getLogger(ConfigManager::class.java) | ||
| const val ID = "id"; |
| return endpoint | ||
| } | ||
|
|
||
| private fun findJsonObjectById(id: String?, array: JsonArray): Optional<JsonObject> { |
There was a problem hiding this comment.
You can use Extension Functions this.
https://kotlinlang.org/docs/reference/extensions.html#extension-functions
…ttps://github.com/Cognifide/cogboard into techtask/Separation-of-credentials-from-endpoints
Done. Code refactored and unit tests added. |
|
|
||
| override fun start() { | ||
| endpoints = config().getJsonArray("endpoints") | ||
| endpoints = config().getJsonArray(ENDPOINTS) |
There was a problem hiding this comment.
this is know confusing - in my opinion there is no need to store endpoints, credentials and most importantly endpoint here. Look below to see how I see it:
| endpointId == it.getString("id") | ||
| }.findFirst() | ||
| .orElse(JsonObject())) | ||
| config.put(CogboardConstants.PROP_ENDPOINT, endpoint.readById(endpointId))} |
There was a problem hiding this comment.
I would like to see it like this:
config.put(CogboardConstants.PROP_ENDPOINT, Endpoint.from(config(), endpointId).asJson())
|
|
||
| companion object { | ||
| val LOGGER: Logger = LoggerFactory.getLogger(ConfigManager::class.java) | ||
| const val CREDENTIALS = "credentials" |
There was a problem hiding this comment.
This should be moved to Endpoint class
| @@ -24,8 +24,8 @@ class GetEndpoints : RoutingHandlerFactory { | |||
| copy.stream().forEach { | |||
| if (it is JsonObject) { | |||
| it.remove("url") | |||
| it.remove("user") | |||
| it.remove("password") | |||
| it.remove("publicUrl") | |||
| import org.junit.jupiter.api.Assertions.assertFalse | ||
| import org.junit.jupiter.api.Test | ||
|
|
||
| internal class EndpointTest { |
There was a problem hiding this comment.
I really like that You introduced JUnit to the project !
| @@ -0,0 +1,14 @@ | |||
| [ | |||
There was a problem hiding this comment.
merge credentials-test.json and endpoints-test.json so it is presented as in actual code.
|
|
||
| @Test | ||
| fun shouldRemoveCredentialsProp() { | ||
| val validEndpoint = Endpoint(endpoints, credentials,"validEndpoint").asJson() |
There was a problem hiding this comment.
next time please use @beforeeach for initialize e.g. endpoint once.
private lateinit var validEndpoint: JsonObject
private lateinit var invalidEndpoint: JsonObject
@BeforeEach
fun init() {
validEndpoint = Endpoint(endpoints, credentials, "validEndpoint").asJson()
invalidEndpoint = Endpoint(endpoints, credentials, "invalidEndpoint").asJson()
}
…als-from-endpoints techtask/Separation-of-credentials-from-endpoints
michalskrzypek
deleted the
techtask/Separation-of-credentials-from-endpoints
branch
No description provided.