fix: throw exception if feature flag is not enabled (#2998) (CP: 24.6) (

#3086) fix: throw exception if feature flag is not enabled (#2998) * fix: throw exception if feature flag is not enabled This also removes the client-side check for the feature flag to let the UI being rendered normally, and server-side get a chance to handle the requests for subscribe or update. Fixes #2937 * remove feature flag test * refine the import * apply changes from the review Co-authored-by: Soroosh Taefi <[email protected]>
vaadin · Dec 23, 2024 · a5342d5 · a5342d5
1 parent 3d40552
commit a5342d5
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 88 deletions.
diff --git a/...ges/java/endpoint/src/main/java/com/vaadin/hilla/signals/config/SignalsConfiguration.java b/...ges/java/endpoint/src/main/java/com/vaadin/hilla/signals/config/SignalsConfiguration.java
@@ -22,6 +22,7 @@
 import com.vaadin.hilla.signals.Signal;
 import com.vaadin.hilla.signals.core.registry.SecureSignalsRegistry;
 import com.vaadin.hilla.signals.handler.SignalsHandler;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
@@ -62,11 +63,11 @@ public SecureSignalsRegistry signalsRegistry() {
      *
      * @return SignalsHandler endpoint instance
      */
-    @ConditionalOnFeatureFlag("fullstackSignals")
     @Bean
-    public SignalsHandler signalsHandler() {
+    public SignalsHandler signalsHandler(
+            @Autowired(required = false) SecureSignalsRegistry signalsRegistry) {
         if (signalsHandler == null) {
-            signalsHandler = new SignalsHandler(signalsRegistry());
+            signalsHandler = new SignalsHandler(signalsRegistry);
         }
         return signalsHandler;
     }

diff --git a/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/handler/SignalsHandler.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/handler/SignalsHandler.java
@@ -32,9 +32,19 @@
 @BrowserCallable
 public class SignalsHandler {
 
+    private static final String FEATURE_FLAG_ERROR_MESSAGE = """
+            %n
+            ***********************************************************************************************************************
+            *  The Hilla Fullstack Signals API is currently considered experimental and may change in the future.                 *
+            *  To use it you need to explicitly enable it in Copilot, or by adding com.vaadin.experimental.fullstackSignals=true  *
+            *    to src/main/resources/vaadin-featureflags.properties.                                                            *
+            ***********************************************************************************************************************
+            %n"""
+            .stripIndent();
+
     private final SecureSignalsRegistry registry;
 
-    public SignalsHandler(SecureSignalsRegistry registry) {
+    public SignalsHandler(@Nullable SecureSignalsRegistry registry) {
         this.registry = registry;
     }
 
@@ -53,6 +63,10 @@ public SignalsHandler(SecureSignalsRegistry registry) {
     public Flux<ObjectNode> subscribe(String providerEndpoint,
             String providerMethod, String clientSignalId, ObjectNode body,
             @Nullable String parentClientSignalId) {
+        if (registry == null) {
+            throw new IllegalStateException(
+                    String.format(FEATURE_FLAG_ERROR_MESSAGE));
+        }
         try {
             if (parentClientSignalId != null) {
                 return subscribe(parentClientSignalId, clientSignalId);
@@ -96,6 +110,10 @@ private Flux<ObjectNode> subscribe(String parentClientSignalId,
     public void update(String clientSignalId, ObjectNode event)
             throws EndpointInvocationException.EndpointAccessDeniedException,
             EndpointInvocationException.EndpointNotFoundException {
+        if (registry == null) {
+            throw new IllegalStateException(
+                    String.format(FEATURE_FLAG_ERROR_MESSAGE));
+        }
         var parentSignalId = ListStateEvent.extractParentSignalId(event);
         if (parentSignalId != null) {
             if (registry.get(parentSignalId) == null) {

diff --git a/...ages/java/endpoint/src/test/java/com/vaadin/hilla/signals/handler/SignalsHandlerTest.java b/...ages/java/endpoint/src/test/java/com/vaadin/hilla/signals/handler/SignalsHandlerTest.java
@@ -22,6 +22,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.when;
 
 public class SignalsHandlerTest {
@@ -234,4 +235,19 @@ public void when_parentSignalIdIsNotNull_andParentSignalDoesNotExist_update_thro
         Mockito.verify(signalsRegistry, Mockito.never())
                 .get(CLIENT_SIGNAL_ID_1);
     }
+
+    @Test
+    public void when_signalRegistryIsNull_anyInteraction_throwsException() {
+        signalsHandler = new SignalsHandler(null);
+        var exception = assertThrows(IllegalStateException.class,
+                () -> signalsHandler.subscribe("endpoint", "method",
+                        CLIENT_SIGNAL_ID_1, null, null));
+        assertTrue(exception.getMessage().contains(
+                "The Hilla Fullstack Signals API is currently considered experimental"));
+
+        exception = assertThrows(IllegalStateException.class,
+                () -> signalsHandler.update(CLIENT_SIGNAL_ID_1, null));
+        assertTrue(exception.getMessage().contains(
+                "The Hilla Fullstack Signals API is currently considered experimental"));
+    }
 }
diff --git a/packages/ts/react-signals/src/FullStackSignal.ts b/packages/ts/react-signals/src/FullStackSignal.ts
@@ -3,14 +3,6 @@ import { nanoid } from 'nanoid';
 import { computed, signal, Signal } from './core.js';
 import { createSetStateEvent, type StateEvent } from './events.js';
 
-interface VaadinGlobal {
-  Vaadin?: {
-    featureFlags?: {
-      fullstackSignals?: boolean;
-    };
-  };
-}
-
 const ENDPOINT = 'SignalsHandler';
 
 /**
@@ -35,13 +27,6 @@ export abstract class DependencyTrackingSignal<T> extends Signal<T> {
   #subscribeCount = -1;
 
   protected constructor(value: T | undefined, onFirstSubscribe: () => void, onLastUnsubscribe: () => void) {
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-    if (!(globalThis as VaadinGlobal).Vaadin?.featureFlags?.fullstackSignals) {
-      // Remove when removing feature flag
-      throw new Error(
-        `The Hilla Fullstack Signals API is currently considered experimental and may change in the future. To use it you need to explicitly enable it in Copilot or by adding com.vaadin.experimental.fullstackSignals=true to vaadin-featureflags.properties`,
-      );
-    }
     super(value);
     this.#onFirstSubscribe = onFirstSubscribe;
     this.#onLastUnsubscribe = onLastUnsubscribe;

diff --git a/packages/ts/react-signals/test/FeatureFlag.spec.ts b/packages/ts/react-signals/test/FeatureFlag.spec.ts
diff --git a/packages/ts/react-signals/test/FullStackSignal.spec.tsx b/packages/ts/react-signals/test/FullStackSignal.spec.tsx
@@ -1,6 +1,4 @@
 /* eslint-disable @typescript-eslint/unbound-method */
-// eslint-disable-next-line import/no-unassigned-import
-import './setup.js';
 
 import { render } from '@testing-library/react';
 import { ActionOnLostSubscription, ConnectClient, type Subscription } from '@vaadin/hilla-frontend';

diff --git a/packages/ts/react-signals/test/NumberSignal.spec.tsx b/packages/ts/react-signals/test/NumberSignal.spec.tsx
@@ -1,6 +1,4 @@
 /* eslint-disable @typescript-eslint/unbound-method */
-// eslint-disable-next-line import/no-unassigned-import
-import './setup.js';
 
 import { render } from '@testing-library/react';
 import { ConnectClient, type Subscription } from '@vaadin/hilla-frontend';

diff --git a/packages/ts/react-signals/test/ValueSignal.spec.tsx b/packages/ts/react-signals/test/ValueSignal.spec.tsx
@@ -1,6 +1,4 @@
 /* eslint-disable @typescript-eslint/unbound-method */
-// eslint-disable-next-line import/no-unassigned-import
-import './setup.js';
 
 import { render } from '@testing-library/react';
 import { ConnectClient, type Subscription } from '@vaadin/hilla-frontend';

diff --git a/packages/ts/react-signals/test/events.spec.ts b/packages/ts/react-signals/test/events.spec.ts
@@ -1,6 +1,3 @@
-// eslint-disable-next-line import/no-unassigned-import
-import './setup.js';
-
 import { expect } from 'chai';
 import {
   createIncrementStateEvent,

diff --git a/packages/ts/react-signals/test/setup.ts b/packages/ts/react-signals/test/setup.ts