Skip to content
This repository was archived by the owner on May 13, 2022. It is now read-only.

Conversation

@martinq
Copy link
Contributor

@martinq martinq commented Jan 22, 2014

I started this by adding a new "replaces" property to handle document replacement. I refactored property initialization into a private helper function and added a new constructor signature so that users could optionally provide a String array of IDs to replace and leave the existing constructors intact.

However upon testing I found that documents must pass signature verification otherwise the Learning Registry node will reject the submission. This applies both for the document that is being submitted and the one that is being replaced. The hash computations coming out of LRJavaLib and the LRSignature module in the Learning Registry server software were different, and I tracked it down to the omission of a normalization step, per the spec here:

http://docs.learningregistry.org/en/latest/spec/Identity_Trust_Auth_and_Security/index.html#signing-a-resource-data-description-document

The main issues were String conversion of booleans and dropping of numeric values. Note however that the spec says that all numeric values should be dropped but the actual Python implementation only drops numeric values in arrays, so I copied the implementation so as to work properly with real-world servers. I've tested against the LR sandbox and have set up integration tests for our project which depends on LRJavaLib, but have not added any unit or integration tests to LRJavaLib proper.

While I was in there, I also refactored the generation of signable and sendable data maps -- these are essentially the same modulo signing information and excluded fields, so I thought it best to have one defer to the other for the sake of DRY.

…tructors remain, defaulting to null replaces arg.
…on LRSignature module in order to pass signature verification on replace
@joehobson
Copy link
Member

@serpentblade / Dan... can you please take a look through this code and the spec documentation above and make sure everything is in order? If it is, merge the pull request and increment the LRJavaLib version number to 1.3 since we're adding a new feature. If possible, please do so before the Wednesday developer call so we can discuss issues or discrepancies at that time. Thanks

@ghost ghost assigned serpentblade Jan 28, 2014
serpentblade added a commit that referenced this pull request Jan 29, 2014
Added "replaces" property and pre-bencode normalization
@serpentblade serpentblade merged commit 0a5e92a into navnorth:master Jan 29, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants