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.

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

Added code tidy comments #1311

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
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
Expand Up @@ -23,6 +23,7 @@ public class ComparisonExpression {

@Valid @NotEmpty private List<FilterOperation<?>> filterOperations;

// TIDY Why are there DB filters in the API layer, these should only be used by operations.
private List<DBFilterBase> dbFilters;

public List<DBFilterBase> getDbFilters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
* you can do this, compare equals to a literal {"username" : "aaron"} This is a shortcut for
* {"username" : {"$eq" : "aaron"}} In here we expand the shortcut into a canonical long form, so it
* is all the same.
*
* <p>TIDY: this class is part of the public API, and yet it is also getting handed into the
* operations and has some serious business logic in it. The is breaking the rules of the
* architecture, the public API classes should be basic POJO with some validation. We need another
* class to represent the logic in here
*/
public class LogicalExpression {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import java.util.List;
import java.util.stream.Collectors;

// TIDY: this class needs a large refactor and tidy, it is breaking the archiecture by having
// classes from the API package use DBFilters from the Operations, and vice-verse
public class ExpressionBuilder {

public static List<Expression<BuiltCondition>> buildExpressions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.FilterOperation;
import java.util.List;

// TIDY What does this do ?
public record CaptureExpression(
Object marker, List<FilterOperation<?>> filterOperations, String path) {}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public Optional<LogicalExpression> apply(T command) {
return matchStrategyCounter.applyStrategy(strategy, filter);
}

// TIDY: Why is this public ?
public void captureRecursive(
LogicalExpression expression,
List<Capture> unmatchedCaptures,
Expand All @@ -74,6 +75,7 @@ public void captureRecursive(
Capture capture = captureIter.next();
List<FilterOperation<?>> matched = capture.match(comparisonExpression);
if (!matched.isEmpty()) {
// TIDY - this must change, no DB Filters that come from the Operator level into the API
comparisonExpression.setDBFilters(
resolveFunction.apply(
new CaptureExpression(capture.marker, matched, comparisonExpression.getPath())));
Expand Down Expand Up @@ -147,6 +149,26 @@ public FilterMatcher<T> compareValues(
}
}

// TIDY - refactor this class, unclear why the original needed to change
// it is based on some original code, but is duplicating counting
// that was done by looking at lists of captures and expressions
//
// Or if it should stay, implement as a strategy that has sublcasses the implement the bahviour
// original below
// switch (strategy) {
// case STRICT:
// if (unmatchedCaptures.isEmpty() && unmatchedExpressions.isEmpty()) {
// // everything group and expression matched
// return Optional.of(captures);
// }
// break;
// case GREEDY:
// if (unmatchedExpressions.isEmpty()) {
// // everything expression matched, some captures may not match
// return Optional.of(captures);
// }
// break;
// }
public static final class MatchStrategyCounter {

private int unmatchedCaptureCount;
Expand Down