search

aws-greengrass / aws-greengrass-shadow-manager

A GreengrassV2 Component that provides offline device shadow documents and optional synchronization to the IoT device shadow service.
Apache License 2.0
9 stars 5 forks source link

fix: respect sync direction when queueing full sync on conflict #170

Closed jcosentino11 closed 1 year ago

jcosentino11 commented 1 year ago

Issue #, if available:

Description of changes:

Changes the type of request enqueued when a conflict is detected from a FullShadowSyncRequest, to one of OverwriteCloudShadowRequest/OverwriteLocalShadowRequest/FullShadowSyncRequest, based on the sync direction.

Also refactors so that sync direction is wrapped in a class which is injected wherever needed. This means that all usages will have the same reference to direction, rather than needing to ensure setters are called.

Why is this change necessary: To prevent full syncs from happening when a conflict is detected, when sync direction is not BETWEEN_DEVICE_AND_CLOUD.

How was this change tested: new unit tests

Any additional information or context required to review the change:

Checklist:

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

jcosentino11 commented 1 year ago

LGTM. Is this addressing the race conditions though, or is this something else?

Not directly related, but preventing full syncs when we don't expect them will help when reasoning about the race condition fix

github-actions[bot] commented 1 year ago

Unit Tests Coverage Report

File Coverage Lines Branches
All files 85% 90% 80% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.LocalUpdateSyncRequest 79% 85% 73% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.Direction 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.LocalDeleteSyncRequest 99% 98% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.FullShadowSyncRequest 84% 98% 71% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.CloudDeleteSyncRequest 80% 78% 83% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.DirectionWrapper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.OverwriteCloudShadowRequest 98% 95% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.BaseSyncRequest 83% 93% 73% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.OverwriteLocalShadowRequest 98% 95% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.CloudUpdateSyncRequest 74% 77% 71% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManagerDatabase 0% 0% 0% :x:
com.aws.greengrass.shadowmanager.ShadowManager 75% 80% 71% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManagerDAOImpl 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.AuthorizationHandlerWrapper 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.PubSubIntegrator 89% 93% 86% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManager$1 43% 43% 0% :x:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractDeleteThingShadowOperationHandler 67% 67% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractListNamedShadowsForThingOperationHandler 67% 67% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractUpdateThingShadowOperationHandler 67% 67% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractGetThingShadowOperationHandler 67% 67% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.JsonMerger 94% 100% 89% :white_check_mark:
com.aws.greengrass.shadowmanager.util.SyncNodeMerger 91% 98% 84% :white_check_mark:
com.aws.greengrass.shadowmanager.util.DataOwner 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.ShadowWriteSynchronizeHelper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.Validator 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.util.JsonUtil 94% 100% 88% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.BaseRequestHandler 77% 77% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.GetThingShadowRequestHandler 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.PubSubClientWrapper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.IpcRateLimiter 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.UpdateThingShadowIPCHandler 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.InboundRateLimiter 94% 89% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.DeleteThingShadowRequestHandler 99% 99% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.InboundRateLimiter$1 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.UpdateThingShadowRequestHandler 92% 95% 89% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.DeleteThingShadowIPCHandler 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.GetThingShadowIPCHandler 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.ListNamedShadowsForThingIPCHandler 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.model.Operation 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.model.PubSubRequest 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.PeriodicSyncStrategy 63% 77% 50% :x:
com.aws.greengrass.shadowmanager.sync.strategy.SyncStrategyFactory 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.BaseSyncStrategy 88% 90% 86% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.RealTimeSyncStrategy 93% 86% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientFactory 55% 85% 25% :x:
com.aws.greengrass.shadowmanager.sync.RequestBlockingQueue 96% 99% 93% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.SyncHandler 71% 88% 53% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientWrapper 91% 91% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.RequestMerger 78% 76% 81% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.CloudDataClient 70% 69% 72% :white_check_mark:
com.aws.greengrass.shadowmanager.model.configuration.ShadowSyncConfiguration 87% 93% 82% :white_check_mark:
com.aws.greengrass.shadowmanager.model.configuration.ThingShadowSyncConfiguration 76% 78% 75% :white_check_mark:
com.aws.greengrass.shadowmanager.configuration.ShadowDocSizeConfiguration 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.configuration.RateLimitsConfiguration 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.configuration.ComponentConfiguration 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.model.StrategyType 86% 92% 80% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.model.Strategy 90% 100% 80% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ResponseMessageBuilder 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowDocument 81% 89% 73% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowStateMetadata 93% 98% 89% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ErrorMessage 75% 100% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowRequest 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowState 84% 87% 81% :white_check_mark:
com.aws.greengrass.shadowmanager.model.LogEvents 100% 100% 0% :white_check_mark:

Minimum allowed coverage is 65%

Generated by :monkey: cobertura-action against 2163adde14f2171f3283b5f2c8583d50e521b778

github-actions[bot] commented 1 year ago

Integration Tests Coverage Report

File Coverage Lines Branches
All files 70% 75% 65% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.LocalUpdateSyncRequest 75% 80% 69% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.Direction 83% 91% 75% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.LocalDeleteSyncRequest 55% 59% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.FullShadowSyncRequest 73% 85% 62% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.CloudDeleteSyncRequest 53% 55% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.DirectionWrapper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.OverwriteCloudShadowRequest 76% 76% 75% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.BaseSyncRequest 46% 50% 41% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.model.OverwriteLocalShadowRequest 33% 33% 0% :x:
com.aws.greengrass.shadowmanager.sync.model.CloudUpdateSyncRequest 63% 61% 64% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManagerDatabase 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManager 90% 92% 88% :white_check_mark:
com.aws.greengrass.shadowmanager.ShadowManagerDAOImpl 84% 90% 77% :white_check_mark:
com.aws.greengrass.shadowmanager.AuthorizationHandlerWrapper 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.PubSubIntegrator 22% 22% 21% :x:
com.aws.greengrass.shadowmanager.ShadowManager$1 14% 14% 0% :x:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractDeleteThingShadowOperationHandler 100% 100% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractListNamedShadowsForThingOperationHandler 100% 100% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractUpdateThingShadowOperationHandler 100% 100% 0% :white_check_mark:
software.amazon.awssdk.aws.greengrass.GeneratedAbstractGetThingShadowOperationHandler 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.JsonMerger 64% 72% 56% :white_check_mark:
com.aws.greengrass.shadowmanager.util.SyncNodeMerger 75% 88% 62% :white_check_mark:
com.aws.greengrass.shadowmanager.util.DataOwner 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.ShadowWriteSynchronizeHelper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.util.Validator 67% 68% 65% :white_check_mark:
com.aws.greengrass.shadowmanager.util.JsonUtil 81% 89% 74% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.BaseRequestHandler 77% 77% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.GetThingShadowRequestHandler 87% 74% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.PubSubClientWrapper 82% 82% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.IpcRateLimiter 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.UpdateThingShadowIPCHandler 55% 55% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.InboundRateLimiter 94% 100% 88% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.DeleteThingShadowRequestHandler 86% 72% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.InboundRateLimiter$1 75% 100% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.UpdateThingShadowRequestHandler 74% 76% 72% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.DeleteThingShadowIPCHandler 53% 53% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.GetThingShadowIPCHandler 95% 95% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.ListNamedShadowsForThingIPCHandler 61% 55% 67% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.model.Operation 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.ipc.model.PubSubRequest 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.PeriodicSyncStrategy 71% 92% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.SyncStrategyFactory 100% 100% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.BaseSyncStrategy 84% 88% 79% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.RealTimeSyncStrategy 83% 90% 75% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientFactory 78% 91% 64% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.RequestBlockingQueue 61% 70% 52% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.SyncHandler 82% 93% 71% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientWrapper 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.RequestMerger 33% 44% 22% :x:
com.aws.greengrass.shadowmanager.sync.CloudDataClient 47% 43% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.model.configuration.ShadowSyncConfiguration 71% 81% 61% :white_check_mark:
com.aws.greengrass.shadowmanager.model.configuration.ThingShadowSyncConfiguration 76% 78% 75% :white_check_mark:
com.aws.greengrass.shadowmanager.configuration.ShadowDocSizeConfiguration 61% 73% 50% :white_check_mark:
com.aws.greengrass.shadowmanager.configuration.RateLimitsConfiguration 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.configuration.ComponentConfiguration 100% 100% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.model.StrategyType 55% 69% 40% :white_check_mark:
com.aws.greengrass.shadowmanager.sync.strategy.model.Strategy 90% 100% 80% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ResponseMessageBuilder 91% 91% 0% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowDocument 81% 89% 73% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowStateMetadata 83% 88% 78% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ErrorMessage 97% 94% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowRequest 88% 77% 100% :white_check_mark:
com.aws.greengrass.shadowmanager.model.ShadowState 96% 98% 94% :white_check_mark:
com.aws.greengrass.shadowmanager.model.LogEvents 100% 100% 0% :white_check_mark:

Minimum allowed coverage is 45%

Generated by :monkey: cobertura-action against 2163adde14f2171f3283b5f2c8583d50e521b778