Skip to content

Commit 30660fc

Browse files
Legiothmshabarov
andauthored
feat: Make signal environment init optional (#22074)
* Make signal environment init optional Also removes the duplicate ___staticMocks variants in VaadinServiceSignalsInitializationTest Fixes #22033 * Update omitted Javadoc reference * Preserve original import order --------- Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
1 parent 55accfe commit 30660fc

File tree

12 files changed

+372
-525
lines changed

12 files changed

+372
-525
lines changed

flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
import java.util.concurrent.atomic.AtomicInteger;
5252
import java.util.concurrent.locks.Lock;
5353
import java.util.concurrent.locks.ReentrantLock;
54-
import java.util.function.Supplier;
5554
import java.util.stream.Collectors;
5655
import java.util.stream.Stream;
5756
import java.util.stream.StreamSupport;
@@ -357,51 +356,66 @@ public void init() throws ServiceException {
357356
}
358357

359358
private void initSignalsEnvironment() {
360-
Executor signalsExecutor;
361-
Supplier<Executor> flowDispatcherOverride;
362-
FeatureFlags featureFlags = FeatureFlags.get(getContext());
363-
if (featureFlags
364-
.isEnabled(FeatureFlags.FLOW_FULLSTACK_SIGNALS.getId())) {
365-
// Use getter method to trigger a multiple TaskExecutor check
366-
signalsExecutor = getExecutor();
367-
flowDispatcherOverride = () -> {
359+
boolean enabled = FeatureFlags.get(getContext())
360+
.isEnabled(FeatureFlags.FLOW_FULLSTACK_SIGNALS.getId());
361+
if (enabled) {
362+
// Trigger check for multiple TaskExecutor candidates
363+
getExecutor();
364+
}
365+
366+
class VaadinServiceEnvironment extends SignalEnvironment
367+
implements Serializable {
368+
@Override
369+
public boolean isActive() {
370+
if (VaadinService.getCurrent() != VaadinService.this) {
371+
return false;
372+
} else if (!enabled) {
373+
throw new DisabledFeatureException(
374+
FeatureFlags.FLOW_FULLSTACK_SIGNALS);
375+
} else {
376+
return true;
377+
}
378+
}
379+
380+
private Executor createCurrentUiDispatcher() {
368381
UI owner = UI.getCurrent();
369382
if (owner == null) {
370-
return null;
383+
return getExecutor();
371384
}
372385

373386
return task -> {
374387
if (UI.getCurrent() == owner) {
375388
task.run();
376389
} else {
377390
try {
378-
SignalEnvironment.defaultDispatcher()
391+
getExecutor()
379392
.execute(() -> owner.access(task::run));
380393
} catch (Exception e) {
381-
// a task is submitted when executor is shut down,
382-
// ignore
394+
// submitted when executor is shut down, ignore
383395
}
384396
}
385397
};
386-
};
387-
} else {
388-
signalsExecutor = task -> {
389-
throw new DisabledFeatureException(
390-
FeatureFlags.FLOW_FULLSTACK_SIGNALS);
391-
};
392-
flowDispatcherOverride = () -> {
393-
throw new DisabledFeatureException(
394-
FeatureFlags.FLOW_FULLSTACK_SIGNALS);
395-
};
396-
}
397-
if (!SignalEnvironment.tryInitialize(createDefaultObjectMapper(),
398-
signalsExecutor)) {
399-
getLogger().warn("Signals environment is already initialized. "
400-
+ "It is recommended to let Vaadin setup Signals environment to prevent unexpected behavior. "
401-
+ "Please, avoid calling SignalEnvironment.tryInitialize() in application code.");
398+
}
399+
400+
@Override
401+
public Executor getResultNotifier() {
402+
return createCurrentUiDispatcher();
403+
}
404+
405+
@Override
406+
public Executor getEffectDispatcher() {
407+
return createCurrentUiDispatcher();
408+
}
409+
410+
@Override
411+
public Executor getFallbackEffectDispatcher() {
412+
return getExecutor();
413+
}
402414
}
415+
403416
Runnable unregister = SignalEnvironment
404-
.addDispatcherOverride(flowDispatcherOverride);
417+
.register(new VaadinServiceEnvironment());
418+
405419
addServiceDestroyListener(event -> unregister.run());
406420
}
407421

flow-server/src/test/java/com/vaadin/flow/component/ComponentEffectTest.java

Lines changed: 11 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,51 +16,29 @@
1616
package com.vaadin.flow.component;
1717

1818
import static org.junit.Assert.assertEquals;
19-
import static org.junit.Assert.assertFalse;
20-
import static org.junit.Assert.assertNotNull;
21-
import static org.junit.Assert.assertThrows;
22-
import static org.junit.Assert.assertTrue;
23-
import static org.junit.Assert.fail;
2419
import static org.mockito.ArgumentMatchers.any;
2520
import static org.mockito.Mockito.mock;
2621
import static org.mockito.Mockito.mockStatic;
2722
import static org.mockito.Mockito.when;
2823

29-
import java.util.ArrayList;
30-
import java.util.List;
3124
import java.util.Locale;
32-
import java.util.concurrent.Executor;
33-
import java.util.concurrent.Executors;
34-
import java.util.concurrent.atomic.AtomicBoolean;
3525
import java.util.concurrent.atomic.AtomicInteger;
36-
import java.util.concurrent.atomic.AtomicReference;
3726

38-
import com.fasterxml.jackson.databind.ObjectMapper;
39-
import org.junit.AfterClass;
40-
import org.junit.Assert;
41-
import org.junit.Before;
42-
import org.junit.BeforeClass;
4327
import org.junit.Test;
4428

45-
import com.vaadin.experimental.DisabledFeatureException;
4629
import com.vaadin.experimental.FeatureFlags;
4730
import com.vaadin.flow.dom.Element;
48-
import com.vaadin.flow.function.SerializableBiConsumer;
49-
import com.vaadin.flow.server.MockVaadinServletService;
31+
import com.vaadin.flow.internal.CurrentInstance;
32+
import com.vaadin.flow.server.VaadinService;
5033
import com.vaadin.flow.shared.Registration;
5134
import com.vaadin.signals.NumberSignal;
52-
import com.vaadin.signals.Signal;
53-
import com.vaadin.signals.SignalEnvironment;
5435
import com.vaadin.signals.ValueSignal;
5536
import com.vaadin.tests.util.MockUI;
5637

5738
public class ComponentEffectTest {
58-
private static final Executor executor = Runnable::run;
59-
private static final ObjectMapper objectMapper = new ObjectMapper();
60-
6139
@Test
6240
public void effect_componentAttachedAndDetached_effectEnabledAndDisabled() {
63-
runWithSignalEnvironmentMocks(() -> {
41+
runWithFeatureFlagEnabled(() -> {
6442
TestComponent component = new TestComponent();
6543
ValueSignal<String> signal = new ValueSignal<>("initial");
6644
AtomicInteger count = new AtomicInteger();
@@ -106,7 +84,7 @@ public void effect_componentAttachedAndDetached_effectEnabledAndDisabled() {
10684

10785
@Test
10886
public void bind_signalValueChanges_componentUpdated() {
109-
runWithSignalEnvironmentMocks(() -> {
87+
runWithFeatureFlagEnabled(() -> {
11088
TestComponent component = new TestComponent();
11189
ValueSignal<String> signal = new ValueSignal<>("initial");
11290

@@ -144,7 +122,7 @@ public void bind_signalValueChanges_componentUpdated() {
144122

145123
@Test
146124
public void format_customLocale_signalValuesChange_formattedStringUpdated() {
147-
runWithSignalEnvironmentMocks(() -> {
125+
runWithFeatureFlagEnabled(() -> {
148126
TestComponent component = new TestComponent();
149127

150128
MockUI ui = new MockUI();
@@ -187,7 +165,7 @@ public void format_customLocale_signalValuesChange_formattedStringUpdated() {
187165

188166
@Test
189167
public void format_defaultLocale_signalValuesChange_formattedStringUpdated() {
190-
runWithSignalEnvironmentMocks(() -> {
168+
runWithFeatureFlagEnabled(() -> {
191169
TestComponent component = new TestComponent();
192170

193171
MockUI ui = new MockUI();
@@ -204,31 +182,17 @@ public void format_defaultLocale_signalValuesChange_formattedStringUpdated() {
204182
});
205183
}
206184

207-
/**
208-
* Other tests may already have initialized the environment with the feature
209-
* flag off and executors that would throw an exception, so it's too late
210-
* now to mock the feature flags. Thus we need to "reinitialize" the
211-
* environment.
212-
*/
213-
private static void runWithSignalEnvironmentMocks(Runnable test) {
214-
try (var environment = mockStatic(SignalEnvironment.class);
215-
var featureFlagStaticMock = mockStatic(FeatureFlags.class)) {
185+
private static void runWithFeatureFlagEnabled(Runnable test) {
186+
try (var featureFlagStaticMock = mockStatic(FeatureFlags.class)) {
216187
FeatureFlags flags = mock(FeatureFlags.class);
217188
when(flags.isEnabled(FeatureFlags.FLOW_FULLSTACK_SIGNALS.getId()))
218189
.thenReturn(true);
219190
featureFlagStaticMock.when(() -> FeatureFlags.get(any()))
220191
.thenReturn(flags);
221-
environment.when(() -> SignalEnvironment.initialized())
222-
.thenReturn(true);
223-
environment.when(() -> SignalEnvironment.defaultDispatcher())
224-
.thenReturn(executor);
225-
environment.when(() -> SignalEnvironment.synchronousDispatcher())
226-
.thenReturn(executor);
227-
environment.when(() -> SignalEnvironment.asynchronousDispatcher())
228-
.thenReturn(executor);
229-
environment.when(() -> SignalEnvironment.objectMapper())
230-
.thenReturn(objectMapper);
231192
test.run();
193+
} finally {
194+
VaadinService.getCurrent().destroy();
195+
CurrentInstance.clearAll();
232196
}
233197
}
234198

flow-server/src/test/java/com/vaadin/flow/server/MockVaadinServletService.java

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
import java.lang.reflect.Field;
2121
import java.util.Collections;
2222
import java.util.List;
23-
import java.util.concurrent.CopyOnWriteArrayList;
24-
import java.util.concurrent.atomic.AtomicReference;
2523

2624
import org.mockito.Mockito;
2725

@@ -168,15 +166,10 @@ public DeploymentConfiguration getDeploymentConfiguration() {
168166

169167
private void resetSignalEnvironment() {
170168
try {
171-
Field state = SignalEnvironment.class.getDeclaredField("state");
172-
state.setAccessible(true);
173-
((AtomicReference<?>) state.get(null)).set(null);
174-
175-
Field dispatcherOverrides = SignalEnvironment.class
176-
.getDeclaredField("dispatcherOverrides");
177-
dispatcherOverrides.setAccessible(true);
178-
((List<?>) dispatcherOverrides.get(null)).clear();
179-
169+
Field environments = SignalEnvironment.class
170+
.getDeclaredField("environments");
171+
environments.setAccessible(true);
172+
((List<?>) environments.get(null)).clear();
180173
} catch (Exception e) {
181174
throw new AssertionError("Failed to reset Signal environment", e);
182175
}

0 commit comments

Comments
 (0)