Skip to content

Commit

Permalink
[PR COMMENTS] Logging format and function rename
Browse files Browse the repository at this point in the history
Use a more compact version of string formatting
in log messages

Rename isMetadataEquals to matchesRelevantPubKey
which is more descriptive of the actual check
  • Loading branch information
julianknutsen committed Nov 13, 2019
1 parent 5ae9dd1 commit 5d35d08
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ public boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageEn
ProtectedStorageEntry storedEntry = map.get(hashOfPayload);

// If we have already seen an Entry with the same hash, verify the metadata is equal
if (storedEntry != null && !protectedStorageEntry.isMetadataEquals(storedEntry))
if (storedEntry != null && !protectedStorageEntry.matchesRelevantPubKey(storedEntry))
return false;

// This is an updated entry. Record it and signal listeners.
Expand Down Expand Up @@ -504,7 +504,7 @@ public boolean remove(ProtectedStorageEntry protectedStorageEntry,
return false;

// If we have already seen an Entry with the same hash, verify the metadata is the same
if (!protectedStorageEntry.isMetadataEquals(storedEntry))
if (!protectedStorageEntry.matchesRelevantPubKey(storedEntry))
return false;

// Valid remove entry, do the remove and signal listeners
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ public boolean isValidForAddOperation() {
if (mailboxStoragePayload != null && mailboxStoragePayload.getOwnerPubKey() != null)
res2 = Utilities.encodeToHex(mailboxStoragePayload.getSenderPubKeyForAddOperation().getEncoded(),true);

log.warn(String.format("ProtectedMailboxStorageEntry::isValidForAddOperation() failed. " +
"Entry owner does not match sender key in payload:\nProtectedStorageEntry=%s\n" +
"SenderPubKeyForAddOperation=%s", res1, res2));
log.warn("ProtectedMailboxStorageEntry::isValidForAddOperation() failed. " +
"Entry owner does not match sender key in payload:\nProtectedStorageEntry=%{}\n" +
"SenderPubKeyForAddOperation=%{}", res1, res2);
}

return result;
Expand Down Expand Up @@ -137,9 +137,9 @@ public boolean isValidForRemoveOperation() {
if (mailboxStoragePayload != null && mailboxStoragePayload.getOwnerPubKey() != null)
res2 = Utilities.encodeToHex(mailboxStoragePayload.getOwnerPubKey().getEncoded(), true);

log.warn(String.format("ProtectedMailboxStorageEntry::isValidForRemoveOperation() failed. " +
"Entry owner does not match Payload owner:\nProtectedStorageEntry=%s\n" +
"PayloadOwner=%s", res1, res2));
log.warn("ProtectedMailboxStorageEntry::isValidForRemoveOperation() failed. " +
"Entry owner does not match Payload owner:\nProtectedStorageEntry={}\n" +
"PayloadOwner={}", res1, res2);
}

return result;
Expand All @@ -150,7 +150,7 @@ public boolean isValidForRemoveOperation() {
* Returns true if the Entry metadata that is expected to stay constant between different versions of the same object
* matches. For ProtectedMailboxStorageEntry, the receiversPubKey must stay the same.
*/
public boolean isMetadataEquals(ProtectedStorageEntry protectedStorageEntry) {
public boolean matchesRelevantPubKey(ProtectedStorageEntry protectedStorageEntry) {
if (!(protectedStorageEntry instanceof ProtectedMailboxStorageEntry)) {
log.error("ProtectedMailboxStorageEntry::isMetadataEquals() failed due to object type mismatch. " +
"ProtectedMailboxStorageEntry required, but got\n" + protectedStorageEntry);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ public boolean isValidForAddOperation() {
if (protectedStoragePayload != null && protectedStoragePayload.getOwnerPubKey() != null)
res2 = Utilities.encodeToHex(protectedStoragePayload.getOwnerPubKey().getEncoded(), true);

log.warn(String.format("ProtectedStorageEntry::isValidForAddOperation() failed. Entry owner does not match Payload owner:\n" +
"ProtectedStorageEntry=%s\nPayloadOwner=%s", res1, res2));
log.warn("ProtectedStorageEntry::isValidForAddOperation() failed. Entry owner does not match Payload owner:\n" +
"ProtectedStorageEntry={}\nPayloadOwner={}", res1, res2);
}

return result;
Expand All @@ -200,8 +200,8 @@ public boolean isValidForRemoveOperation() {
if (protectedStoragePayload != null && protectedStoragePayload.getOwnerPubKey() != null)
res2 = Utilities.encodeToHex(protectedStoragePayload.getOwnerPubKey().getEncoded(), true);

log.warn(String.format("ProtectedStorageEntry::isValidForRemoveOperation() failed. Entry owner does not match Payload owner:\n" +
"ProtectedStorageEntry=%s\nPayloadOwner=%s", res1, res2));
log.warn("ProtectedStorageEntry::isValidForRemoveOperation() failed. Entry owner does not match Payload owner:\n" +
"ProtectedStorageEntry={}\nPayloadOwner={}", res1, res2);
}

return result;
Expand All @@ -218,11 +218,11 @@ boolean isSignatureValid() {
boolean result = Sig.verify(this.ownerPubKey, hashOfDataAndSeqNr, this.signature);

if (!result)
log.warn(String.format("ProtectedStorageEntry::isSignatureValid() failed.\n%s", this));
log.warn("ProtectedStorageEntry::isSignatureValid() failed.\n{}}", this);

return result;
} catch (CryptoException e) {
log.error(String.format("ProtectedStorageEntry::isSignatureValid() exception %s", e.toString()));
log.error("ProtectedStorageEntry::isSignatureValid() exception {}", e.toString());
return false;
}
}
Expand All @@ -231,7 +231,7 @@ boolean isSignatureValid() {
* Returns true if the Entry metadata that is expected to stay constant between different versions of the same object
* matches.
*/
public boolean isMetadataEquals(ProtectedStorageEntry protectedStorageEntry) {
public boolean matchesRelevantPubKey(ProtectedStorageEntry protectedStorageEntry) {
boolean result = protectedStorageEntry.getOwnerPubKey().equals(this.ownerPubKey);

if (!result) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public void isMetadataEquals() throws NoSuchAlgorithmException, CryptoException

ProtectedStorageEntry seqNrTwo = buildProtectedMailboxStorageEntry(mailboxStoragePayload, senderKeys, receiverKeys.getPublic(), 2);

Assert.assertTrue(seqNrOne.isMetadataEquals(seqNrTwo));
Assert.assertTrue(seqNrOne.matchesRelevantPubKey(seqNrTwo));
}

// TESTCASE: isMetadataEquals() should fail if the receiversPubKey changes
Expand All @@ -197,6 +197,6 @@ public void isMetadataEquals_receiverPubKeyChanged() throws NoSuchAlgorithmExcep

ProtectedStorageEntry seqNrTwo = buildProtectedMailboxStorageEntry(mailboxStoragePayload, senderKeys, senderKeys.getPublic(), 1);

Assert.assertFalse(seqNrOne.isMetadataEquals(seqNrTwo));
Assert.assertFalse(seqNrOne.matchesRelevantPubKey(seqNrTwo));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public void isMetadataEquals() throws NoSuchAlgorithmException, CryptoException

ProtectedStorageEntry seqNrTwo = buildProtectedStorageEntry(ownerKeys, ownerKeys, 2);

Assert.assertTrue(seqNrOne.isMetadataEquals(seqNrTwo));
Assert.assertTrue(seqNrOne.matchesRelevantPubKey(seqNrTwo));
}

// TESTCASE: isMetadataEquals() should fail if the OwnerPubKey changes
Expand All @@ -211,6 +211,6 @@ public void isMetadataEquals_OwnerPubKeyChanged() throws NoSuchAlgorithmExceptio

ProtectedStorageEntry protectedStorageEntryTwo = buildProtectedStorageEntry(ownerKeys, notOwner, 1);

Assert.assertFalse(protectedStorageEntryOne.isMetadataEquals(protectedStorageEntryTwo));
Assert.assertFalse(protectedStorageEntryOne.matchesRelevantPubKey(protectedStorageEntryTwo));
}
}

0 comments on commit 5d35d08

Please sign in to comment.