Skip to content
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

Introduce common IKey interface #564

Merged
merged 2 commits into from
Dec 11, 2019

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Dec 4, 2019

Currently JavaThemis deals only with asymmetric keys and thus has an AsymmetricKey class providing common implementation of storage and utilities for PrivateKey and PublicKey classes. However, we are going to introduce a new type of keys — SymmetricKey — which will need these
utilities as well. Since it's not an AsymmetricKey, let's refactor our class hierarchy a bit.

Extract declaration of the toByteArray() utility method into a new interface IKey. This interface will be implemented by all key classes of Themis and will provide common utilities. (E.g., base64 formatting may be added here in the future.)

Extract storage implementation into a new KeyBytes class. It is an abstract package-private class, intended to be a base class for all keys implemented by Themis. It provides key field to access key bytes directly and implements IKey interface. It also maintains the invariant that all valid keys must be non-null and not empty.

AsymmetricKey is left as a marker abstract class. It inherits storage and IKey implementation from KeyBytes. Plus, it still hosts the constants for asymmetric key types.

Checklist

  • Change is covered by automated tests
  • Benchmark results are attached (not applicable)
  • The coding guidelines are followed
  • Public API has proper documentation
  • Example projects and code samples are updated (not significant)
  • Changelog is updated

@ilammy ilammy added the W-JavaThemis ☕ Wrapper: Java, Java and Kotlin API label Dec 4, 2019
@ilammy ilammy mentioned this pull request Dec 4, 2019
6 tasks
@vixentael
Copy link
Contributor

Preliminary review: LGTM. Making single Interface makes total sense in Java world.

Currently JavaThemis deals only with asymmetric keys and thus has
an "AsymmetricKey" class providing common implementation of storage and
utilities for PrivateKey and PublicKey classes. However, we are going to
introduce a new type of keys -- "SymmetricKey" -- which will need these
utilities as well. Since it's not an AsymmetricKey, let's refactor our
class hierarchy a bit.

Extract declaration of the "toByteArray()" utility method into a new
interface "IKey". This interface will be implemented by all key classes
of Themis and will provide common utilities. (E.g., base64 formatting
may be added here in the future.)

Extract storage implementation into a new "KeyBytes" class. It is an
abstract package-private class, intended to be a base class for all
keys implemented by Themis. It provides "key" field to access key bytes
directly and implements IKey interface. It also maintains the invariant
that all valid keys must be non-null and not empty.

AsymmetricKey is left as a marker abstract class. It inherits storage
and IKey implementation from KeyBytes. Plus, it still hosts the
constants for asymmetric key types.
@ilammy ilammy force-pushed the java/refactor-key-hierarchy branch from 60ce827 to a3487c9 Compare December 6, 2019 09:14
@ilammy ilammy marked this pull request as ready for review December 6, 2019 09:14
Copy link
Collaborator

@Lagovas Lagovas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

KeyBytes keeps a copy of the original byte[] array and copies it again
in toByteArray() implementation. This is because Java arrays are always
mutable that we have to copy them in order to be sure that the user can
never modify the data stored in KeyBytes.
@ilammy ilammy merged commit 8fe48f5 into cossacklabs:master Dec 11, 2019
@ilammy ilammy deleted the java/refactor-key-hierarchy branch December 11, 2019 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
W-JavaThemis ☕ Wrapper: Java, Java and Kotlin API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants