Feature/objects 1036 #1
Conversation
smartcosmos-dao-user-details is a rather pointless repositories since it contains just 2 classes, that will hardly exist without the rest of this repository
| <groupId>org.projectlombok</groupId> | ||
| <artifactId>lombok</artifactId> | ||
| <scope>provided</scope> | ||
| </dependency> |
There was a problem hiding this comment.
Why provided scope? This is for builds and will prevent transient dependencies from recognizing this as a required dependency. That means you are at the mercy of whoever else declares these dependencies and the version they declare.
There was a problem hiding this comment.
Don't ask me why, but lombok is always and everywhere declared with provided scope, probably because it's only relevant during compile time when Lombok does its magic.
| public static final String CODE = "code"; | ||
| public static final String MESSAGE = "message"; | ||
|
|
||
| private static final int VERSION = 1; |
There was a problem hiding this comment.
this should be VERSION_1, VERSION_ONE or VERSION_01 otherwise it's a useless definition in the future since it will have to change anyway.
This is intended to be a forward thinking compatibility feature where the version number will change over time and can be used to route requests. Even though this is a response it should be consistent with our other implementations
| * @return the AuthenticateUserService bean | ||
| */ | ||
| @Bean | ||
| @ConditionalOnMissingBean |
There was a problem hiding this comment.
We have to be careful here? If a developer uses their own auto-configuration this could engage before that causing confusion. I suggest a log.info("Default authentication service being started") or something similar to indicate in the logs that we are engaging on their behalf by default.
There was a problem hiding this comment.
If there is no value in the annotation, it basically says use me if you don't have anything of my type.
I'll add the log message.
| public static final String PASSWORD_HASH = "passwordHash"; | ||
| public static final String AUTHORITIES = "authorities"; | ||
|
|
||
| private static final int VERSION = 1; |
| @@ -0,0 +1,71 @@ | |||
| package net.smartcosmos.userdetails.autoconfigure; | |||
| .findAny() | ||
| .get(); | ||
|
|
||
| Assert.assertNotNull("the JSON message converter must not be null", |
There was a problem hiding this comment.
That is nice to ensure but if someone is messing around with this we have big problems.
There was a problem hiding this comment.
This must be a left-over from copying some previous code. To be honest, I have no idea why it is there or why we would need this in a test class.
We don't need an assertion to realize something might go wrong if this guy is missing.
What changes were proposed in this pull request?
Provides the common code for all user details services.
Note:
This also includes all classes that previously were part of smartcosmos-dao-user-details, because after some clean up this repository contained only two classes that actually are not independent from all the classes in this repository. Thus, this split eventually made no sense.
How is this patch documented?
How was this patch tested?
Depends On
Nothing