github-actions[bot]
commented
1 year ago Unit Tests Coverage Report
| File | Coverage | Lines | Branches | |
|---|---|---|---|---|
| All files | 84% |
90% |
79% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.MergedFullShadowSyncRequest | 21% |
26% |
15% |
:x: |
| 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% |
97% |
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% |
76% |
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.IpcRateLimiter | 100% |
100% |
100% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.ipc.PubSubClientWrapper | 100% |
100% |
0% |
: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.InboundRateLimiter$1 | 100% |
100% |
100% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.ipc.DeleteThingShadowRequestHandler | 99% |
99% |
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 | 93% |
97% |
89% |
: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 | 79% |
77% |
81% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.CloudDataClient | 74% |
72% |
77% |
: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 7b737ffbc564802adb22a40facaf8d9acee88273
jcosentino11
Issue #, if available:
Description of changes:
:warning: This change undoes the (unreleased) fix in https://github.com/aws-greengrass/aws-greengrass-shadow-manager/pull/166, which introduces a bug where the IPC thread could potentially be blocked, and takes a different approach by restricting sync info to being read by only one thread.
This change adjusts where
SyncRequest#isUpdateNecessaryis called, so that it is only called when aSyncRequestis executing on a sync thread. This is different than previous behavior, where an IPC or MQTT CRT thread could call this method, to read shadow sync information from the local database to make a decision on whether or not the SyncRequest should be put on the queue. This ensures that shadow sync information is only read and written to the local database from a single thread.Since "unnecessary" requests aren't being filtered before being put on the RequestBlockingQueue anymore, there needs to be a filtering mechanism when the
SyncRequestexecutes. Otherwise, the behavior fixed in https://github.com/aws-greengrass/aws-greengrass-shadow-manager/pull/106 will regress.This behavior already works for requests that are not merged into a full sync by RequestMerger, because
SyncRequests already checkisUpdateNecessaryat the beginning. To make it work for full syncs, we need to maintain a "history" of all requests that were merged together for a particular full sync request. Then, when the full sync executes, it can look through these "sub-requests", filter out the unnecessary ones, merge them all together, and execute the result. All of this behavior is wrapped in a newMergedFullShadowSyncRequestclass.The rest of the changes in the PR are cleanup from reverting https://github.com/aws-greengrass/aws-greengrass-shadow-manager/pull/166:
along with a bunch of flaky test fixes.
Why is this change necessary: To address a customer issue where cloud updates were being dropped due to a race condition when reading outdated sync information from the IPC thread. (more details of the original issue in https://github.com/aws-greengrass/aws-greengrass-shadow-manager/pull/166)
How was this change tested: Integration 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.