Feature/remote content hash #296
Conversation
This contains the hashing logic within the RemotePost Hashing extension alongside hashing for additional types for use outside of RemotePost
The test is too slow to go in the existing test suite.
diegoreymendez
requested review from
diegoreymendez
and removed request for
diegoreymendez
WordPressKit/SHAHasher.m
Outdated
| return [NSData dataWithBytes:digest length:CC_SHA256_DIGEST_LENGTH]; | ||
| } | ||
|
|
||
| + (NSString *)sha256StringFromData:(NSData *)data { |
There was a problem hiding this comment.
I think this method is poorly named (I know it existed already) because it doesn't really do anything in terms of SHA256. As it is right now, this is more like hexStringFromData.
| /// and at present are treated as genuine updates as they are triggered by the user changing their posts content. | ||
| - (NSString *)contentHash | ||
| { | ||
| NSString *hashedContents = [NSString stringWithFormat:@"%@/%@/%@%@/%@/%@%@/%@/%@%@/%@/%@%@/%@/%@%@/%@/%@%@/%@/%@%@/%@/%@%@/%@/%@", |
There was a problem hiding this comment.
Considering we have support to hash each component in the methods below, and then combine the hash of each component (through combineHashes), is there a reason behind the choice of using a big string concatenating all of the post elements?
There was a problem hiding this comment.
Performance mostly.
Hashing every property will likely minimize collisions but it took ~ 0.1s when I performance tested this. I fear this could result in a significant delay when dealing with many posts.
Property hash performance
For an AbstractPost upload I don't think this is a problem as a post upload isn't an action that blocks the user. However for downloading a RemotePost update this could result in real user delays as the non-arrival of this update will block them from continued use of the app.
ConcatenatedStringHash Performance
After concatenating the strings we're down to < 0.01s
I'm still a little bit concerned that this has the potential to be a bit too slow if we're going to be checking large numbers of posts each update. The swift algorithm I wrote earlier this month just performs a bitwise shift operation, this would be much faster than the others if it prove necessary.
Let me know your thoughts, perhaps I've missed something somewhere.
How are posts currently downloaded when in large numbers? Do we download them in batches?
This doesn't actually do any sha256 hashing
Description
Adds a hash to
RemotePostso we can identify when a remote posts content has been updated.A lot of this logic was already present in
AbstractPostso the non specialized parts of the hash have been extracted so any client of WordPressKit can generate a hash. It's also been adapted to be faster seeing as this hash will not be uploaded nor need to meet cryptographic standards but will be ran for every post the server sends and will block the user if it runs slow.AbstractPostwill still be able to use the slower better collision proof version of this hash.Fixes
Part of Conflict Resolution Dialog.
At present we use
dateModifiedto identify if a remote post has changed which is not fully reliable.Testing Details
Running the tests and analyzing that they do indeed test that this is functional should suffice.