Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Sonar code smells cleanup #151

Merged
merged 3 commits into from
Dec 19, 2023
Merged

Sonar code smells cleanup #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4f9797
Cleanup raw use of parameterized class
tgeens Dec 19, 2023
9bda6ab
Sonar: cleanup repeated literal 'variable cannot be null'
tgeens Dec 19, 2023
f4a81cd
Sonar: cleanup duplicated literal
tgeens Dec 19, 2023
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
35 changes: 19 additions & 16 deletions thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/NumericFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,13 @@ public static NumericFunction multiply(ThunkExpression<?> left, ThunkExpression<
}

public static NumericFunction multiply(List<ThunkExpression<?>> 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) {
Expand All @@ -52,9 +50,8 @@ public static NumericFunction plus(ThunkExpression<?> left, ThunkExpression<?> r


public static NumericFunction plus(@NonNull List<ThunkExpression<?>> 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)
Expand All @@ -63,9 +60,7 @@ public static NumericFunction plus(@NonNull List<ThunkExpression<?>> terms) {
}

public static NumericFunction divide(@NonNull List<ThunkExpression<?>> terms) {
if (terms.size() != 2) {
throw new IllegalArgumentException("Expected exactly 2 terms");
}
assertHasSize(terms, 2);

return divide(terms.get(0), terms.get(1));
}
Expand All @@ -75,9 +70,7 @@ public static NumericFunction divide(ThunkExpression<?> left, ThunkExpression<?>
}

public static NumericFunction minus(@NonNull List<ThunkExpression<?>> terms) {
if (terms.size() != 2) {
throw new IllegalArgumentException("Expected exactly 2 terms");
}
assertHasSize(terms, 2);

return minus(terms.get(0), terms.get(1));
}
Expand All @@ -87,9 +80,7 @@ public static NumericFunction minus(ThunkExpression<?> left, ThunkExpression<?>
}

public static NumericFunction modulus(@NonNull List<ThunkExpression<?>> terms) {
if (terms.size() != 2) {
throw new IllegalArgumentException("Expected exactly 2 terms");
}
assertHasSize(terms, 2);

return modulus(terms.get(0), terms.get(1));
}
Expand All @@ -98,4 +89,16 @@ public static NumericFunction modulus(ThunkExpression<?> left, ThunkExpression<?
return new NumericFunction(Operator.MODULUS, left, right);
}

private static void assertHasSize(List<?> 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));
}
}

}
21 changes: 6 additions & 15 deletions thunx-model/src/main/java/com/contentgrid/thunx/predicates/model/SymbolicReference.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<PathBuilder> pathCallback) {
Objects.requireNonNull(varName, "variable cannot be null");

public static SymbolicReference of(@NonNull String varName, Consumer<PathBuilder> pathCallback) {
var pathBuilder = new PathBuilder();
pathCallback.accept(pathBuilder);

Expand All @@ -70,23 +66,18 @@ public static SymbolicReference of(String varName, Consumer<PathBuilder> 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<PathElement> path) {
Objects.requireNonNull(variable, "variable cannot be null");

public static SymbolicReference of(@NonNull Variable variable, @NonNull Stream<PathElement> path) {
return SymbolicReference.of(variable, path.collect(Collectors.toList()));
}

public static SymbolicReference of(Variable variable, List<PathElement> path) {
Objects.requireNonNull(variable, "variable cannot be null");

public static SymbolicReference of(@NonNull Variable variable, @NonNull List<PathElement> path) {
return new SymbolicReference(new SymbolicRefSubject(variable), path);
}

Expand Down
38 changes: 10 additions & 28 deletions ...st/java/com/contentgrid/thunx/spring/security/ReactivePolicyAuthorizationManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -78,33 +78,15 @@ private static Mono<Authentication> authentication() {
return Mono.just(new TestingAuthenticationToken("mario", null));
}

private static PolicyDecisionPointClient<?, ?> policySaysYes() {
return new PolicyDecisionPointClient<>() {
@Override
public CompletableFuture<PolicyDecision> conditional(Object authContext,
Object requestContext) {
return CompletableFuture.completedFuture(PolicyDecisions.allowed());
}
};
private static PolicyDecisionPointClient<Authentication, ServerWebExchange> policySaysYes() {
return (authentication, exchange) -> CompletableFuture.completedFuture(PolicyDecisions.allowed());
}

private static PolicyDecisionPointClient<?, ?> policySaysNo() {
return new PolicyDecisionPointClient<>() {
@Override
public CompletableFuture<PolicyDecision> conditional(Object authContext,
Object requestContext) {
return CompletableFuture.completedFuture(PolicyDecisions.denied());
}
};
private static PolicyDecisionPointClient<Authentication, ServerWebExchange> policySaysNo() {
return (authentication, exchange) -> CompletableFuture.completedFuture(PolicyDecisions.denied());
}

private static PolicyDecisionPointClient<?, ?> policySaysMaybe(ThunkExpression<Boolean> expression) {
return new PolicyDecisionPointClient<>() {
@Override
public CompletableFuture<PolicyDecision> conditional(Object authContext,
Object requestContext) {
return CompletableFuture.completedFuture(PolicyDecisions.conditional(expression));
}
};
private static PolicyDecisionPointClient<Authentication, ServerWebExchange> policySaysMaybe(ThunkExpression<Boolean> expression) {
return (authentication, exchange) -> CompletableFuture.completedFuture(PolicyDecisions.conditional(expression));
}
}