From a5c06ec70d6f721012470b1f1ce83c36a5b04c76 Mon Sep 17 00:00:00 2001 From: Dmytro Skarzhynets Date: Mon, 18 Dec 2023 09:41:45 +0200 Subject: [PATCH] Refactor: strategy creation, hide implementation details from clients --- .../IntegrationActivityManagerTest.java | 46 +++++------ .../activity/AbstractActivityManager.java | 53 ++++++++----- .../transport/activity/ActivityState.java | 3 - .../strategy/ActivityStrategyType.java} | 36 ++++++--- .../strategy/AllEventsActivityStrategy.java | 12 ++- .../FirstAndLastEventActivityStrategy.java | 2 +- .../strategy/FirstEventActivityStrategy.java | 2 +- .../strategy/LastEventActivityStrategy.java | 12 ++- .../service/DefaultTransportService.java | 11 ++- .../strategy/ActivityStrategyFactoryTest.java | 70 ----------------- .../strategy/ActivityStrategyTypeTest.java} | 39 +++++----- .../AllEventsActivityStrategyTest.java | 2 +- .../LastEventActivityStrategyTest.java | 2 +- .../service/TransportActivityManagerTest.java | 77 ++++--------------- 14 files changed, 148 insertions(+), 219 deletions(-) rename common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/{service/TransportActivityState.java => activity/strategy/ActivityStrategyType.java} (68%) delete mode 100644 common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyFactoryTest.java rename common/transport/transport-api/src/{main/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyFactory.java => test/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyTypeTest.java} (62%) diff --git a/application/src/test/java/org/thingsboard/server/service/integration/IntegrationActivityManagerTest.java b/application/src/test/java/org/thingsboard/server/service/integration/IntegrationActivityManagerTest.java index fdb1710189..3b85261bac 100644 --- a/application/src/test/java/org/thingsboard/server/service/integration/IntegrationActivityManagerTest.java +++ b/application/src/test/java/org/thingsboard/server/service/integration/IntegrationActivityManagerTest.java @@ -34,6 +34,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.MethodSource; import org.mockito.ArgumentCaptor; import org.mockito.Mock; @@ -46,10 +47,7 @@ import org.thingsboard.server.common.msg.queue.TopicPartitionInfo; import org.thingsboard.server.common.transport.activity.ActivityReportCallback; import org.thingsboard.server.common.transport.activity.ActivityState; import org.thingsboard.server.common.transport.activity.strategy.ActivityStrategy; -import org.thingsboard.server.common.transport.activity.strategy.AllEventsActivityStrategy; -import org.thingsboard.server.common.transport.activity.strategy.FirstAndLastEventActivityStrategy; -import org.thingsboard.server.common.transport.activity.strategy.FirstEventActivityStrategy; -import org.thingsboard.server.common.transport.activity.strategy.LastEventActivityStrategy; +import org.thingsboard.server.common.transport.activity.strategy.ActivityStrategyType; import org.thingsboard.server.gen.transport.TransportProtos; import org.thingsboard.server.queue.TbQueueCallback; import org.thingsboard.server.queue.TbQueueProducer; @@ -181,33 +179,32 @@ public class IntegrationActivityManagerTest { verify(integrationServiceMock).onActivity(key); } - @ParameterizedTest - @MethodSource("provideTestParamsForCreateNewState") - void givenDifferentReportingStrategies_whenCreatingNewState_thenShouldCreateEmptyStateWithCorrectStrategy( - String reportingStrategyName, ActivityStrategy reportingStrategy - ) { + @Test + void givenKey_whenCreatingNewState_thenShouldCorrectlyCreateNewEmptyState() { // GIVEN var key = new IntegrationActivityKey(TENANT_ID, DEVICE_ID); when(integrationServiceMock.createNewState(key)).thenCallRealMethod(); - ReflectionTestUtils.setField(integrationServiceMock, "reportingStrategyName", reportingStrategyName); - - ActivityState expectedState = new ActivityState<>(); - expectedState.setStrategy(reportingStrategy); // WHEN - ActivityState actualState = integrationServiceMock.createNewState(key); + ActivityState actualNewState = integrationServiceMock.createNewState(key); // THEN - assertThat(actualState).isEqualTo(expectedState); + ActivityState expectedNewState = new ActivityState<>(); + assertThat(actualNewState).isEqualTo(expectedNewState); } - private static Stream provideTestParamsForCreateNewState() { - return Stream.of( - Arguments.of("ALL", new AllEventsActivityStrategy()), - Arguments.of("FIRST", new FirstEventActivityStrategy()), - Arguments.of("LAST", new LastEventActivityStrategy()), - Arguments.of("FIRST_AND_LAST", new FirstAndLastEventActivityStrategy()) - ); + @ParameterizedTest + @EnumSource(ActivityStrategyType.class) + void givenDifferentReportingStrategies_whenGettingStrategy_thenShouldReturnCorrectStrategy(ActivityStrategyType reportingStrategyType) { + // GIVEN + doCallRealMethod().when(integrationServiceMock).getStrategy(); + ReflectionTestUtils.setField(integrationServiceMock, "reportingStrategyType", reportingStrategyType); + + // WHEN + ActivityStrategy actualStrategy = integrationServiceMock.getStrategy(); + + // THEN + assertThat(actualStrategy).isEqualTo(reportingStrategyType.toStrategy()); } @Test @@ -215,12 +212,8 @@ public class IntegrationActivityManagerTest { // GIVEN var key = new IntegrationActivityKey(TENANT_ID, DEVICE_ID); - ActivityStrategy strategySpy = spy(new FirstEventActivityStrategy()); - ActivityState state = new ActivityState<>(); state.setLastRecordedTime(123L); - state.setLastReportedTime(312L); - state.setStrategy(strategySpy); ActivityState stateSpy = spy(state); when(integrationServiceMock.updateState(key, state)).thenCallRealMethod(); @@ -231,7 +224,6 @@ public class IntegrationActivityManagerTest { // THEN assertThat(updatedState).isSameAs(state); verifyNoInteractions(stateSpy); - verifyNoInteractions(strategySpy); } @ParameterizedTest diff --git a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/AbstractActivityManager.java b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/AbstractActivityManager.java index 07217e57e8..2124eb13d2 100644 --- a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/AbstractActivityManager.java +++ b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/AbstractActivityManager.java @@ -30,9 +30,11 @@ */ package org.thingsboard.server.common.transport.activity; +import lombok.Data; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.thingsboard.server.common.data.StringUtils; +import org.thingsboard.server.common.transport.activity.strategy.ActivityStrategy; import org.thingsboard.server.queue.scheduler.SchedulerComponent; import java.util.Map; @@ -45,7 +47,7 @@ import java.util.concurrent.atomic.AtomicBoolean; @Slf4j public abstract class AbstractActivityManager implements ActivityManager { - private final ConcurrentMap> states = new ConcurrentHashMap<>(); + private final ConcurrentMap states = new ConcurrentHashMap<>(); @Autowired protected SchedulerComponent scheduler; @@ -53,6 +55,15 @@ public abstract class AbstractActivityManager implements Activity protected String name; private boolean initialized; + @Data + private class ActivityStateWrapper { + + private volatile ActivityState state; + private volatile long lastReportedTime; + private volatile ActivityStrategy strategy; + + } + @Override public synchronized void init(String name, long reportingPeriodMillis) { if (!initialized) { @@ -70,6 +81,8 @@ public abstract class AbstractActivityManager implements Activity protected abstract ActivityState createNewState(Key key); + protected abstract ActivityStrategy getStrategy(); + protected abstract ActivityState updateState(Key key, ActivityState state); protected abstract boolean hasExpired(long lastRecordedTime); @@ -92,27 +105,31 @@ public abstract class AbstractActivityManager implements Activity long newLastRecordedTime = System.currentTimeMillis(); var shouldReport = new AtomicBoolean(false); - var activityState = states.compute(key, (__, state) -> { - if (state == null) { + var activityStateWrapper = states.compute(key, (__, stateWrapper) -> { + if (stateWrapper == null) { var newState = createNewState(key); if (newState == null) { return null; } - state = newState; + stateWrapper = new ActivityStateWrapper(); + stateWrapper.setState(newState); + stateWrapper.setStrategy(getStrategy()); } + var state = stateWrapper.getState(); if (state.getLastRecordedTime() < newLastRecordedTime) { state.setLastRecordedTime(newLastRecordedTime); } - shouldReport.set(state.getStrategy().onActivity()); - return state; + shouldReport.set(stateWrapper.getStrategy().onActivity()); + return stateWrapper; }); - if (activityState == null) { + if (activityStateWrapper == null) { return; } + var activityState = activityStateWrapper.getState(); long lastRecordedTime = activityState.getLastRecordedTime(); - long lastReportedTime = activityState.getLastReportedTime(); + long lastReportedTime = activityStateWrapper.getLastReportedTime(); if (shouldReport.get() && lastReportedTime < lastRecordedTime) { log.debug("[{}] Going to report first activity event for key: [{}].", name, key); reportActivity(key, activityState.getMetadata(), lastRecordedTime, new ActivityReportCallback<>() { @@ -131,18 +148,19 @@ public abstract class AbstractActivityManager implements Activity @Override public long getLastRecordedTime(Key key) { - ActivityState state = states.get(key); - return state == null ? 0L : state.getLastRecordedTime(); + ActivityStateWrapper stateWrapper = states.get(key); + return stateWrapper == null ? 0L : stateWrapper.getState().getLastRecordedTime(); } private void onReportingPeriodEnd() { log.debug("[{}] Going to end reporting period.", name); - for (Map.Entry> entry : states.entrySet()) { + for (Map.Entry entry : states.entrySet()) { var key = entry.getKey(); - var currentState = entry.getValue(); + var stateWrapper = entry.getValue(); + var currentState = stateWrapper.getState(); long lastRecordedTime = currentState.getLastRecordedTime(); - long lastReportedTime = currentState.getLastReportedTime(); + long lastReportedTime = stateWrapper.getLastReportedTime(); var metadata = currentState.getMetadata(); boolean hasExpired; @@ -151,10 +169,9 @@ public abstract class AbstractActivityManager implements Activity var updatedState = updateState(key, currentState); if (updatedState != null) { lastRecordedTime = updatedState.getLastRecordedTime(); - lastReportedTime = updatedState.getLastReportedTime(); metadata = updatedState.getMetadata(); hasExpired = hasExpired(lastRecordedTime); - shouldReport = updatedState.getStrategy().onReportingPeriodEnd(); + shouldReport = stateWrapper.getStrategy().onReportingPeriodEnd(); } else { states.remove(key); hasExpired = false; @@ -185,9 +202,9 @@ public abstract class AbstractActivityManager implements Activity } private void updateLastReportedTime(Key key, long newLastReportedTime) { - states.computeIfPresent(key, (__, state) -> { - state.setLastReportedTime(Math.max(state.getLastReportedTime(), newLastReportedTime)); - return state; + states.computeIfPresent(key, (__, stateWrapper) -> { + stateWrapper.setLastReportedTime(Math.max(stateWrapper.getLastReportedTime(), newLastReportedTime)); + return stateWrapper; }); } diff --git a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/ActivityState.java b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/ActivityState.java index f0f607ddd3..b470a52d9d 100644 --- a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/ActivityState.java +++ b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/ActivityState.java @@ -31,14 +31,11 @@ package org.thingsboard.server.common.transport.activity; import lombok.Data; -import org.thingsboard.server.common.transport.activity.strategy.ActivityStrategy; @Data public class ActivityState { private volatile long lastRecordedTime; - private volatile long lastReportedTime; - private volatile ActivityStrategy strategy; private volatile Metadata metadata; } diff --git a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/service/TransportActivityState.java b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyType.java similarity index 68% rename from common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/service/TransportActivityState.java rename to common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyType.java index 470699ec5d..bda213936e 100644 --- a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/service/TransportActivityState.java +++ b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyType.java @@ -28,17 +28,35 @@ * DOES NOT CONVEY OR IMPLY ANY RIGHTS TO REPRODUCE, DISCLOSE OR DISTRIBUTE ITS CONTENTS, * OR TO MANUFACTURE, USE, OR SELL ANYTHING THAT IT MAY DESCRIBE, IN WHOLE OR IN PART. */ -package org.thingsboard.server.common.transport.service; +package org.thingsboard.server.common.transport.activity.strategy; -import lombok.Data; -import lombok.EqualsAndHashCode; -import org.thingsboard.server.common.transport.activity.ActivityState; -import org.thingsboard.server.gen.transport.TransportProtos; +public enum ActivityStrategyType { -@Data -@EqualsAndHashCode(callSuper = true) -public class TransportActivityState extends ActivityState { + ALL { + @Override + public ActivityStrategy toStrategy() { + return AllEventsActivityStrategy.getInstance(); + } + }, + FIRST { + @Override + public ActivityStrategy toStrategy() { + return new FirstEventActivityStrategy(); + } + }, + LAST { + @Override + public ActivityStrategy toStrategy() { + return LastEventActivityStrategy.getInstance(); + } + }, + FIRST_AND_LAST { + @Override + public ActivityStrategy toStrategy() { + return new FirstAndLastEventActivityStrategy(); + } + }; - private volatile TransportProtos.SessionInfoProto sessionInfoProto; + public abstract ActivityStrategy toStrategy(); } diff --git a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/AllEventsActivityStrategy.java b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/AllEventsActivityStrategy.java index ba7ceab517..6af3469fd5 100644 --- a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/AllEventsActivityStrategy.java +++ b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/AllEventsActivityStrategy.java @@ -30,10 +30,16 @@ */ package org.thingsboard.server.common.transport.activity.strategy; -import lombok.EqualsAndHashCode; +public final class AllEventsActivityStrategy implements ActivityStrategy { -@EqualsAndHashCode -public class AllEventsActivityStrategy implements ActivityStrategy { + private static final AllEventsActivityStrategy INSTANCE = new AllEventsActivityStrategy(); + + private AllEventsActivityStrategy() { + } + + public static AllEventsActivityStrategy getInstance() { + return INSTANCE; + } @Override public boolean onActivity() { diff --git a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/FirstAndLastEventActivityStrategy.java b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/FirstAndLastEventActivityStrategy.java index 225d02ccb8..71682da1ac 100644 --- a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/FirstAndLastEventActivityStrategy.java +++ b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/FirstAndLastEventActivityStrategy.java @@ -33,7 +33,7 @@ package org.thingsboard.server.common.transport.activity.strategy; import lombok.EqualsAndHashCode; @EqualsAndHashCode -public class FirstAndLastEventActivityStrategy implements ActivityStrategy { +public final class FirstAndLastEventActivityStrategy implements ActivityStrategy { private boolean firstEventReceived; diff --git a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/FirstEventActivityStrategy.java b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/FirstEventActivityStrategy.java index f4f53de6c0..66c2b8a8e2 100644 --- a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/FirstEventActivityStrategy.java +++ b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/FirstEventActivityStrategy.java @@ -33,7 +33,7 @@ package org.thingsboard.server.common.transport.activity.strategy; import lombok.EqualsAndHashCode; @EqualsAndHashCode -public class FirstEventActivityStrategy implements ActivityStrategy { +public final class FirstEventActivityStrategy implements ActivityStrategy { private boolean firstEventReceived; diff --git a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/LastEventActivityStrategy.java b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/LastEventActivityStrategy.java index f16fbed8d6..fd9fe1f3eb 100644 --- a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/LastEventActivityStrategy.java +++ b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/LastEventActivityStrategy.java @@ -30,10 +30,16 @@ */ package org.thingsboard.server.common.transport.activity.strategy; -import lombok.EqualsAndHashCode; +public final class LastEventActivityStrategy implements ActivityStrategy { -@EqualsAndHashCode -public class LastEventActivityStrategy implements ActivityStrategy { + private static final LastEventActivityStrategy INSTANCE = new LastEventActivityStrategy(); + + private LastEventActivityStrategy() { + } + + public static LastEventActivityStrategy getInstance() { + return INSTANCE; + } @Override public boolean onActivity() { diff --git a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/service/DefaultTransportService.java b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/service/DefaultTransportService.java index da0ef3ca99..fb14936ad5 100644 --- a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/service/DefaultTransportService.java +++ b/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/service/DefaultTransportService.java @@ -74,7 +74,8 @@ import org.thingsboard.server.common.transport.TransportTenantProfileCache; import org.thingsboard.server.common.transport.activity.AbstractActivityManager; import org.thingsboard.server.common.transport.activity.ActivityReportCallback; import org.thingsboard.server.common.transport.activity.ActivityState; -import org.thingsboard.server.common.transport.activity.strategy.ActivityStrategyFactory; +import org.thingsboard.server.common.transport.activity.strategy.ActivityStrategy; +import org.thingsboard.server.common.transport.activity.strategy.ActivityStrategyType; import org.thingsboard.server.common.transport.auth.GetOrCreateDeviceFromGatewayResponse; import org.thingsboard.server.common.transport.auth.TransportDeviceInfo; import org.thingsboard.server.common.transport.auth.ValidateDeviceCredentialsResponse; @@ -158,7 +159,7 @@ public class DefaultTransportService extends AbstractActivityManager state = new ActivityState<>(); state.setMetadata(session.getSessionInfo()); - state.setStrategy(ActivityStrategyFactory.createStrategy(reportingStrategyName)); return state; } + @Override + protected ActivityStrategy getStrategy() { + return reportingStrategyType.toStrategy(); + } + @Override protected ActivityState updateState(UUID sessionId, ActivityState state) { SessionMetaData session = sessions.get(sessionId); diff --git a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyFactoryTest.java b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyFactoryTest.java deleted file mode 100644 index c6eda7eb25..0000000000 --- a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyFactoryTest.java +++ /dev/null @@ -1,70 +0,0 @@ -/** - * ThingsBoard, Inc. ("COMPANY") CONFIDENTIAL - * - * Copyright © 2016-2023 ThingsBoard, Inc. All Rights Reserved. - * - * NOTICE: All information contained herein is, and remains - * the property of ThingsBoard, Inc. and its suppliers, - * if any. The intellectual and technical concepts contained - * herein are proprietary to ThingsBoard, Inc. - * and its suppliers and may be covered by U.S. and Foreign Patents, - * patents in process, and are protected by trade secret or copyright law. - * - * Dissemination of this information or reproduction of this material is strictly forbidden - * unless prior written permission is obtained from COMPANY. - * - * Access to the source code contained herein is hereby forbidden to anyone except current COMPANY employees, - * managers or contractors who have executed Confidentiality and Non-disclosure agreements - * explicitly covering such access. - * - * The copyright notice above does not evidence any actual or intended publication - * or disclosure of this source code, which includes - * information that is confidential and/or proprietary, and is a trade secret, of COMPANY. - * ANY REPRODUCTION, MODIFICATION, DISTRIBUTION, PUBLIC PERFORMANCE, - * OR PUBLIC DISPLAY OF OR THROUGH USE OF THIS SOURCE CODE WITHOUT - * THE EXPRESS WRITTEN CONSENT OF COMPANY IS STRICTLY PROHIBITED, - * AND IN VIOLATION OF APPLICABLE LAWS AND INTERNATIONAL TREATIES. - * THE RECEIPT OR POSSESSION OF THIS SOURCE CODE AND/OR RELATED INFORMATION - * DOES NOT CONVEY OR IMPLY ANY RIGHTS TO REPRODUCE, DISCLOSE OR DISTRIBUTE ITS CONTENTS, - * OR TO MANUFACTURE, USE, OR SELL ANYTHING THAT IT MAY DESCRIBE, IN WHOLE OR IN PART. - */ -package org.thingsboard.server.common.transport.activity.strategy; - -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertInstanceOf; -import static org.junit.jupiter.api.Assertions.assertThrows; - -public class ActivityStrategyFactoryTest { - - @Test - public void testCreateAllEventsStrategy() { - ActivityStrategy strategy = ActivityStrategyFactory.createStrategy("ALL"); - assertInstanceOf(AllEventsActivityStrategy.class, strategy, "Should return an instance of AllEventsActivityStrategy."); - } - - @Test - public void testCreateFirstEventStrategy() { - ActivityStrategy strategy = ActivityStrategyFactory.createStrategy("FIRST"); - assertInstanceOf(FirstEventActivityStrategy.class, strategy, "Should return an instance of FirstEventActivityStrategy."); - } - - @Test - public void testCreateLastEventStrategy() { - ActivityStrategy strategy = ActivityStrategyFactory.createStrategy("LAST"); - assertInstanceOf(LastEventActivityStrategy.class, strategy, "Should return an instance of LastEventActivityStrategy."); - } - - @Test - public void testCreateFirstAndLastEventStrategy() { - ActivityStrategy strategy = ActivityStrategyFactory.createStrategy("FIRST_AND_LAST"); - assertInstanceOf(FirstAndLastEventActivityStrategy.class, strategy, "Should return an instance of FirstAndLastEventActivityStrategy."); - } - - @Test - public void testCreateUnknownStrategy() { - assertThrows(IllegalArgumentException.class, () -> ActivityStrategyFactory.createStrategy("UNKNOWN"), - "Should throw IllegalArgumentException for unknown strategy names."); - } - -} diff --git a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyFactory.java b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyTypeTest.java similarity index 62% rename from common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyFactory.java rename to common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyTypeTest.java index f4619e2afd..854659c1bf 100644 --- a/common/transport/transport-api/src/main/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyFactory.java +++ b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/ActivityStrategyTypeTest.java @@ -30,29 +30,30 @@ */ package org.thingsboard.server.common.transport.activity.strategy; -public final class ActivityStrategyFactory { +import org.junit.jupiter.api.Test; - private static final String ALL_EVENTS_STRATEGY_NAME = "ALL"; - private static final String FIRST_EVENT_STRATEGY_NAME = "FIRST"; - private static final String LAST_EVENT_STRATEGY_NAME = "LAST"; - private static final String FIRST_AND_LAST_STRATEGY_NAME = "FIRST_AND_LAST"; +import static org.assertj.core.api.Assertions.assertThat; - private ActivityStrategyFactory() { +public class ActivityStrategyTypeTest { + + @Test + public void testCreateAllEventsStrategy() { + assertThat(ActivityStrategyType.ALL.toStrategy()).isEqualTo(AllEventsActivityStrategy.getInstance()); } - public static ActivityStrategy createStrategy(String strategyName) { - switch (strategyName) { - case ALL_EVENTS_STRATEGY_NAME: - return new AllEventsActivityStrategy(); - case FIRST_EVENT_STRATEGY_NAME: - return new FirstEventActivityStrategy(); - case LAST_EVENT_STRATEGY_NAME: - return new LastEventActivityStrategy(); - case FIRST_AND_LAST_STRATEGY_NAME: - return new FirstAndLastEventActivityStrategy(); - default: - throw new IllegalArgumentException("Unknown activity strategy name: [" + strategyName + "."); - } + @Test + public void testCreateFirstEventStrategy() { + assertThat(ActivityStrategyType.FIRST.toStrategy()).isEqualTo(new FirstEventActivityStrategy()); + } + + @Test + public void testCreateLastEventStrategy() { + assertThat(ActivityStrategyType.LAST.toStrategy()).isEqualTo(LastEventActivityStrategy.getInstance()); + } + + @Test + public void testCreateFirstAndLastEventStrategy() { + assertThat(ActivityStrategyType.FIRST_AND_LAST.toStrategy()).isEqualTo(new FirstAndLastEventActivityStrategy()); } } diff --git a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/AllEventsActivityStrategyTest.java b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/AllEventsActivityStrategyTest.java index 22cbc10ca2..01f4bbcef1 100644 --- a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/AllEventsActivityStrategyTest.java +++ b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/AllEventsActivityStrategyTest.java @@ -41,7 +41,7 @@ public class AllEventsActivityStrategyTest { @BeforeEach public void setUp() { - strategy = new AllEventsActivityStrategy(); + strategy = AllEventsActivityStrategy.getInstance(); } @Test diff --git a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/LastEventActivityStrategyTest.java b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/LastEventActivityStrategyTest.java index 51da85a9c3..7b49c2215b 100644 --- a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/LastEventActivityStrategyTest.java +++ b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/activity/strategy/LastEventActivityStrategyTest.java @@ -42,7 +42,7 @@ public class LastEventActivityStrategyTest { @BeforeEach public void setUp() { - strategy = new LastEventActivityStrategy(); + strategy = LastEventActivityStrategy.getInstance(); } @Test diff --git a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/TransportActivityManagerTest.java b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/TransportActivityManagerTest.java index 6ad48043ce..c966cc5535 100644 --- a/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/TransportActivityManagerTest.java +++ b/common/transport/transport-api/src/test/java/org/thingsboard/server/common/transport/service/TransportActivityManagerTest.java @@ -35,6 +35,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.MethodSource; import org.mockito.ArgumentCaptor; import org.mockito.Mock; @@ -45,10 +46,7 @@ import org.thingsboard.server.common.transport.TransportServiceCallback; import org.thingsboard.server.common.transport.activity.ActivityReportCallback; import org.thingsboard.server.common.transport.activity.ActivityState; import org.thingsboard.server.common.transport.activity.strategy.ActivityStrategy; -import org.thingsboard.server.common.transport.activity.strategy.AllEventsActivityStrategy; -import org.thingsboard.server.common.transport.activity.strategy.FirstAndLastEventActivityStrategy; -import org.thingsboard.server.common.transport.activity.strategy.FirstEventActivityStrategy; -import org.thingsboard.server.common.transport.activity.strategy.LastEventActivityStrategy; +import org.thingsboard.server.common.transport.activity.strategy.ActivityStrategyType; import org.thingsboard.server.gen.transport.TransportProtos; import java.util.UUID; @@ -60,9 +58,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.thingsboard.server.common.transport.service.DefaultTransportService.SESSION_EVENT_MSG_CLOSED; import static org.thingsboard.server.common.transport.service.DefaultTransportService.SESSION_EXPIRED_NOTIFICATION_PROTO; @@ -195,25 +191,18 @@ public class TransportActivityManagerTest { verify(transportServiceMock).onActivity(SESSION_ID); } - @ParameterizedTest - @MethodSource("provideTestParamsForCreateNewState") - void givenDifferentReportingStrategies_whenCreatingNewState_thenShouldCreateEmptyStateWithCorrectStrategy( - String reportingStrategyName, ActivityStrategy reportingStrategy - ) { + @Test + void givenKey_whenCreatingNewState_thenShouldCorrectlyCreateNewEmptyState() { // GIVEN TransportProtos.SessionInfoProto sessionInfo = TransportProtos.SessionInfoProto.newBuilder() .setSessionIdMSB(SESSION_ID.getMostSignificantBits()) .setSessionIdLSB(SESSION_ID.getLeastSignificantBits()) .build(); - SessionMsgListener listenerMock = mock(SessionMsgListener.class); - sessions.put(SESSION_ID, new SessionMetaData(sessionInfo, TransportProtos.SessionType.ASYNC, listenerMock)); - - ReflectionTestUtils.setField(transportServiceMock, "reportingStrategyName", reportingStrategyName); + sessions.put(SESSION_ID, new SessionMetaData(sessionInfo, TransportProtos.SessionType.ASYNC, null)); when(transportServiceMock.createNewState(SESSION_ID)).thenCallRealMethod(); ActivityState expectedState = new ActivityState<>(); - expectedState.setStrategy(reportingStrategy); expectedState.setMetadata(sessionInfo); // WHEN @@ -223,13 +212,18 @@ public class TransportActivityManagerTest { assertThat(actualState).isEqualTo(expectedState); } - private static Stream provideTestParamsForCreateNewState() { - return Stream.of( - Arguments.of("ALL", new AllEventsActivityStrategy()), - Arguments.of("FIRST", new FirstEventActivityStrategy()), - Arguments.of("LAST", new LastEventActivityStrategy()), - Arguments.of("FIRST_AND_LAST", new FirstAndLastEventActivityStrategy()) - ); + @ParameterizedTest + @EnumSource(ActivityStrategyType.class) + void givenDifferentReportingStrategies_whenGettingStrategy_thenShouldReturnCorrectStrategy(ActivityStrategyType reportingStrategyType) { + // GIVEN + doCallRealMethod().when(transportServiceMock).getStrategy(); + ReflectionTestUtils.setField(transportServiceMock, "reportingStrategyType", reportingStrategyType); + + // WHEN + ActivityStrategy actualStrategy = transportServiceMock.getStrategy(); + + // THEN + assertThat(actualStrategy).isEqualTo(reportingStrategyType.toStrategy()); } @Test @@ -242,9 +236,7 @@ public class TransportActivityManagerTest { ActivityState state = new ActivityState<>(); state.setLastRecordedTime(123L); - state.setLastReportedTime(312L); state.setMetadata(sessionInfo); - state.setStrategy(new FirstEventActivityStrategy()); when(transportServiceMock.updateState(SESSION_ID, state)).thenCallRealMethod(); @@ -266,14 +258,10 @@ public class TransportActivityManagerTest { sessions.put(SESSION_ID, new SessionMetaData(sessionInfo, TransportProtos.SessionType.ASYNC, listenerMock)); long lastRecordedTime = 123L; - long lastReportedTime = 312L; - ActivityStrategy strategySpy = spy(new FirstEventActivityStrategy()); ActivityState state = new ActivityState<>(); state.setLastRecordedTime(lastRecordedTime); - state.setLastReportedTime(lastReportedTime); state.setMetadata(TransportProtos.SessionInfoProto.getDefaultInstance()); - state.setStrategy(strategySpy); when(transportServiceMock.updateState(SESSION_ID, state)).thenCallRealMethod(); @@ -283,10 +271,7 @@ public class TransportActivityManagerTest { // THEN assertThat(updatedState).isSameAs(state); assertThat(updatedState.getLastRecordedTime()).isEqualTo(lastRecordedTime); - assertThat(updatedState.getLastReportedTime()).isEqualTo(lastReportedTime); assertThat(updatedState.getMetadata()).isEqualTo(sessionInfo); - assertThat(updatedState.getStrategy()).isEqualTo(strategySpy); - verifyNoInteractions(strategySpy); } @Test @@ -303,14 +288,10 @@ public class TransportActivityManagerTest { sessions.put(SESSION_ID, new SessionMetaData(sessionInfo, TransportProtos.SessionType.ASYNC, listenerMock)); long lastRecordedTime = 123L; - long lastReportedTime = 312L; - ActivityStrategy strategySpy = spy(new FirstEventActivityStrategy()); ActivityState state = new ActivityState<>(); state.setLastRecordedTime(lastRecordedTime); - state.setLastReportedTime(lastReportedTime); state.setMetadata(TransportProtos.SessionInfoProto.getDefaultInstance()); - state.setStrategy(strategySpy); when(transportServiceMock.updateState(SESSION_ID, state)).thenCallRealMethod(); @@ -320,10 +301,7 @@ public class TransportActivityManagerTest { // THEN assertThat(updatedState).isSameAs(state); assertThat(updatedState.getLastRecordedTime()).isEqualTo(lastRecordedTime); - assertThat(updatedState.getLastReportedTime()).isEqualTo(lastReportedTime); assertThat(updatedState.getMetadata()).isEqualTo(sessionInfo); - assertThat(updatedState.getStrategy()).isEqualTo(strategySpy); - verifyNoInteractions(strategySpy); verify(transportServiceMock, never()).getLastRecordedTime(gwSessionId); } @@ -349,14 +327,10 @@ public class TransportActivityManagerTest { sessions.put(SESSION_ID, new SessionMetaData(sessionInfo, TransportProtos.SessionType.ASYNC, listenerMock)); long lastRecordedTime = 123L; - long lastReportedTime = 312L; - ActivityStrategy strategySpy = spy(new FirstEventActivityStrategy()); ActivityState state = new ActivityState<>(); state.setLastRecordedTime(lastRecordedTime); - state.setLastReportedTime(lastReportedTime); state.setMetadata(TransportProtos.SessionInfoProto.getDefaultInstance()); - state.setStrategy(strategySpy); when(transportServiceMock.updateState(SESSION_ID, state)).thenCallRealMethod(); @@ -366,10 +340,7 @@ public class TransportActivityManagerTest { // THEN assertThat(updatedState).isSameAs(state); assertThat(updatedState.getLastRecordedTime()).isEqualTo(lastRecordedTime); - assertThat(updatedState.getLastReportedTime()).isEqualTo(lastReportedTime); assertThat(updatedState.getMetadata()).isEqualTo(sessionInfo); - assertThat(updatedState.getStrategy()).isEqualTo(strategySpy); - verifyNoInteractions(strategySpy); verify(transportServiceMock, never()).getLastRecordedTime(gwSessionId); } @@ -400,14 +371,10 @@ public class TransportActivityManagerTest { sessions.put(SESSION_ID, new SessionMetaData(sessionInfo, TransportProtos.SessionType.ASYNC, listenerMock)); long lastRecordedTime = 123L; - long lastReportedTime = 312L; - ActivityStrategy strategySpy = spy(new FirstEventActivityStrategy()); ActivityState state = new ActivityState<>(); state.setLastRecordedTime(lastRecordedTime); - state.setLastReportedTime(lastReportedTime); state.setMetadata(TransportProtos.SessionInfoProto.getDefaultInstance()); - state.setStrategy(strategySpy); when(transportServiceMock.updateState(SESSION_ID, state)).thenCallRealMethod(); @@ -417,10 +384,7 @@ public class TransportActivityManagerTest { // THEN assertThat(updatedState).isSameAs(state); assertThat(updatedState.getLastRecordedTime()).isEqualTo(gwLastRecordedTime); - assertThat(updatedState.getLastReportedTime()).isEqualTo(lastReportedTime); assertThat(updatedState.getMetadata()).isEqualTo(sessionInfo); - assertThat(updatedState.getStrategy()).isEqualTo(strategySpy); - verifyNoInteractions(strategySpy); } @Test @@ -449,14 +413,10 @@ public class TransportActivityManagerTest { sessions.put(SESSION_ID, new SessionMetaData(sessionInfo, TransportProtos.SessionType.ASYNC, listenerMock)); long lastRecordedTime = 123L; - long lastReportedTime = 312L; - ActivityStrategy strategySpy = spy(new FirstEventActivityStrategy()); ActivityState state = new ActivityState<>(); state.setLastRecordedTime(lastRecordedTime); - state.setLastReportedTime(lastReportedTime); state.setMetadata(TransportProtos.SessionInfoProto.getDefaultInstance()); - state.setStrategy(strategySpy); when(transportServiceMock.updateState(SESSION_ID, state)).thenCallRealMethod(); @@ -466,10 +426,7 @@ public class TransportActivityManagerTest { // THEN assertThat(updatedState).isSameAs(state); assertThat(updatedState.getLastRecordedTime()).isEqualTo(lastRecordedTime); - assertThat(updatedState.getLastReportedTime()).isEqualTo(lastReportedTime); assertThat(updatedState.getMetadata()).isEqualTo(sessionInfo); - assertThat(updatedState.getStrategy()).isEqualTo(strategySpy); - verifyNoInteractions(strategySpy); } @ParameterizedTest