-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Copy to view node: added tests #10529
base: master
Are you sure you want to change the base?
[WIP] Copy to view node: added tests #10529
Conversation
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Outdated
Show resolved
Hide resolved
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Outdated
Show resolved
Hide resolved
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Outdated
Show resolved
Hide resolved
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Outdated
Show resolved
Hide resolved
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Outdated
Show resolved
Hide resolved
entityView.setStartTimeMs(Instant.now().minus(1, ChronoUnit.DAYS).toEpochMilli()); | ||
entityView.setEndTimeMs(Instant.now().plus(1, ChronoUnit.DAYS).toEpochMilli()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make Instant.now().minus(1, ChronoUnit.DAYS).toEpochMilli() and Instant.now().plus(1, ChronoUnit.DAYS).toEpochMilli() final fields and reuse them in each test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moreover, consider to move creation of entity view to some method and reuse it each test.
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Show resolved
Hide resolved
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Outdated
Show resolved
Hide resolved
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Outdated
Show resolved
Hide resolved
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Outdated
Show resolved
Hide resolved
...s/src/test/java/org/thingsboard/rule/engine/action/TbCopyAttributesToEntityViewNodeTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments added above.
EntityView entityView = getEntityView(); | ||
|
||
TbMsg msg = TbMsg.newMsg( | ||
TbMsgType.POST_ATTRIBUTES_REQUEST, DEVICE_ID, new TbMsgMetaData(Map.of("scope", AttributeScope.CLIENT_SCOPE.name())), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TbMsgType.POST_ATTRIBUTES_REQUEST, DEVICE_ID, new TbMsgMetaData(Map.of("scope", AttributeScope.CLIENT_SCOPE.name())), | |
TbMsgType.POST_ATTRIBUTES_REQUEST, DEVICE_ID, new TbMsgMetaData(Map.of(DataConstants.SCOPE, AttributeScope.CLIENT_SCOPE.name())), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constructed metadata will not take any effect. See lines 83-84 in the node implementation. However empty metadata will cause the message to fail. Please change the scope in metadata to AttributeScope.SERVER_SCOPE.name() and verify that attributes will be still saved with client scope.
TbMsgType.POST_ATTRIBUTES_REQUEST, DEVICE_ID, new TbMsgMetaData(Map.of("scope", AttributeScope.CLIENT_SCOPE.name())), | ||
"{\"attribute1\": 100, \"attribute2\": \"value2\"}"); | ||
|
||
mockEntityViewService(entityView); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mockEntityViewService(entityView); | |
mockEntityViewLookup(entityView); |
return null; | ||
}).when(telemetryServiceMock).saveAndNotify(any(), any(), any(AttributeScope.class), anyList(), any(FutureCallback.class)); | ||
TbMsg newMsg = TbMsg.newMsg(msg, msg.getQueueName(), msg.getRuleChainId(), msg.getRuleNodeId()); | ||
doAnswer(invocation -> newMsg).when(ctxMock).newMsg(any(), any(String.class), any(), any(), any(), any()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add todo to use newMsg with any(TbMsgType)
node.onMsg(ctxMock, msg); | ||
|
||
verify(entityViewServiceMock).findEntityViewsByTenantIdAndEntityIdAsync(eq(TENANT_ID), eq(DEVICE_ID)); | ||
verify(telemetryServiceMock).saveAndNotify(eq(TENANT_ID), eq(entityView.getId()), eq(AttributeScope.CLIENT_SCOPE), anyList(), any(FutureCallback.class)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please capture the list and verify that it includes the correct attribute inside.
node.onMsg(ctxMock, msg); | ||
|
||
verify(entityViewServiceMock).findEntityViewsByTenantIdAndEntityIdAsync(eq(TENANT_ID), eq(DEVICE_ID)); | ||
verify(telemetryServiceMock).deleteAndNotify(eq(TENANT_ID), eq(entityView.getId()), eq(AttributeScope.CLIENT_SCOPE), anyList(), any(FutureCallback.class)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please capture the list and verify that it includes the correct attribute inside.
verify(telemetryServiceMock).saveAndNotify(eq(TENANT_ID), eq(entityView.getId()), eq(AttributeScope.CLIENT_SCOPE), anyList(), any(FutureCallback.class)); | ||
verify(ctxMock).ack(eq(msg)); | ||
verify(ctxMock).enqueueForTellNext(eq(newMsg), eq(TbNodeConnectionType.SUCCESS)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use verifyNoMoreInteractions(...);
if (msg.isTypeOneOf(ATTRIBUTES_UPDATED, ATTRIBUTES_DELETED, | ||
ACTIVITY_EVENT, INACTIVITY_EVENT, POST_ATTRIBUTES_REQUEST)) { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will skip the validation if someone mistakenly removes those types from the list of allowed types. Could you please add a test that will ensure that there will be no tellFailure with "Unsupported msg type [" + msgType + "]" exception message if the type is supported?
EntityViewId entityViewId = new EntityViewId(UUID.fromString("d117f1a4-24ea-4fdd-b94e-5a472e99d925")); | ||
EntityView entityView = new EntityView(entityViewId); | ||
entityView.setStartTimeMs(Instant.now().minus(2, ChronoUnit.DAYS).toEpochMilli()); | ||
entityView.setEndTimeMs(Instant.now().minus(1, ChronoUnit.DAYS).toEpochMilli()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reuse the existing method of getEntityView() by adding an overload version of it by passing start and endTime.
private EntityView getEntityView() { | ||
EntityViewId entityViewId = new EntityViewId(UUID.fromString("a2109747-d1f4-475a-baaa-55f5d4897ad8")); | ||
EntityView entityView = new EntityView(entityViewId); | ||
entityView.setStartTimeMs(Instant.now().minus(1, ChronoUnit.DAYS).toEpochMilli()); | ||
entityView.setEndTimeMs(Instant.now().plus(1, ChronoUnit.DAYS).toEpochMilli()); | ||
AttributesEntityView attributes = new AttributesEntityView(List.of("attribute1"), Collections.emptyList(), Collections.emptyList()); | ||
entityView.setKeys(new TelemetryEntityView(Collections.emptyList(), attributes)); | ||
return entityView; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be moved to constants:
private final long ENTITY_VIEW_START_TS = Instant.now().minus(1, ChronoUnit.DAYS).toEpochMilli();
private final long ENTITY_VIEW_END_TS = Instant.now().plus(1, ChronoUnit.DAYS).toEpochMilli();
private final AttributesEntityView CLIENT_ATTRIBUTES = new AttributesEntityView(List.of("attribute1"), Collections.emptyList(), Collections.emptyList());
private final TelemetryEntityView TELEMETRY_ENTITY_VIEW = new TelemetryEntityView(Collections.emptyList(), CLIENT_ATTRIBUTES);
and this:
private final EntityViewId ENTITY_VIEW_ID = new EntityViewId(UUID.fromString("a2109747-d1f4-475a-baaa-55f5d4897ad8"));
Finally method will be looks like this:
private EntityView getEntityView() {
EntityView entityView = new EntityView(ENTITY_VIEW_ID);
entityView.setStartTimeMs(ENTITY_VIEW_START_TS);
entityView.setEndTimeMs(ENTITY_VIEW_END_TS);
entityView.setKeys(TELEMETRY_ENTITY_VIEW);
return entityView;
}
Pull Request description
Added tests for Copy to view node.
General checklist
Back-End feature checklist