Conversation
|
PR comment is missing. |
| public class AliasCreateResponse { | ||
| private String alias; | ||
|
|
||
| public AliasCreateResponse(String alias) { |
There was a problem hiding this comment.
Should be covered by @RequiredArgsConstructor which is a part of @Data annotation.
Reason why It's not there is the absence of @NotNull annotation.
|
My bad. Mistakenly assumed I added it. Will consider this case in future. |
| @@ -0,0 +1,15 @@ | |||
| package com.verygood.security.coding.exception; | |||
|
|
|||
| public class CipherDecryptException extends RuntimeException { | |||
There was a problem hiding this comment.
Do we really need 2 identical exceptions?
Maybe we can pass MODE in message.
| return encrypt(strToEncrypt, secret, DEFAULT_ALGORITHM); | ||
| } | ||
|
|
||
| public static String encrypt(String whatToEncrypt, String secret, String algorithm) throws CipherEncryptException { |
There was a problem hiding this comment.
encrypt and decrypt have a lot of duplication.
Would be nice to have part of It extracted in separate method.
| private AliasesService aliasesService = new AliasesServiceImpl(new MockAliasRepository()); | ||
|
|
||
| @Test | ||
| public void inputIsEncryted() { |
There was a problem hiding this comment.
Not sure we need @RunWith(SpringRunner.class) here.
We don't need Spring context to test AliasesService logic.
|
All remarks are reasonable. Thanks a lot for sharing your thoughts on this. |
|
In general @rsavko no need to resolve PR comments. |
|
You mentioned a lot of great ideas. Thanks a lot, I highly appreciate this! 😄 |
2 participants
No description provided.