diff --git a/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/Signal.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/Signal.java index c574bbe11..6ddeef46b 100644 --- a/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/Signal.java +++ b/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/Signal.java @@ -16,8 +16,6 @@ public abstract class Signal { - private static final Logger LOGGER = LoggerFactory.getLogger(Signal.class); - private final ReentrantLock lock = new ReentrantLock(); private final UUID id = UUID.randomUUID(); @@ -76,7 +74,7 @@ public Flux subscribe() { .onBackpressureBuffer(); return sink.asFlux().doOnSubscribe(ignore -> { - LOGGER.debug("New Flux subscription..."); + getLogger().debug("New Flux subscription..."); lock.lock(); try { var snapshot = createSnapshotEvent(); @@ -88,7 +86,7 @@ public Flux subscribe() { }).doFinally(ignore -> { lock.lock(); try { - LOGGER.debug("Unsubscribing from Signal..."); + getLogger().debug("Unsubscribing from Signal..."); subscribers.remove(sink); } finally { lock.unlock(); @@ -133,10 +131,17 @@ private void notifySubscribers(ObjectNode processedEvent) { delegate.notifySubscribers(processedEvent); return; } + if (StateEvent.isRejected(processedEvent)) { + getLogger().warn( + "Operation with id '{}' is rejected with validator message: '{}'", + StateEvent.extractId(processedEvent), + StateEvent.extractValidationError(processedEvent)); + StateEvent.clearValidationError(processedEvent); + } subscribers.removeIf(sink -> { boolean failure = sink.tryEmitNext(processedEvent).isFailure(); if (failure) { - LOGGER.debug("Failed push"); + getLogger().debug("Failed push"); } return failure; }); @@ -203,4 +208,8 @@ public int hashCode() { public static void setMapper(ObjectMapper mapper) { StateEvent.setMapper(mapper); } + + private Logger getLogger() { + return LoggerFactory.getLogger(Signal.class); + } } diff --git a/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/core/event/StateEvent.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/core/event/StateEvent.java index 80c3fb3cb..b33dd3db4 100644 --- a/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/core/event/StateEvent.java +++ b/packages/java/endpoint/src/main/java/com/vaadin/hilla/signals/core/event/StateEvent.java @@ -7,8 +7,6 @@ import java.util.Arrays; import java.util.Objects; import java.util.Optional; -import java.util.function.Function; -import java.util.function.Supplier; /** * A utility class for representing state events out of an ObjectNode. This @@ -253,7 +251,23 @@ public static boolean isRejected(ObjectNode event) { return event.has(Field.ACCEPTED) && !event.get(Field.ACCEPTED).asBoolean() && event.has(Field.VALIDATION_ERROR) - && event.get(Field.VALIDATION_ERROR).asText() != null; + && !event.get(Field.VALIDATION_ERROR).asText().isBlank(); + } + + public static String extractValidationError(ObjectNode event) { + if (!isRejected(event)) { + throw new IllegalStateException( + "The event is not rejected, so it does not have a validation error"); + } + return event.get(Field.VALIDATION_ERROR).asText(); + } + + public static void clearValidationError(ObjectNode event) { + if (!isRejected(event)) { + throw new IllegalStateException( + "The event is not rejected, so it does not have a validation error"); + } + event.remove(Field.VALIDATION_ERROR); } private static JsonNode valueAsJsonNode(Object value) { @@ -338,4 +352,8 @@ public String getValidationError() { public void setValidationError(String validationError) { this.validationError = validationError; } + + public void clearValidationError() { + this.validationError = null; + } } diff --git a/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/ValueSignalTest.java b/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/ValueSignalTest.java index c27768650..6de797dee 100644 --- a/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/ValueSignalTest.java +++ b/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/ValueSignalTest.java @@ -15,7 +15,10 @@ import org.junit.AfterClass; import org.junit.BeforeClass; +import org.mockito.MockedStatic; import org.mockito.Mockito; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.context.ApplicationContext; import reactor.core.publisher.Flux; @@ -28,6 +31,10 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class ValueSignalTest { @@ -480,6 +487,28 @@ public void readonlyInstance_doesNotAllowAnyModifications() { assertEquals("Foo", readonlySignal.getValue()); } + @Test + public void submit_eventThatIsRejected_logsTheValidationError() { + Logger logger = Mockito.mock(Logger.class); + try (MockedStatic mockedLoggerFactory = mockStatic( + LoggerFactory.class)) { + + mockedLoggerFactory + .when(() -> LoggerFactory.getLogger(Signal.class)) + .thenReturn(logger); + + var signal = new ValueSignal<>("Foo", String.class) + .withOperationValidator(op -> ValidationResult + .reject("No changes allowed")); + var operation = createSetEvent("Bar"); + signal.submit(operation); + + verify(logger, times(1)).warn( + "Operation with id '{}' is rejected with validator message: '{}'", + StateEvent.extractId(operation), "No changes allowed"); + } + } + private ObjectNode createSetEvent(T value) { var setEvent = new StateEvent<>(UUID.randomUUID().toString(), StateEvent.EventType.SET, value); diff --git a/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/core/event/StateEventTest.java b/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/core/event/StateEventTest.java index f43b37e01..4aae30fcf 100644 --- a/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/core/event/StateEventTest.java +++ b/packages/java/endpoint/src/test/java/com/vaadin/hilla/signals/core/event/StateEventTest.java @@ -376,4 +376,55 @@ public void convertValue_jsonWithValidValue_returnsConvertedValue() { eventJson.get(StateEvent.Field.VALUE), Person.class); assertEquals(new Person("John Doe", 42, true), person); } + + @Test + public void extractValidationError_throws_whenEventIsNotRejected() { + var eventJson = MAPPER.createObjectNode(); + assertThrows(IllegalStateException.class, + () -> StateEvent.extractValidationError(eventJson)); + } + + @Test + public void extractValidationError_throws_whenEventIsRejectedButDoesNotHaveValidationError() { + var event = new StateEvent<>("id", StateEvent.EventType.SET, "value"); + event.setAccepted(false); + event.setValidationError(""); + assertThrows(IllegalStateException.class, + () -> StateEvent.extractValidationError(event.toJson())); + } + + @Test + public void extractValidationError_returnsValidationError() { + var eventJson = MAPPER.createObjectNode(); + eventJson.put(StateEvent.Field.VALIDATION_ERROR, "error"); + eventJson.put(StateEvent.Field.ACCEPTED, false); + assertEquals("error", StateEvent.extractValidationError(eventJson)); + + var event = new StateEvent<>("id", StateEvent.EventType.SET, "value"); + event.setAccepted(false); + event.setValidationError("error"); + assertEquals("error", + StateEvent.extractValidationError(event.toJson())); + } + + @Test + public void clearValidationError_removesValidationError() { + var eventJson = MAPPER.createObjectNode(); + eventJson.put(StateEvent.Field.VALIDATION_ERROR, "error"); + eventJson.put(StateEvent.Field.ACCEPTED, false); + assertEquals("error", StateEvent.extractValidationError(eventJson)); + StateEvent.clearValidationError(eventJson); + assertFalse(eventJson.has(StateEvent.Field.VALIDATION_ERROR)); + + var event = new StateEvent<>("id", StateEvent.EventType.SET, "value"); + event.setAccepted(false); + event.setValidationError("error"); + assertEquals("error", + StateEvent.extractValidationError(event.toJson())); + assertEquals("error", event.getValidationError()); + event.clearValidationError(); + var eventAsJson = event.toJson(); + assertNull(event.getValidationError()); + assertFalse(eventAsJson.has(StateEvent.Field.VALIDATION_ERROR)); + } } diff --git a/packages/ts/react-signals/src/ListSignal.ts b/packages/ts/react-signals/src/ListSignal.ts index 5c26d4b27..95c8c8692 100644 --- a/packages/ts/react-signals/src/ListSignal.ts +++ b/packages/ts/react-signals/src/ListSignal.ts @@ -64,7 +64,10 @@ export class ListSignal extends CollectionSignal protected override [$processServerResponse](event: StateEvent): void { if (!event.accepted) { - this[$resolveOperation](event.id, 'server rejected the operation'); + this[$resolveOperation]( + event.id, + `Server rejected the operation with id '${event.id}'. See the server log for more details.`, + ); return; } if (isListSnapshotStateEvent(event)) { diff --git a/packages/ts/react-signals/src/ValueSignal.ts b/packages/ts/react-signals/src/ValueSignal.ts index 83114ab11..e2dce28d3 100644 --- a/packages/ts/react-signals/src/ValueSignal.ts +++ b/packages/ts/react-signals/src/ValueSignal.ts @@ -111,7 +111,7 @@ export class ValueSignal extends FullStackSignal { if (event.accepted || isSnapshotStateEvent(event)) { this.#applyAcceptedEvent(event); } else { - reason = 'server rejected the operation'; + reason = `Server rejected the operation with id '${event.id}'. See the server log for more details.`; } // `then` callbacks can be associated to the record or the event // it depends on the operation that was performed