From d85847a88ac3b5551cda110b276570ca6a8241ce Mon Sep 17 00:00:00 2001 From: JoseLion Date: Mon, 4 Dec 2023 00:32:57 -0500 Subject: [PATCH] fix(core): Handle possible nulls in factory methods --- .../joselion/maybe/CloseableHandler.java | 34 +++++---- .../github/joselion/maybe/EffectHandler.java | 11 ++- .../java/io/github/joselion/maybe/Maybe.java | 10 +-- .../github/joselion/maybe/SolveHandler.java | 38 ++++++---- .../io/github/joselion/maybe/Maybe.java | 10 +-- .../joselion/maybe/CloseableHandlerTests.java | 47 ++++++++++++ .../joselion/maybe/EffectHandlerTests.java | 30 ++++++++ .../joselion/maybe/SolveHandlerTests.java | 72 +++++++++++++++---- 8 files changed, 201 insertions(+), 51 deletions(-) diff --git a/src/main/java/io/github/joselion/maybe/CloseableHandler.java b/src/main/java/io/github/joselion/maybe/CloseableHandler.java index ef9d195..1fc94f2 100644 --- a/src/main/java/io/github/joselion/maybe/CloseableHandler.java +++ b/src/main/java/io/github/joselion/maybe/CloseableHandler.java @@ -1,5 +1,7 @@ package io.github.joselion.maybe; +import static java.util.Objects.isNull; + import java.util.Optional; import io.github.joselion.maybe.helpers.Commons; @@ -22,12 +24,8 @@ public class CloseableHandler { private final Either value; - private CloseableHandler(final T resource) { - this.value = Either.ofRight(resource); - } - - private CloseableHandler(final E error) { - this.value = Either.ofLeft(error); + private CloseableHandler(final Either value) { + this.value = value; } /** @@ -39,7 +37,12 @@ private CloseableHandler(final E error) { * @return a new instance of CloseableHandler with the given resource */ static CloseableHandler from(final T resource) { - return new CloseableHandler<>(resource); + final var nullException = new NullPointerException("The \"Maybe\" resource solved to null"); + final var either = isNull(resource) // NOSONAR + ? Either.ofLeft(Commons.cast(nullException)) + : Either.ofRight(resource); + + return new CloseableHandler<>(either); } /** @@ -51,7 +54,12 @@ static CloseableHandler fro * @return a new instance of the failed CloseableHandler with the error */ static CloseableHandler failure(final E error) { - return new CloseableHandler<>(error); + final var nullException = new NullPointerException("The \"Maybe\" error was null"); + final var either = isNull(error) // NOSONAR + ? Either.ofLeft(Commons.cast(nullException)) + : Either.ofLeft(error); + + return new CloseableHandler<>(either); } /** @@ -95,13 +103,13 @@ public SolveHandler solve( return this.value .mapLeft(Commons::cast) .unwrap( - SolveHandler::ofError, + SolveHandler::failure, resource -> { try (var res = resource) { - return SolveHandler.ofSuccess(solver.apply(res)); + return SolveHandler.from(solver.apply(res)); } catch (final Throwable e) { //NOSONAR final var error = Commons.cast(e); - return SolveHandler.ofError(error); + return SolveHandler.failure(error); } } ); @@ -154,14 +162,14 @@ public EffectHandler effect( return this.value .mapLeft(Commons::cast) .unwrap( - EffectHandler::ofError, + EffectHandler::failure, resource -> { try (var res = resource) { effect.accept(res); return EffectHandler.empty(); } catch (final Throwable e) { // NOSONAR final var error = Commons.cast(e); - return EffectHandler.ofError(error); + return EffectHandler.failure(error); } } ); diff --git a/src/main/java/io/github/joselion/maybe/EffectHandler.java b/src/main/java/io/github/joselion/maybe/EffectHandler.java index d745cc8..3dd2a27 100644 --- a/src/main/java/io/github/joselion/maybe/EffectHandler.java +++ b/src/main/java/io/github/joselion/maybe/EffectHandler.java @@ -1,11 +1,14 @@ package io.github.joselion.maybe; +import static java.util.Objects.isNull; + import java.util.Optional; import java.util.function.Consumer; import java.util.function.Function; import org.eclipse.jdt.annotation.Nullable; +import io.github.joselion.maybe.helpers.Commons; import io.github.joselion.maybe.util.function.ThrowingConsumer; import io.github.joselion.maybe.util.function.ThrowingRunnable; @@ -45,8 +48,12 @@ static EffectHandler empty() { * @param error the error to instanciate the EffectHandler * @return a EffectHandler instance with an error value */ - static EffectHandler ofError(final E error) { - return new EffectHandler<>(error); + static EffectHandler failure(final E error) { + return new EffectHandler<>( + isNull(error) // NOSONAR + ? Commons.cast(new NullPointerException("The \"Maybe\" error was null")) + : error + ); } /** diff --git a/src/main/java/io/github/joselion/maybe/Maybe.java b/src/main/java/io/github/joselion/maybe/Maybe.java index d0e6d2e..1676e57 100644 --- a/src/main/java/io/github/joselion/maybe/Maybe.java +++ b/src/main/java/io/github/joselion/maybe/Maybe.java @@ -157,10 +157,10 @@ public static SolveHandler from( final ThrowingSupplier solver ) { try { - return SolveHandler.ofSuccess(solver.get()); + return SolveHandler.from(solver.get()); } catch (Throwable e) { // NOSONAR final var error = Commons.cast(e); - return SolveHandler.ofError(error); + return SolveHandler.failure(error); } } @@ -199,7 +199,7 @@ public static EffectHandler from(final ThrowingRunnable return EffectHandler.empty(); } catch (Throwable e) { // NOSONAR final var error = Commons.cast(e); - return EffectHandler.ofError(error); + return EffectHandler.failure(error); } } @@ -444,7 +444,7 @@ public SolveHandler solve( .orElseThrow(); } catch (final NoSuchElementException e) { final var error = Commons.cast(e); - return SolveHandler.ofError(error); + return SolveHandler.failure(error); } } @@ -484,7 +484,7 @@ public EffectHandler effect(final ThrowingConsumercast(e); - return EffectHandler.ofError(error); + return EffectHandler.failure(error); } } diff --git a/src/main/java/io/github/joselion/maybe/SolveHandler.java b/src/main/java/io/github/joselion/maybe/SolveHandler.java index dcf50a3..d4d8768 100644 --- a/src/main/java/io/github/joselion/maybe/SolveHandler.java +++ b/src/main/java/io/github/joselion/maybe/SolveHandler.java @@ -1,5 +1,7 @@ package io.github.joselion.maybe; +import static java.util.Objects.isNull; + import java.util.Optional; import java.util.function.Consumer; import java.util.function.Function; @@ -39,8 +41,13 @@ private SolveHandler(final Either value) { * @param success the success value to instantiate the SolveHandler * @return a SolveHandler instance with a success value */ - static SolveHandler ofSuccess(final T success) { - return new SolveHandler<>(Either.ofRight(success)); + static SolveHandler from(final T success) { + final var nullException = new NullPointerException("The \"Maybe\" value solved to null"); + final var either = isNull(success) // NOSONAR + ? Either.ofLeft(Commons.cast(nullException)) + : Either.ofRight(success); + + return new SolveHandler<>(either); } /** @@ -51,8 +58,13 @@ static SolveHandler ofSuccess(final T success) { * @param error the error to instantiate the SolveHandler * @return a SolveHandler instance with an error value */ - static SolveHandler ofError(final E error) { - return new SolveHandler<>(Either.ofLeft(error)); + static SolveHandler failure(final E error) { + final var nullException = new NullPointerException("The \"Maybe\" error was null"); + final var either = isNull(error) // NOSONAR + ? Either.ofLeft(Commons.cast(nullException)) + : Either.ofLeft(error); + + return new SolveHandler<>(either); } /** @@ -140,7 +152,7 @@ public SolveHandler catchError( .filter(ofType::isInstance) .map(ofType::cast) .map(handler) - .map(SolveHandler::ofSuccess) + .map(SolveHandler::from) .orElse(this); } @@ -156,7 +168,7 @@ public SolveHandler catchError( public SolveHandler catchError(final Function handler) { return this.value .mapLeft(handler) - .mapLeft(SolveHandler::ofSuccess) + .mapLeft(SolveHandler::from) .leftOrElse(this); } @@ -229,7 +241,7 @@ public SolveHandler solve( return this.value .mapLeft(Commons::cast) .unwrap( - SolveHandler::ofError, + SolveHandler::failure, Maybe.partial(solver) ); } @@ -305,7 +317,7 @@ public EffectHandler effect(final ThrowingConsumercast) .unwrap( - EffectHandler::ofError, + EffectHandler::failure, Maybe.partial(effect) ); } @@ -341,8 +353,8 @@ public SolveHandler map(final Function mapper) return this.value .mapRight(mapper) .unwrap( - SolveHandler::ofError, - SolveHandler::ofSuccess + SolveHandler::failure, + SolveHandler::from ); } @@ -358,12 +370,12 @@ public SolveHandler map(final Function mapper) */ public SolveHandler cast(final Class type) { return this.value.unwrap( - error -> ofError(new ClassCastException(error.getMessage())), + error -> failure(new ClassCastException(error.getMessage())), success -> { try { - return ofSuccess(type.cast(success)); + return from(type.cast(success)); } catch (ClassCastException error) { - return ofError(error); + return failure(error); } } ); diff --git a/src/main/java17/io/github/joselion/maybe/Maybe.java b/src/main/java17/io/github/joselion/maybe/Maybe.java index b3a340d..fdb0b10 100644 --- a/src/main/java17/io/github/joselion/maybe/Maybe.java +++ b/src/main/java17/io/github/joselion/maybe/Maybe.java @@ -157,10 +157,10 @@ public static SolveHandler from( final ThrowingSupplier solver ) { try { - return SolveHandler.ofSuccess(solver.get()); + return SolveHandler.from(solver.get()); } catch (Throwable e) { // NOSONAR final var error = Commons.cast(e); - return SolveHandler.ofError(error); + return SolveHandler.failure(error); } } @@ -199,7 +199,7 @@ public static EffectHandler from(final ThrowingRunnable return EffectHandler.empty(); } catch (Throwable e) { // NOSONAR final var error = Commons.cast(e); - return EffectHandler.ofError(error); + return EffectHandler.failure(error); } } @@ -444,7 +444,7 @@ public SolveHandler solve( .orElseThrow(); } catch (final NoSuchElementException e) { final var error = Commons.cast(e); - return SolveHandler.ofError(error); + return SolveHandler.failure(error); } } @@ -484,7 +484,7 @@ public EffectHandler effect(final ThrowingConsumercast(e); - return EffectHandler.ofError(error); + return EffectHandler.failure(error); } } diff --git a/src/test/java/io/github/joselion/maybe/CloseableHandlerTests.java b/src/test/java/io/github/joselion/maybe/CloseableHandlerTests.java index 02136bd..21edec2 100644 --- a/src/test/java/io/github/joselion/maybe/CloseableHandlerTests.java +++ b/src/test/java/io/github/joselion/maybe/CloseableHandlerTests.java @@ -37,6 +37,53 @@ throw FAIL_EXCEPTION; }; + @Nested class from { + @Nested class when_the_resource_is_not_null { + @Test void returns_a_handler_with_the_value() { + final var fis = getFIS(); + final var handler = CloseableHandler.from(fis); + + assertThat(handler.resource()).containsSame(fis); + assertThat(handler.error()).isEmpty(); + } + } + + @Nested class when_the_resource_is_null { + @Test void returns_a_handler_with_a_NullPointerException_error() { + final var handler = CloseableHandler.from(null); + + assertThat(handler.resource()).isEmpty(); + assertThat(handler.error()) + .get(THROWABLE) + .isExactlyInstanceOf(NullPointerException.class) + .hasMessage("The \"Maybe\" resource solved to null"); + } + } + } + + @Nested class failure { + @Nested class when_the_error_is_not_null { + @Test void returns_a_handler_with_the_error() { + final var handler = CloseableHandler.failure(FAIL_EXCEPTION); + + assertThat(handler.resource()).isEmpty(); + assertThat(handler.error()).containsSame(FAIL_EXCEPTION); + } + } + + @Nested class when_the_error_is_null { + @Test void returns_a_handler_with_a_NullPointerException_error() { + final var handler = CloseableHandler.failure(null); + + assertThat(handler.resource()).isEmpty(); + assertThat(handler.error()) + .get(THROWABLE) + .isExactlyInstanceOf(NullPointerException.class) + .hasMessage("The \"Maybe\" error was null"); + } + } + } + @Nested class solve { @Nested class when_the_resource_is_present { @Nested class when_the_operation_succeeds { diff --git a/src/test/java/io/github/joselion/maybe/EffectHandlerTests.java b/src/test/java/io/github/joselion/maybe/EffectHandlerTests.java index ecc87c3..7b39916 100644 --- a/src/test/java/io/github/joselion/maybe/EffectHandlerTests.java +++ b/src/test/java/io/github/joselion/maybe/EffectHandlerTests.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.InstanceOfAssertFactories.THROWABLE; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -31,6 +32,35 @@ private final ThrowingRunnable noOp = () -> { }; + @Nested class empty { + @Test void returns_an_empty_handler() { + final var handler = EffectHandler.empty(); + + assertThat(handler.error()).isEmpty(); + } + } + + @Nested class failure { + @Nested class when_the_error_is_not_null { + @Test void returns_a_handler_with_the_error() { + final var handler = EffectHandler.failure(FAIL_EXCEPTION); + + assertThat(handler.error()).containsSame(FAIL_EXCEPTION); + } + } + + @Nested class when_the_error_is_null { + @Test void returns_a_handler_with_a_NullPointerException_error() { + final var handler = EffectHandler.failure(null); + + assertThat(handler.error()) + .get(THROWABLE) + .isExactlyInstanceOf(NullPointerException.class) + .hasMessage("The \"Maybe\" error was null"); + } + } + } + @Nested class doOnSuccess { @Nested class when_the_value_is_present { @Test void calls_the_effect_callback() { diff --git a/src/test/java/io/github/joselion/maybe/SolveHandlerTests.java b/src/test/java/io/github/joselion/maybe/SolveHandlerTests.java index aa57384..25e108d 100644 --- a/src/test/java/io/github/joselion/maybe/SolveHandlerTests.java +++ b/src/test/java/io/github/joselion/maybe/SolveHandlerTests.java @@ -40,6 +40,52 @@ private final ThrowingSupplier okOp = () -> OK; + @Nested class from { + @Nested class when_the_value_is_not_null { + @Test void returns_a_handler_withThe_value() { + final var handler = SolveHandler.from(OK); + + assertThat(handler.success()).containsSame(OK); + assertThat(handler.error()).isEmpty(); + } + } + + @Nested class when_the_value_is_null { + @Test void returns_a_handler_with_a_NullPointerException_error() { + final var handler = SolveHandler.from(null); + + assertThat(handler.success()).isEmpty(); + assertThat(handler.error()) + .get(THROWABLE) + .isExactlyInstanceOf(NullPointerException.class) + .hasMessage("The \"Maybe\" value solved to null"); + } + } + } + + @Nested class failure { + @Nested class when_the_error_is_not_null { + @Test void returns_a_handler_with_the_error() { + final var handler = SolveHandler.failure(FAIL_EXCEPTION); + + assertThat(handler.success()).isEmpty(); + assertThat(handler.error()).containsSame(FAIL_EXCEPTION); + } + } + + @Nested class when_the_error_is_null { + @Test void returns_a_handler_with_a_NullPointerException_error() { + final var handler = SolveHandler.failure(null); + + assertThat(handler.success()).isEmpty(); + assertThat(handler.error()) + .get(THROWABLE) + .isExactlyInstanceOf(NullPointerException.class) + .hasMessage("The \"Maybe\" error was null"); + } + } + } + @Nested class doOnSuccess { @Nested class when_the_value_is_present { @Test void calls_the_effect_callback() { @@ -179,7 +225,7 @@ @Nested class solve { @Nested class when_the_value_is_present { - @Test void calls_the_solver_callback_and_returns_a_new_handler() { + @Test void calls_the_solver_callback_and_returns_a_handler() { final var solverSpy = Spy.>lambda(String::length); final var successSpy = Spy.>lambda(String::length); final var errorSpy = Spy.>lambda(e -> -1); @@ -247,7 +293,7 @@ @Nested class effect { @Nested class when_the_value_is_present { - @Test void calls_the_solver_callback_and_returns_a_new_handler() throws FileSystemException { + @Test void calls_the_solver_callback_and_returns_a_handler() throws FileSystemException { final var effectSpy = Spy.>lambda(v -> throwingOp.get()); final var successSpy = Spy.>lambda(v -> throwingOp.get()); final var errorSpy = Spy.>lambda(err -> { }); @@ -269,7 +315,7 @@ @Nested class when_the_error_is_present { @Nested class and_the_error_callback_is_provided { - @Test void calls_only_the_error_callback_and_returns_a_new_handler() throws FileSystemException { + @Test void calls_only_the_error_callback_and_returns_a_handler() throws FileSystemException { final var successSpy = Spy.>lambda(v -> { }); final var errorSpy = Spy.>lambda( err -> throwingOp.get() @@ -316,8 +362,8 @@ @Nested class map { @Nested class when_the_value_is_present { - @Test void returns_a_new_handler_applying_the_mapper_function() { - final var handler = SolveHandler.ofSuccess("Hello world!") + @Test void returns_a_handler_applying_the_mapper_function() { + final var handler = SolveHandler.from("Hello world!") .map(String::length); assertThat(handler.success()).contains(12); @@ -327,8 +373,8 @@ } @Nested class when_the_error_is_present { - @Test void returns_a_new_handler_with_the_previous_error() { - final var handler = SolveHandler.ofError(FAIL_EXCEPTION) + @Test void returns_a_handler_with_the_previous_error() { + final var handler = SolveHandler.failure(FAIL_EXCEPTION) .map(Object::toString); assertThat(handler.success()).isEmpty(); @@ -340,9 +386,9 @@ @Nested class cast { @Nested class when_the_value_is_present { @Nested class and_the_object_can_be_cast { - @Test void returns_a_new_handler_with_the_cast_value() { + @Test void returns_a_handler_with_the_cast_value() { final var anyValue = (Object) "Hello"; - final var handler = SolveHandler.ofSuccess(anyValue) + final var handler = SolveHandler.from(anyValue) .cast(String.class); assertThat(handler.success()).contains("Hello"); @@ -351,8 +397,8 @@ } @Nested class and_the_object_can_not_be_cast { - @Test void returns_a_new_handler_with_the_cast_exception() { - final var handler = SolveHandler.ofSuccess(3) + @Test void returns_a_handler_with_the_cast_exception() { + final var handler = SolveHandler.from(3) .cast(String.class); assertThat(handler.success()).isEmpty(); @@ -365,8 +411,8 @@ } @Nested class when_the_error_is_present { - @Test void returns_a_new_handler_with_a_cast_exception() { - final var handler = SolveHandler.ofError(FAIL_EXCEPTION) + @Test void returns_a_handler_with_a_cast_exception() { + final var handler = SolveHandler.failure(FAIL_EXCEPTION) .cast(String.class); assertThat(handler.success()).isEmpty();