interledger-deprecated / java-crypto-conditions

WE'VE MOVED: This project has moved to Hyperledger Quilt
https://github.com/hyperledger/quilt/tree/master/crypto-conditions
Apache License 2.0
5 stars 12 forks source link

Feature/immutable-crypto-conditions #68

Closed sappenin closed 7 years ago

sappenin commented 7 years ago

This PR introduces immutable Conditions and Fulfillments. Previously, many of the internals of each Condition and Fulfillment contained data that was lazily-loaded or initialized. Because of this mutability, certain thread-safety aspects of the code base were in doubt. Additionally, equals/hashcode implementations were sometimes difficult to make consistent because they were often based upon lazily initialized data (equals/hashcode/toString is often called anyway throughout the course of using a Condition or Fulfillment, so the lazy optimizations were likely not adding value in many cases anyway).

After performing some basic timing tests against the 0.2.0-SNAPSHOT construction and encoding times vs the time taken to preemptively compute things like fingerprints (the model used in this PR), there isn't much of a performance savings gained by lazily computing certain pieces of information. Thus, this proposed implementation removes this in favor of final attributes in each class.

This PR includes the following additional changes, and also addresses #66, #61, #59, #57, #55, #54,#47, and #43.

codecov[bot] commented 7 years ago

Codecov Report

Merging #68 into development will increase coverage by 31.52%. The diff coverage is 65.54%.

Impacted file tree graph

@@                Coverage Diff                 @@
##             development      #68       +/-   ##
==================================================
+ Coverage          37.89%   69.41%   +31.52%     
- Complexity            97      206      +109     
==================================================
  Files                 24       25        +1     
  Lines                863     1030      +167     
  Branches              86      116       +30     
==================================================
+ Hits                 327      715      +388     
+ Misses               507      256      -251     
- Partials              29       59       +30
Impacted Files Coverage Δ Complexity Δ
...erledger/cryptoconditions/der/DerOutputStream.java 100% <ø> (+36.36%) 8 <0> (ø) :arrow_down:
...ger/cryptoconditions/utils/UnsignedBigInteger.java 50% <ø> (ø) 2 <0> (?)
...erledger/cryptoconditions/NamedInformationUri.java 87.75% <ø> (ø) 12 <0> (?)
...ledger/cryptoconditions/CryptoConditionReader.java 79.62% <100%> (ø) 20 <0> (?)
...dger/cryptoconditions/PreimageSha256Condition.java 100% <100%> (ø) 4 <3> (?)
...rg/interledger/cryptoconditions/ConditionBase.java 33.33% <25.92%> (-16.67%) 5 <5> (-1)
...dger/cryptoconditions/PrefixSha256Fulfillment.java 36.36% <36.36%> (ø) 6 <6> (?)
...rledger/cryptoconditions/RsaSha256Fulfillment.java 38.29% <38.29%> (ø) 6 <6> (?)
...ger/cryptoconditions/Ed25519Sha256Fulfillment.java 38.29% <38.29%> (ø) 5 <5> (?)
...r/cryptoconditions/ThresholdSha256Fulfillment.java 39.13% <39.13%> (ø) 7 <7> (?)
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c07ba6d...7ea3d28. Read the comment docs.