Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Merged
merged 1 commit into from
Dec 23, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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) {
Original file line number Diff line number Diff line change
@@ -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"));
}
}
15 changes: 0 additions & 15 deletions packages/ts/react-signals/src/FullStackSignal.ts
Original file line number Diff line number Diff line change
@@ -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;
41 changes: 0 additions & 41 deletions packages/ts/react-signals/test/FeatureFlag.spec.ts

This file was deleted.

2 changes: 0 additions & 2 deletions packages/ts/react-signals/test/FullStackSignal.spec.tsx
Original file line number Diff line number Diff line change
@@ -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';
2 changes: 0 additions & 2 deletions packages/ts/react-signals/test/NumberSignal.spec.tsx
Original file line number Diff line number Diff line change
@@ -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';
2 changes: 0 additions & 2 deletions packages/ts/react-signals/test/ValueSignal.spec.tsx
Original file line number Diff line number Diff line change
@@ -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';
3 changes: 0 additions & 3 deletions packages/ts/react-signals/test/events.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// eslint-disable-next-line import/no-unassigned-import
import './setup.js';

import { expect } from 'chai';
import {
createIncrementStateEvent,
19 changes: 0 additions & 19 deletions packages/ts/react-signals/test/setup.ts

This file was deleted.

Loading