rbattle
commented
3 years ago nit: the commit + pr should be amended to correct the spelling of shadow
Closed nikkhilmuthye closed 3 years ago
rbattle
commented
3 years ago nit: the commit + pr should be amended to correct the spelling of shadow
github-actions[bot]
commented
3 years ago Unit Tests Coverage Report
| File | Coverage | Lines | Branches | |
|---|---|---|---|---|
| All files | 86% |
92% |
81% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.LocalUpdateSyncRequest | 91% |
100% |
81% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.LocalDeleteSyncRequest | 100% |
100% |
100% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.FullShadowSyncRequest | 82% |
97% |
68% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.CloudDeleteSyncRequest | 75% |
76% |
75% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.BaseSyncRequest | 78% |
55% |
100% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.CloudUpdateSyncRequest | 96% |
91% |
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 | 22% |
22% |
0% |
:x: |
| com.aws.greengrass.shadowmanager.sync.strategy.SyncStrategyFactory | 100% |
100% |
100% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.strategy.BaseSyncStrategy | 75% |
82% |
67% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.strategy.RealTimeSyncStrategy | 92% |
95% |
90% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientFactory | 45% |
79% |
12% |
:x: |
| com.aws.greengrass.shadowmanager.sync.RequestBlockingQueue | 96% |
99% |
93% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.SyncHandler | 77% |
94% |
61% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientWrapper | 91% |
91% |
0% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.RequestMerger | 74% |
71% |
77% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.CloudDataClient | 75% |
76% |
75% |
: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.ShadowManagerDatabase | 0% |
0% |
0% |
:x: |
| com.aws.greengrass.shadowmanager.ShadowManager | 78% |
85% |
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.ShadowManager$1 | 50% |
50% |
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.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.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 | 96% |
100% |
92% |
: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 | 86% |
71% |
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: |
| 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 | 100% |
100% |
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 | 94% |
95% |
93% |
: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: |
Minimum allowed coverage is 65%
Generated by :monkey: cobertura-action against 987886d40194b7204dff2e0f5832af4a84f657bc
github-actions[bot]
commented
3 years ago Integration Tests Coverage Report
| File | Coverage | Lines | Branches | |
|---|---|---|---|---|
| All files | 61% |
69% |
52% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.LocalUpdateSyncRequest | 42% |
66% |
19% |
:x: |
| com.aws.greengrass.shadowmanager.sync.model.LocalDeleteSyncRequest | 62% |
74% |
50% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.FullShadowSyncRequest | 34% |
43% |
25% |
:x: |
| com.aws.greengrass.shadowmanager.sync.model.CloudDeleteSyncRequest | 75% |
76% |
75% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.BaseSyncRequest | 78% |
55% |
100% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.model.CloudUpdateSyncRequest | 66% |
66% |
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 | 0% |
0% |
0% |
:x: |
| com.aws.greengrass.shadowmanager.sync.strategy.SyncStrategyFactory | 68% |
86% |
50% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.strategy.BaseSyncStrategy | 80% |
94% |
67% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.strategy.RealTimeSyncStrategy | 67% |
63% |
70% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientFactory | 85% |
100% |
69% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.RequestBlockingQueue | 47% |
56% |
39% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.SyncHandler | 82% |
91% |
72% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.IotDataPlaneClientWrapper | 100% |
100% |
0% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.RequestMerger | 6% |
10% |
2% |
:x: |
| com.aws.greengrass.shadowmanager.sync.CloudDataClient | 51% |
48% |
54% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.model.configuration.ShadowSyncConfiguration | 70% |
79% |
61% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.model.configuration.ThingShadowSyncConfiguration | 76% |
78% |
75% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.ShadowManagerDatabase | 100% |
100% |
100% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.ShadowManager | 82% |
85% |
79% |
: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.ShadowManager$1 | 17% |
17% |
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.sync.strategy.model.StrategyType | 46% |
46% |
0% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.sync.strategy.model.Strategy | 10% |
10% |
0% |
:x: |
| com.aws.greengrass.shadowmanager.util.JsonMerger | 34% |
38% |
31% |
:x: |
| com.aws.greengrass.shadowmanager.util.SyncNodeMerger | 0% |
0% |
0% |
:x: |
| com.aws.greengrass.shadowmanager.util.DataOwner | 0% |
0% |
0% |
:x: |
| com.aws.greengrass.shadowmanager.util.ShadowWriteSynchronizeHelper | 100% |
100% |
0% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.util.Validator | 62% |
64% |
59% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.util.JsonUtil | 80% |
86% |
73% |
: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 | 81% |
86% |
77% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.model.ErrorMessage | 97% |
94% |
100% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.model.ShadowRequest | 86% |
71% |
100% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.model.ShadowState | 89% |
91% |
88% |
:white_check_mark: |
| com.aws.greengrass.shadowmanager.model.LogEvents | 100% |
100% |
0% |
: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 | 80% |
80% |
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 | 73% |
74% |
71% |
: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: |
Minimum allowed coverage is 45%
Generated by :monkey: cobertura-action against 987886d40194b7204dff2e0f5832af4a84f657bc
clogwog
commented
3 years ago just a quick question. if i just remove the iot:DeleteThingShadow for now (until this change is available) would you think i can get around this deleting my server created shadow ? My tests seem to indicate that without the iot:DeleteThingShadow permission the shadows aren't sync-ed.. and with it they are deleted.
Another question: how long does it normally take for merges to make it all the way into production ? I assume the merge from synchBug to main will be packaged with other releases ? or do you guys do continuous integration to production ?
https://forums.aws.amazon.com/thread.jspa?threadID=346841&tstart=0
nikkhilmuthye
commented
3 years ago just a quick question. if i just remove the iot:DeleteThingShadow for now (until this change is available) would you think i can get around this deleting my server created shadow ? My tests seem to indicate that without the iot:DeleteThingShadow permission the shadows aren't sync-ed.. and with it they are deleted.
Another question: how long does it normally take for merges to make it all the way into production ? I assume the merge from synchBug to main will be packaged with other releases ? or do you guys do continuous integration to production ?
https://forums.aws.amazon.com/thread.jspa?threadID=346841&tstart=0
I created a new GitHub issue https://github.com/aws-greengrass/aws-greengrass-shadow-manager/issues/98 for you to track.
You can remove the iot:DeleteThingShadow policy action from your IoT policy and cert policy to make sure that Shadow manager is unable to delete a cloud shadow. But after that Shadow manager will be unable to delete any shadow in the cloud if you have a valid delete shadow scenario.
Not sure I completely understood this
My tests seem to indicate that without the iot:DeleteThingShadow permission the shadows aren't sync-ed.. and with it they are deleted.
For your other question, you can keep track of the issue I just created. I will update the ticket when we are ready to release this patch.
Issue #, if available:
Description of changes: Check the shadow update time if either the cloud shadow or local shadow is non-existent. Also make sure that the last sync time and cloud update time in the sync info is correct.
Why is this change necessary: If a cloud shadow has been updated after being deleted before that, then make sure the full sync updates the local shadow with that update. If a local shadow has been updated after being deleted before that, then make sure the full sync updates the cloud shadow with that update.
How was this change tested: Added 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.