Restore is less useful because of IsLatest check

[tonytamps]

Hey,

Thanks for this library - it's actually incredibly well thought out and well structured. I'm really pleased with your approach here.

I was going through the normal testing and I discovered that I couldn't restore successfully. I debugged the restore process and found that unless the record was the most recent, it won't be restored. See the snippet below from restoreDynamoDb.js

s3ToDynamoDb(versionList) {
        return this.getDbTableKeys()
            .then(keys => {
                let promises = [];
                Object.keys(versionList).forEach((key, index) => {
                    if ( (! versionList[key].DeletedMarker &&  versionList[key].IsLatest) ) {
                        promises.push(this.processVersion(versionList[key], keys));
                    }
                });

                return Promise.all(promises);
            })
            .catch(err => {
                throw err;
            });
    }

specifically, the line if ( (! versionList[key].DeletedMarker && versionList[key].IsLatest) ).

The issue is that restoring to a particular date is most likely done after some data has been deleted or accidentally modified. Since this is considered a change, the ideal record to be restored is no longer the 'latest' record and is ignored.

Am I using restore wrong or is this actually a bug?

[tonytamps]

If the && versionList[key].IsLatest is removed from the statement the restore operates as I would expect.

Since the previous functions building the version list perform the checks on date to get the latest change before the given restore date, it should be safe to remove it and assume that the record at this point is the latest for the given set of records.

[shevchenkos]

Hi, thanks for time spending on debugging. I think this is result of this pull request https://github.com/Purple-Unicorns/DynamoDbBackUp/pull/20 That time I didn't debug enough this change. Probably this change solved only particular problem of author. I'll try to find time to look at this issue.

[tonytamps]

To be frank, that PR looks a little bit garbage. I don't understand why the current solution didn't satisfy his needs and why he would implement in this way.

I'm happy to make a PR but it'll probably undo his changes. That okay?

On Tue, Jan 17, 2017, 18:28 Serhii notifications@github.com wrote:

Hi, thanks for time spending on debugging. I think this is result of this pull request #20 https://github.com/Purple-Unicorns/DynamoDbBackUp/pull/20 That time I didn't debug enough this change. Probably this change solved only particular problem of author. I'll try to find time to look at this issue.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Purple-Unicorns/DynamoDbBackUp/issues/23#issuecomment-273041045, or mute the thread https://github.com/notifications/unsubscribe-auth/ACP2KLRDjSdmQP0X8zHsAj0PadnomxpLks5rTG2RgaJpZM4LlNLw .

[shevchenkos]

Yeah, please make a PR.

[shevchenkos]

@tonytamps, I've just updated npm package.

[tonytamps]

Thanks mate, appreciate that a lot. I'll close this now.