fix: use local shadow document to update the last sync doc during a c…

[saranyailla]

…loud update request

Issue #, if available:

Description of changes:

In Shadow manager, there's a race condition where CloudUpdateSyncRequest is created for shadow state X, but when executed, the shadow is at state Y. The sync info gets updated with state Y instead of the actual synced state X. This prevents subsequent requests to be skipped with "Cloud shadow already contains update payload". Restarting GG or shadow manager will not fix this issue.

When MergedFullShadowSyncRequest processes multiple CloudUpdateSyncRequests, the first request incorrectly updates sync info to the latest local version, causing subsequent requests to be skipped with "Cloud shadow already contains update payload". This occurs because CloudUpdateSyncRequest.isUpdateNecessary() updates the sync info to the current shadow version in updateSyncInformationVersion() but the lastSyncedDocument doesn't reflect what was actually synced, creating inconsistency.

Solution:
Modified updateSyncInformationVersion() to use the shadow document state that was captured when the sync request was created, ensuring sync info accurately reflects the state that was actually processed, not the current shadow state.

Why is this change necessary:
Fixes issues where local shadows are not synced to the cloud intermittently even when sync direction allows it.

How was this change tested:

Any additional information or context required to review the change:

Checklist:

• Updated the README if applicable
• Updated or added new unit tests
• Updated or added new integration tests
• If your code makes a remote network call, it was tested with a proxy

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

Unit Tests Coverage Report

File	Coverage	Lines	Branches
All files	83%	88%	77%	✅

Minimum allowed coverage is 65%

Generated by 🐒 cobertura-action against 1b6dd49

[github-actions]

Integration Tests Coverage Report

File	Coverage	Lines	Branches
All files	73%	76%	69%	✅

Minimum allowed coverage is 45%

Generated by 🐒 cobertura-action against 1b6dd49

[nivsto]

It'd be great to get this fix merged and released, as the current behavior causes lots of issues :-/

[kstribrnAmzn]

Suggested change

	if (!isUpdateNecessary(currentLocalShadowDocument, currentSyncInformation, context)) {
	if (!isUpdateNecessary(localShadowDocument, currentSyncInformation, context)) {

I think we can remove the DAO call to get the latest shadow. The currentSyncInformation will always lag the shadow document in the request (aka the localShadowDocument so this condition will always evaluate to the same regardless of which shadow document is used.

This will reduce the amount of state required for this function which should make it simpler to test.