From b4f9797e92457b21889678161820a05852916774 Mon Sep 17 00:00:00 2001 From: Toon Geens Date: Tue, 19 Dec 2023 11:46:02 +0100 Subject: [PATCH 1/3] Cleanup raw use of parameterized class --- ...eactivePolicyAuthorizationManagerTest.java | 38 +++++-------------- 1 file changed, 10 insertions(+), 28 deletions(-) diff --git a/thunx-spring-gateway/src/test/java/com/contentgrid/thunx/spring/security/ReactivePolicyAuthorizationManagerTest.java b/thunx-spring-gateway/src/test/java/com/contentgrid/thunx/spring/security/ReactivePolicyAuthorizationManagerTest.java index b1045d36..e94d8241 100644 --- a/thunx-spring-gateway/src/test/java/com/contentgrid/thunx/spring/security/ReactivePolicyAuthorizationManagerTest.java +++ b/thunx-spring-gateway/src/test/java/com/contentgrid/thunx/spring/security/ReactivePolicyAuthorizationManagerTest.java @@ -2,7 +2,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import com.contentgrid.thunx.pdp.PolicyDecision; import com.contentgrid.thunx.pdp.PolicyDecisionComponentImpl; import com.contentgrid.thunx.pdp.PolicyDecisionPointClient; import com.contentgrid.thunx.pdp.PolicyDecisions; @@ -18,6 +17,7 @@ import org.springframework.security.authentication.TestingAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.web.server.authorization.AuthorizationContext; +import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -26,7 +26,7 @@ class ReactivePolicyAuthorizationManagerTest { @Test void accessGranted() { var authorizationManager = new ReactivePolicyAuthorizationManager( - new PolicyDecisionComponentImpl(policySaysYes())); + new PolicyDecisionComponentImpl<>(policySaysYes())); var exchange = MockServerWebExchange.from(MockServerHttpRequest.get("http://localhost")); var context = new AuthorizationContext(exchange); @@ -39,7 +39,7 @@ void accessGranted() { @Test void accessDenied() { var authorizationManager = new ReactivePolicyAuthorizationManager( - new PolicyDecisionComponentImpl(policySaysNo())); + new PolicyDecisionComponentImpl<>(policySaysNo())); var exchange = MockServerWebExchange.from(MockServerHttpRequest.get("http://localhost")); var context = new AuthorizationContext(exchange); @@ -54,7 +54,7 @@ void conditionalAccess() { var thunk = Comparison.areEqual(Scalar.of(42), Variable.named("foo")); var pdpClient = policySaysMaybe(thunk); - var authorizationManager = new ReactivePolicyAuthorizationManager(new PolicyDecisionComponentImpl(pdpClient)); + var authorizationManager = new ReactivePolicyAuthorizationManager(new PolicyDecisionComponentImpl<>(pdpClient)); var exchange = MockServerWebExchange.from(MockServerHttpRequest.get("http://localhost")); var context = new AuthorizationContext(exchange); @@ -78,33 +78,15 @@ private static Mono authentication() { return Mono.just(new TestingAuthenticationToken("mario", null)); } - private static PolicyDecisionPointClient policySaysYes() { - return new PolicyDecisionPointClient<>() { - @Override - public CompletableFuture conditional(Object authContext, - Object requestContext) { - return CompletableFuture.completedFuture(PolicyDecisions.allowed()); - } - }; + private static PolicyDecisionPointClient policySaysYes() { + return (authentication, exchange) -> CompletableFuture.completedFuture(PolicyDecisions.allowed()); } - private static PolicyDecisionPointClient policySaysNo() { - return new PolicyDecisionPointClient<>() { - @Override - public CompletableFuture conditional(Object authContext, - Object requestContext) { - return CompletableFuture.completedFuture(PolicyDecisions.denied()); - } - }; + private static PolicyDecisionPointClient policySaysNo() { + return (authentication, exchange) -> CompletableFuture.completedFuture(PolicyDecisions.denied()); } - private static PolicyDecisionPointClient policySaysMaybe(ThunkExpression expression) { - return new PolicyDecisionPointClient<>() { - @Override - public CompletableFuture conditional(Object authContext, - Object requestContext) { - return CompletableFuture.completedFuture(PolicyDecisions.conditional(expression)); - } - }; + private static PolicyDecisionPointClient policySaysMaybe(ThunkExpression expression) { + return (authentication, exchange) -> CompletableFuture.completedFuture(PolicyDecisions.conditional(expression)); } } \ No newline at end of file From 9bda6abc69a93c4ba94b53e48372fafa9177c82c Mon Sep 17 00:00:00 2001 From: Toon Geens Date: Tue, 19 Dec 2023 11:57:25 +0100 Subject: [PATCH 2/3] Sonar: cleanup repeated literal 'variable cannot be null' --- .../predicates/model/SymbolicReference.java | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/SymbolicReference.java b/thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/SymbolicReference.java index 28d1b8f2..dfe8666b 100644 --- a/thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/SymbolicReference.java +++ b/thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/SymbolicReference.java @@ -3,13 +3,13 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Objects; import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; import lombok.Data; import lombok.EqualsAndHashCode; import lombok.Getter; +import lombok.NonNull; import lombok.RequiredArgsConstructor; @EqualsAndHashCode @@ -52,15 +52,11 @@ public static SymbolicReference of(String variable, PathElement... path) { return of(Variable.named(variable), path); } - public static SymbolicReference of(Variable variable, PathElement... path) { - Objects.requireNonNull(variable, "variable cannot be null"); - + public static SymbolicReference of(@NonNull Variable variable, PathElement... path) { return new SymbolicReference(new SymbolicRefSubject(variable), Arrays.asList(path)); } - public static SymbolicReference of(String varName, Consumer pathCallback) { - Objects.requireNonNull(varName, "variable cannot be null"); - + public static SymbolicReference of(@NonNull String varName, Consumer pathCallback) { var pathBuilder = new PathBuilder(); pathCallback.accept(pathBuilder); @@ -70,23 +66,18 @@ public static SymbolicReference of(String varName, Consumer pathCal /** * Parse the reference and split by '.' while assuming all path-elements are string-types */ - public static SymbolicReference parse(String reference) { - Objects.requireNonNull(reference, "variable cannot be null"); + public static SymbolicReference parse(@NonNull String reference) { var parts = reference.split("\\."); var subject = parts[0]; return of(Variable.named(subject), Arrays.stream(parts).skip(1).map(part -> path(part))); } - public static SymbolicReference of(Variable variable, Stream path) { - Objects.requireNonNull(variable, "variable cannot be null"); - + public static SymbolicReference of(@NonNull Variable variable, @NonNull Stream path) { return SymbolicReference.of(variable, path.collect(Collectors.toList())); } - public static SymbolicReference of(Variable variable, List path) { - Objects.requireNonNull(variable, "variable cannot be null"); - + public static SymbolicReference of(@NonNull Variable variable, @NonNull List path) { return new SymbolicReference(new SymbolicRefSubject(variable), path); } From f4a81cd2869cb5f655ead1dc0badaa70ce9151f7 Mon Sep 17 00:00:00 2001 From: Toon Geens Date: Tue, 19 Dec 2023 12:04:40 +0100 Subject: [PATCH 3/3] Sonar: cleanup duplicated literal --- .../predicates/model/NumericFunction.java | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/NumericFunction.java b/thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/NumericFunction.java index b7d1c8d8..8c4e7961 100644 --- a/thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/NumericFunction.java +++ b/thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/NumericFunction.java @@ -35,15 +35,13 @@ public static NumericFunction multiply(ThunkExpression left, ThunkExpression< } public static NumericFunction multiply(List> terms) { - if (terms.size() < 2) { - throw new IllegalArgumentException("Expected 2 or more terms"); - } + assertHasMinimumSize(terms, 2); return terms.stream() // multiplication is an associative operation .reduce(NumericFunction::multiply) .map(NumericFunction.class::cast) - .orElseThrow(() -> new IllegalArgumentException("Expected 1 or more terms")); + .orElseThrow(); } public static NumericFunction plus(ThunkExpression left, ThunkExpression right) { @@ -52,9 +50,8 @@ public static NumericFunction plus(ThunkExpression left, ThunkExpression r public static NumericFunction plus(@NonNull List> terms) { - if (terms.size() < 2) { - throw new IllegalArgumentException("Expected 2 or more terms"); - } + assertHasMinimumSize(terms, 2); + return terms.stream() // addition is an associative operation .reduce(NumericFunction::plus) @@ -63,9 +60,7 @@ public static NumericFunction plus(@NonNull List> terms) { } public static NumericFunction divide(@NonNull List> terms) { - if (terms.size() != 2) { - throw new IllegalArgumentException("Expected exactly 2 terms"); - } + assertHasSize(terms, 2); return divide(terms.get(0), terms.get(1)); } @@ -75,9 +70,7 @@ public static NumericFunction divide(ThunkExpression left, ThunkExpression } public static NumericFunction minus(@NonNull List> terms) { - if (terms.size() != 2) { - throw new IllegalArgumentException("Expected exactly 2 terms"); - } + assertHasSize(terms, 2); return minus(terms.get(0), terms.get(1)); } @@ -87,9 +80,7 @@ public static NumericFunction minus(ThunkExpression left, ThunkExpression } public static NumericFunction modulus(@NonNull List> terms) { - if (terms.size() != 2) { - throw new IllegalArgumentException("Expected exactly 2 terms"); - } + assertHasSize(terms, 2); return modulus(terms.get(0), terms.get(1)); } @@ -98,4 +89,16 @@ public static NumericFunction modulus(ThunkExpression left, ThunkExpression terms, int expectedSize) { + if (terms.size() != expectedSize) { + throw new IllegalArgumentException("Expected exactly %s terms".formatted(expectedSize)); + } + } + + private static void assertHasMinimumSize(List terms, int minimumSize) { + if (terms.size() < minimumSize) { + throw new IllegalArgumentException("Expected %s or more terms".formatted(minimumSize)); + } + } + }