(node) don't spuriously add ExplicitHashKey values to user records

[chrislewis]

This fixes issue #48 and includes a new test demonstrating the fix.

Note that this PR requires changes (specifically the dev dependencies) in PR #46.

[geronimo-iia]

@chrislewis hi, you could see #42, I've made a lot of changes about test/doc/size limit over 1Mb, etc etc ...

[chrislewis]

Hi @geronimo-iia, thanks for pointing that out. That PR is quite large and does not appear to fix what this this one does. Specifically, because of this line. This is the JSON structure that gets serialized to a protobuf blob, but the object passed always has the explicit_hash_key_index key with the value of explicitHashKeyTable[record.explicitHashKey], even though explicitHashKeyTable[record.explicitHashKey] may be undefined. This results in the string value "undefined" being written as the explicit hash key within the blob, which is invalid, and (at least) the JVM consumer library fails to parse such records.

As for the rest of it, while the other issues in that PR sound like ones that should be fixed, consider breaking those changes up into smaller PRs, and perhaps leave out the restructuring parts. I can't say I know the success rate of external PRs being merged in this project, but I can say that, in general, larger PRs are far less likely to get attention.

[geronimo-iia]

Hi, I'll see your change and try to introduce it in my pr also. I've ever see this issue on this subject, just no much time to spend on it.
I know that large PR are very difficult to merge/accept, ... in fact, we had use this library at work and make huge test and huge change to obtain something which work as needed ... and this is the result: a huge pr :(
At this point of work, i didn't know how split this work into smaller change (...)

If you use this library actually, you will meet kinesis rejection because message will be over 1Mo. Take care of it :)

Regards

Jerome

[geronimo-iia]

Hello, I've added your change in my code base, if you would have a try you could use 'aws-kinesis-agg-fix' version 4.0.2

[geronimo-iia]

@chrislewis it was fixed in new release 4.0.2, (my previous pr was accepted)

[chrislewis]

@IanMeyers does the closure mean it's been addressed?

[IanMeyers]

I took your comment to indicate that it was fixed in version 4.0.2. If that's not the case then will re-open.