-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clean up base64 processing #695
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for the PR. It would be nice to throw KuraRuntimeException instead of so generic RuntimeException as in KRE, you can specify error codes.
*/ | ||
return new String(Base64.encodeBase64(encryptedBytes, false), "UTF-8"); | ||
} catch (UnsupportedEncodingException e) { | ||
throw new RuntimeException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of throwing generic Runtime Exception, it would be better off with KuraRuntimeException, in which you can specify the KuraErrorCode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a big difference, the exception will never ever be thrown and the whole try-catch will be gone with java 7 anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if that should be a blocker, then I don't really care.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace with String(byte[] bytes, Charset charset) which does not throw exceptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Since Kura 3.0.0 will only target Java 8, why not thinking of using only base64 java 8 primitives? Indirectly it was already used. And we don't need the dependency on commons.net. |
fddd979
to
1a50206
Compare
Updated to use Java 8 only. Rebased. |
1a50206
to
ee30fad
Compare
This change re-implements base64 processing by using standard Java 8 functionality. In addition a unit test was added to verify the same output as the old implementation. Signed-off-by: Jens Reimann <jreimann@redhat.com>
ee30fad
to
9a0743a
Compare
@MMaiero please vote |
This change forwards base 64 encoding to the already existing
dependency commons-net.
By re-using code from commons-net the reflection based method invocation
can be replaced by a proper method call.
In addition a unit test was added to verify the same output as the old
implementation.
Signed-off-by: Jens Reimann jreimann@redhat.com