diff --git a/bin/test/de/ii/xtraplatform/feature/core/TestEntityRepository.class b/bin/test/de/ii/xtraplatform/feature/core/TestEntityRepository.class new file mode 100644 index 000000000..8b66e0332 Binary files /dev/null and b/bin/test/de/ii/xtraplatform/feature/core/TestEntityRepository.class differ diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/SqlPathSyntax.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/SqlPathSyntax.java index e9ee2c27b..d49a390e3 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/SqlPathSyntax.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/SqlPathSyntax.java @@ -22,8 +22,13 @@ @Value.Immutable @Value.Style(deepImmutablesDetection = true) +@SuppressWarnings("PMD.ExcessivePublicCount") public interface SqlPathSyntax { + String REGEX_GROUP_START = "(?<"; + String REGEX_GROUP_VALUE_END = ">.+?)\\}"; + String REGEX_NON_CAPTURING_GROUP_START = "(?:"; + default List asList(String path) { return getPathSplitter().splitToList(path); } @@ -57,7 +62,7 @@ default boolean getOidFlag(String path) { return matcher.find(); } - default String setOidFlag(String path) { + default String addOidFlag(String path) { return String.format("%s{oid}", path); } @@ -67,7 +72,7 @@ default boolean getSpatialFlag(String path) { return matcher.find(); } - default String setSpatialFlag(String path) { + default String addSpatialFlag(String path) { return String.format("%s{spatial}", path); } @@ -77,7 +82,7 @@ default boolean getTemporalFlag(String path) { return matcher.find(); } - default String setTemporalFlag(String path) { + default String addTemporalFlag(String path) { return String.format("%s{temporal}", path); } @@ -91,7 +96,7 @@ default OptionalInt getPriorityFlag(String path) { return OptionalInt.empty(); } - default String setPriorityFlag(String path, int priority) { + default String addPriorityFlag(String path, int priority) { return String.format("%s{priority=%d}", path, priority); } @@ -125,7 +130,7 @@ default Optional getSortKeyFlag(String flags) { return Optional.empty(); } - default String setQueryableFlag(String path, String queryable) { + default String addQueryableFlag(String path, String queryable) { return String.format("%s{queryable=%s}", path, queryable); } @@ -152,7 +157,7 @@ default Optional getFilterFlagExpression(String flags) { // TODO: start end separator for flags @Value.Derived default String getPriorityFlagPattern() { - return "\\{priority=" + "(?<" + MatcherGroups.PRIORITY + ">[0-9]+)\\}"; + return "\\{priority=" + REGEX_GROUP_START + MatcherGroups.PRIORITY + ">[0-9]+)\\}"; } @Value.Derived @@ -172,22 +177,22 @@ default String getTemporalFlagPattern() { @Value.Derived default String getQueryableFlagPattern() { - return "\\{queryable=" + "(?<" + MatcherGroups.QUERYABLE + ">.+?)\\}"; + return "\\{queryable=" + REGEX_GROUP_START + MatcherGroups.QUERYABLE + REGEX_GROUP_VALUE_END; } @Value.Derived default String getConstantFlagPattern() { - return "\\{constant=" + "(?<" + MatcherGroups.CONSTANT + ">.+?)\\}"; + return "\\{constant=" + REGEX_GROUP_START + MatcherGroups.CONSTANT + REGEX_GROUP_VALUE_END; } @Value.Derived default String getSortKeyFlagPattern() { - return "\\{sortKey=" + "(?<" + MatcherGroups.SORT_KEY + ">.+?)\\}"; + return "\\{sortKey=" + REGEX_GROUP_START + MatcherGroups.SORT_KEY + REGEX_GROUP_VALUE_END; } @Value.Derived default String getFilterFlagPattern() { - return "\\{filter=" + "(?<" + MatcherGroups.FILTER + ">.+?)\\}"; + return "\\{filter=" + REGEX_GROUP_START + MatcherGroups.FILTER + REGEX_GROUP_VALUE_END; } @Value.Derived @@ -270,7 +275,7 @@ default String getIdentifierPattern() { @Value.Derived default String getColumnPattern() { return getIdentifierPattern() - + "(?:" + + REGEX_NON_CAPTURING_GROUP_START + getMultiColumnSeparator() + getIdentifierPattern() + ")*"; @@ -294,13 +299,13 @@ default String getJoinConditionEnd() { @Value.Derived default String getJoinConditionPattern() { return Pattern.quote(getJoinConditionStart()) - + "(?<" + + REGEX_GROUP_START + MatcherGroups.SOURCE_FIELD + ">" + getIdentifierPattern() + ")" + Pattern.quote(getJoinConditionSeparator()) - + "(?<" + + REGEX_GROUP_START + MatcherGroups.TARGET_FIELD + ">" + getIdentifierPattern() @@ -311,11 +316,11 @@ default String getJoinConditionPattern() { @Value.Derived default String getJoinConditionPlainPattern() { return Pattern.quote(getJoinConditionStart()) - + "(?:" + + REGEX_NON_CAPTURING_GROUP_START + getIdentifierPattern() + ")" + Pattern.quote(getJoinConditionSeparator()) - + "(?:" + + REGEX_NON_CAPTURING_GROUP_START + getIdentifierPattern() + ")" + Pattern.quote(getJoinConditionEnd()); @@ -323,15 +328,15 @@ default String getJoinConditionPlainPattern() { @Value.Derived default String getTablePatternString() { - return "(?:" + return REGEX_NON_CAPTURING_GROUP_START + getJoinConditionPattern() + ")?" - + "(?<" + + REGEX_GROUP_START + MatcherGroups.TABLE + ">" + getIdentifierPattern() + ")" - + "(?<" + + REGEX_GROUP_START + MatcherGroups.TABLE_FLAGS + ">" + getFlagsPattern() @@ -340,13 +345,13 @@ default String getTablePatternString() { @Value.Derived default String getTablePatternPlainString() { - return "(?:" + return REGEX_NON_CAPTURING_GROUP_START + getJoinConditionPlainPattern() + ")?" - + "(?:" + + REGEX_NON_CAPTURING_GROUP_START + getIdentifierPattern() + ")" - + "(?:" + + REGEX_NON_CAPTURING_GROUP_START + getFlagsPattern() + ")?"; } @@ -360,7 +365,7 @@ default Pattern getTablePattern() { default Pattern getJoinedTablePattern() { return Pattern.compile( getJoinConditionPattern() - + "(?<" + + REGEX_GROUP_START + MatcherGroups.TABLE + ">" + getIdentifierPattern() @@ -373,17 +378,17 @@ default Pattern getColumnPathPattern() { "^(?<" + MatcherGroups.PATH + ">" - + "(?:" + + REGEX_NON_CAPTURING_GROUP_START + getPathSeparator() + getTablePatternString() + ")+)" + getPathSeparator() - + "(?<" + + REGEX_GROUP_START + MatcherGroups.COLUMNS + ">" + getColumnPattern() + ")?" - + "(?<" + + REGEX_GROUP_START + MatcherGroups.PATH_FLAGS + ">" + getFlagsPattern() @@ -394,14 +399,14 @@ default Pattern getColumnPathPattern() { default Pattern getPartialColumnPathPattern() { return Pattern.compile( getPathPatternString() - + "(?:" + + REGEX_NON_CAPTURING_GROUP_START + getPathSeparator() - + "(?<" + + REGEX_GROUP_START + MatcherGroups.COLUMNS + ">" + getColumnPattern() + "))" - + "(?<" + + REGEX_GROUP_START + MatcherGroups.PATH_FLAGS + ">" + getFlagsPattern() @@ -415,13 +420,13 @@ default Pattern getPathPattern() { @Value.Derived default String getPathPatternString() { - return "(?<" + return REGEX_GROUP_START + MatcherGroups.PATH + ">" + getPathSeparator() + "?" + getTablePatternPlainString() - + "(?:" + + REGEX_NON_CAPTURING_GROUP_START + getPathSeparator() + getTablePatternPlainString() + ")*)"; diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/AggregateStatsQueryGenerator.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/AggregateStatsQueryGenerator.java index 7ff5213a1..c510cbe4f 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/AggregateStatsQueryGenerator.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/AggregateStatsQueryGenerator.java @@ -7,7 +7,6 @@ */ package de.ii.xtraplatform.features.sql.app; -import de.ii.xtraplatform.features.sql.domain.SchemaSql; import de.ii.xtraplatform.features.sql.domain.SqlDialect; import de.ii.xtraplatform.features.sql.domain.SqlQueryColumn; import de.ii.xtraplatform.features.sql.domain.SqlQueryColumn.Operation; @@ -17,12 +16,11 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class AggregateStatsQueryGenerator { - private static final Logger LOGGER = LoggerFactory.getLogger(AggregateStatsQueryGenerator.class); + private static final String FORMAT_TABLE_ALIAS = "%s %s"; + private static final String FORMAT_WHERE = " WHERE %s"; private final SqlDialect sqlDialect; private final FilterEncoderSql filterEncoder; @@ -37,10 +35,10 @@ public String getCountQuery(SqlQueryMapping mapping) { List aliases = AliasGenerator.getAliases(sourceSchema); - String mainTable = String.format("%s %s", sourceSchema.getName(), aliases.get(0)); + String mainTable = String.format(FORMAT_TABLE_ALIAS, sourceSchema.getName(), aliases.get(0)); Optional filter = getFilter(mapping, sourceSchema); - String where = filter.isPresent() ? String.format(" WHERE %s", filter.get()) : ""; + String where = filter.isPresent() ? String.format(FORMAT_WHERE, filter.get()) : ""; return String.format("SELECT COUNT(*) FROM %s%s", mainTable, where); } @@ -52,7 +50,7 @@ public String getSpatialExtentQuery( List aliases = AliasGenerator.getAliases(spatial); String spatialAlias = aliases.get(aliases.size() - 1); - String mainTable = String.format("%s %s", mainSchema.getName(), aliases.get(0)); + String mainTable = String.format(FORMAT_TABLE_ALIAS, mainSchema.getName(), aliases.get(0)); String column = SqlQueryColumnOperations.getQualifiedColumnResolved( @@ -66,7 +64,7 @@ public String getSpatialExtentQuery( String join = JoinGenerator.getJoins(spatial, aliases, filterEncoder); Optional filter = getFilter(mapping, mainSchema); - String where = filter.isPresent() ? String.format(" WHERE %s", filter.get()) : ""; + String where = filter.isPresent() ? String.format(FORMAT_WHERE, filter.get()) : ""; return String.format( "SELECT %s FROM %s%s%s%s", columnExtent, mainTable, join.isEmpty() ? "" : " ", join, where); @@ -79,7 +77,7 @@ public String getTemporalExtentQuery( List aliases = AliasGenerator.getAliases(instant); String temporalAlias = aliases.get(aliases.size() - 1); - String mainTable = String.format("%s %s", mainSchema.getName(), aliases.get(0)); + String mainTable = String.format(FORMAT_TABLE_ALIAS, mainSchema.getName(), aliases.get(0)); SqlQueryColumn instantColumnDatetime = SqlQueryColumnOperations.dateToDatetime(instantColumn); @@ -90,7 +88,7 @@ public String getTemporalExtentQuery( String join = JoinGenerator.getJoins(instant, aliases, filterEncoder); Optional filter = getFilter(mapping, mainSchema); - String where = filter.isPresent() ? String.format(" WHERE %s", filter.get()) : ""; + String where = filter.isPresent() ? String.format(FORMAT_WHERE, filter.get()) : ""; return String.format( "SELECT MIN(%s), MAX(%s) FROM %s%s%s%s", @@ -109,7 +107,7 @@ public String getTemporalExtentQuery( List aliases = AliasGenerator.getAliases(intervalStart); String temporalAlias = aliases.get(aliases.size() - 1); - String mainTable = String.format("%s %s", mainSchema.getName(), aliases.get(0)); + String mainTable = String.format(FORMAT_TABLE_ALIAS, mainSchema.getName(), aliases.get(0)); SqlQueryColumn intervalStartColumnColumnDatetime = SqlQueryColumnOperations.dateToDatetime(intervalStartColumn); @@ -127,7 +125,7 @@ public String getTemporalExtentQuery( String join = JoinGenerator.getJoins(intervalStart, aliases, filterEncoder); Optional filter = getFilter(mapping, mainSchema); - String where = filter.isPresent() ? String.format(" WHERE %s", filter.get()) : ""; + String where = filter.isPresent() ? String.format(FORMAT_WHERE, filter.get()) : ""; return String.format( "SELECT MIN(%s), MAX(%s) FROM %s%s%s%s", @@ -139,7 +137,8 @@ public String getTemporalExtentQuery( List endAliases = AliasGenerator.getAliases(intervalEnd); String endAlias = endAliases.get(endAliases.size() - 1); - String mainTable = String.format("%s %s", mainSchema.getName(), startAliases.get(0)); + String mainTable = + String.format(FORMAT_TABLE_ALIAS, mainSchema.getName(), startAliases.get(0)); String columnStart = SqlQueryColumnOperations.getQualifiedColumnResolved( @@ -157,7 +156,7 @@ public String getTemporalExtentQuery( String.format("%s%s%s", mainTable, endJoin.isEmpty() ? "" : " ", endJoin); Optional filter = getFilter(mapping, mainSchema); - String where = filter.isPresent() ? String.format(" WHERE %s", filter.get()) : ""; + String where = filter.isPresent() ? String.format(FORMAT_WHERE, filter.get()) : ""; return String.format( "SELECT * FROM (SELECT MIN(%s) FROM %s%s) AS A, (SELECT MAX(%s) from %s%s) AS B;", @@ -165,20 +164,7 @@ public String getTemporalExtentQuery( } } - private Optional getFilter(SchemaSql schemaSql) { - return schemaSql.getFilter().map(cql -> filterEncoder.encode(cql, schemaSql)); - } - private Optional getFilter(SqlQueryMapping mapping, SqlQuerySchema schemaSql) { return schemaSql.getFilter().map(cql -> filterEncoder.encode(cql, mapping)); } - - private String getQualifiedColumn(String table, String column) { - if (column.startsWith("[EXPRESSION]{sql=")) { - return column.substring(17, column.length() - 1).replaceAll("\\$T\\$", table); - } - return column.contains("(") - ? column.replaceAll("((?:\\w+\\()+)(\\w+)((?:\\))+)", "$1" + table + ".$2$3 AS $2") - : String.format("%s.%s", table, column); - } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/AliasGenerator.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/AliasGenerator.java index 6a4ba0e8e..045d577e0 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/AliasGenerator.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/AliasGenerator.java @@ -9,7 +9,6 @@ import com.google.common.collect.ImmutableList; import de.ii.xtraplatform.features.sql.domain.SchemaSql; -import de.ii.xtraplatform.features.sql.domain.SqlQueryJoin; import de.ii.xtraplatform.features.sql.domain.SqlQuerySchema; import de.ii.xtraplatform.features.sql.domain.SqlRelation; import java.util.List; @@ -17,9 +16,11 @@ import java.util.stream.IntStream; import java.util.stream.Stream; -class AliasGenerator { +final class AliasGenerator { - public static List getAliases(List parents, SchemaSql schema) { + private AliasGenerator() {} + + static List getAliases(List parents, SchemaSql schema) { char alias = 'A'; if (parents.isEmpty() && schema.getRelation().isEmpty()) { @@ -35,18 +36,20 @@ public static List getAliases(List parents, SchemaSql schema) .collect(Collectors.toList()); for (SqlRelation relation : relations) { - aliases.add(String.valueOf(alias++)); + aliases.add(String.valueOf(alias)); + alias++; if (relation.isM2N()) { - aliases.add(String.valueOf(alias++)); + aliases.add(String.valueOf(alias)); + alias++; } } - aliases.add(String.valueOf(alias++)); + aliases.add(String.valueOf(alias)); return aliases.build(); } - public static List getAliases(SchemaSql schema) { + static List getAliases(SchemaSql schema) { char alias = 'A'; if (schema.getParentPath().isEmpty()) { @@ -55,16 +58,17 @@ public static List getAliases(SchemaSql schema) { ImmutableList.Builder aliases = new ImmutableList.Builder<>(); - for (String relation : schema.getParentPath()) { - aliases.add(String.valueOf(alias++)); + for (int i = 0; i < schema.getParentPath().size(); i++) { + aliases.add(String.valueOf(alias)); + alias++; } - aliases.add(String.valueOf(alias++)); + aliases.add(String.valueOf(alias)); return aliases.build(); } - public static List getAliases(SqlQuerySchema schema) { + static List getAliases(SqlQuerySchema schema) { char alias = 'A'; if (schema.getRelations().isEmpty()) { @@ -73,28 +77,26 @@ public static List getAliases(SqlQuerySchema schema) { ImmutableList.Builder aliases = new ImmutableList.Builder<>(); - for (SqlQueryJoin relation : schema.getRelations()) { - aliases.add(String.valueOf(alias++)); + for (int i = 0; i < schema.getRelations().size(); i++) { + aliases.add(String.valueOf(alias)); + alias++; } - aliases.add(String.valueOf(alias++)); + aliases.add(String.valueOf(alias)); return aliases.build(); } - public static List getAliases(List tablePath) { - char alias = 'A'; - + static List getAliases(List tablePath) { ImmutableList.Builder aliases = new ImmutableList.Builder<>(); - for (Object table : tablePath) { - aliases.add(String.valueOf(alias++)); - } + IntStream.range(0, tablePath.size()) + .forEach(i -> aliases.add(String.valueOf((char) ('A' + i)))); return aliases.build(); } - public static List getAliases(SchemaSql schema, int level) { + static List getAliases(SchemaSql schema, int level) { if (level > 0) { String prefix = IntStream.range(0, level).mapToObj(i -> "A").collect(Collectors.joining()); @@ -104,7 +106,7 @@ public static List getAliases(SchemaSql schema, int level) { return getAliases(schema); } - public static List getAliases(SqlQuerySchema schema, int level) { + static List getAliases(SqlQuerySchema schema, int level) { if (level > 0) { String prefix = IntStream.range(0, level).mapToObj(i -> "A").collect(Collectors.joining()); @@ -114,7 +116,7 @@ public static List getAliases(SqlQuerySchema schema, int level) { return getAliases(schema); } - public static List getAliases(List tablePath, int level) { + static List getAliases(List tablePath, int level) { if (level > 0) { String prefix = IntStream.range(0, level).mapToObj(i -> "A").collect(Collectors.joining()); @@ -124,7 +126,7 @@ public static List getAliases(List tablePath, int level) { return getAliases(tablePath); } - public static List getAliases(List parents, SchemaSql schema, int level) { + static List getAliases(List parents, SchemaSql schema, int level) { if (level > 0) { String prefix = IntStream.range(0, level).mapToObj(i -> "A").collect(Collectors.joining()); diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/DecoderFactorySqlExpression.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/DecoderFactorySqlExpression.java index b648b3357..13fff958f 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/DecoderFactorySqlExpression.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/DecoderFactorySqlExpression.java @@ -56,6 +56,7 @@ public Decoder createDecoder() { } @Override + @SuppressWarnings("PMD.UseObjectForClearerAPI") public Tuple parseSourcePath( String path, String column, String flags, String connectorSpec) { Matcher matcher = SQL_FLAG.matcher(flags); diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureDataSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureDataSql.java index b803553a9..85f5979ce 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureDataSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureDataSql.java @@ -107,6 +107,7 @@ default ModifiableSqlRowData getCurrentRow(SqlQuerySchema table) { } // NOTE: json columns work using special handling in the encoder, the patch is applied to original + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) default FeatureDataSql patchWith(FeatureDataSql partial) { // joins not supported yet if (getRows().size() == 1 && partial.getRows().size() == 1) { @@ -147,9 +148,11 @@ default FeatureDataSql patchWith(FeatureDataSql partial) { return merged; } - LOGGER.warn( - "Patch is not supported for type '{}': the mapping contains joins", - getMapping().getMainSchema().getName()); + if (LOGGER.isWarnEnabled()) { + LOGGER.warn( + "Patch is not supported for type '{}': the mapping contains joins", + getMapping().getMainSchema().getName()); + } return this; } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureDecoderSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureDecoderSql.java index 889dc41a7..2bf044c3b 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureDecoderSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureDecoderSql.java @@ -39,6 +39,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@SuppressWarnings("PMD.CouplingBetweenObjects") public class FeatureDecoderSql extends FeatureTokenDecoder< SqlRow, FeatureSchema, SchemaMapping, ModifiableContext> @@ -66,7 +67,7 @@ public class FeatureDecoderSql private GeometryDecoderWkt geometryDecoderWkt; private GeometryDecoderWkb geometryDecoderWkb; private NestingTracker nestingTracker; - private Map, Integer> schemaIndexes; + private final Map, Integer> schemaIndexes; public FeatureDecoderSql( Map mappings, @@ -75,6 +76,7 @@ public FeatureDecoderSql( Map subDecoderFactories, boolean geometryAsWkb, WkbDialect wkbDialect) { + super(); this.mappings = mappings; this.query = query; this.geometryAsWkb = geometryAsWkb; @@ -173,6 +175,7 @@ private void handleMetaRow(SqlRowMeta sqlRow) { } } + @SuppressWarnings("PMD.CyclomaticComplexity") private void handleValueRow(SqlRow sqlRow) { if (LOGGER.isTraceEnabled()) { @@ -203,7 +206,6 @@ private void handleValueRow(SqlRow sqlRow) { || currentQueryIndex != sqlRow.getQueryIndex()) { if (featureStarted) { getDownstream().onFeatureEnd(context); - this.featureStarted = false; multiplicityTracker.reset(); subDecoders.values().forEach(Decoder::reset); } @@ -229,13 +231,14 @@ private void handleValueRow(SqlRow sqlRow) { } // TODO: move general parts to NestingTracker + @SuppressWarnings("PMD.CyclomaticComplexity") private void handleNesting(SqlRow sqlRow, List indexes) { while (nestingTracker.isNested() && (nestingTracker.doesNotStartWithPreviousPath(sqlRow.getPath()) - || (nestingTracker.inObject() && nestingTracker.isSamePath(sqlRow.getPath()) - || (nestingTracker.inArray() - && nestingTracker.isSamePath(sqlRow.getPath()) - && nestingTracker.hasParentIndexChanged(indexes))))) { + || nestingTracker.inObject() && nestingTracker.isSamePath(sqlRow.getPath()) + || (nestingTracker.inArray() + && nestingTracker.isSamePath(sqlRow.getPath()) + && nestingTracker.hasParentIndexChanged(indexes)))) { nestingTracker.close(); } @@ -260,6 +263,7 @@ private void handleNesting(SqlRow sqlRow, List indexes) { } } + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) private void handleColumns(SqlRow sqlRow) { for (int i = 0; i < sqlRow.getValues().size() && i < sqlRow.getColumnPaths().size(); i++) { // TODO: this is a workaround, ideally the paths SchemaMapping would contain the column diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureEncoderSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureEncoderSql.java index cdfbda0fd..26f026b1d 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureEncoderSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureEncoderSql.java @@ -27,6 +27,7 @@ import de.ii.xtraplatform.geometries.domain.transform.CoordinatesTransformer; import de.ii.xtraplatform.geometries.domain.transform.ImmutableCrsTransform; import java.io.IOException; +import java.time.DateTimeException; import java.time.LocalDateTime; import java.time.ZoneId; import java.time.ZonedDateTime; @@ -42,6 +43,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@SuppressWarnings("PMD.CouplingBetweenObjects") public class FeatureEncoderSql extends FeatureTokenEncoderBaseSimple< SqlQuerySchema, @@ -55,12 +57,9 @@ public class FeatureEncoderSql private final EpsgCrs inputCrs; private final EpsgCrs nativeCrs; private final CrsTransformerFactory crsTransformerFactory; - private final Optional crsTransformer; private final Optional timeZone; - private final Optional nullValue; - private Map jsonColumns; + private final Map jsonColumns; private final boolean isPatch; - private final boolean trace; private ModifiableFeatureDataSql currentFeature; private Tuple currentJsonColumn; @@ -79,16 +78,14 @@ public FeatureEncoderSql( CrsTransformerFactory crsTransformerFactory, Optional timeZone, Optional nullValue) { + super(); this.mapping = mapping; this.inputCrs = inputCrs; this.crsTransformerFactory = crsTransformerFactory; - this.crsTransformer = crsTransformerFactory.getTransformer(inputCrs, nativeCrs); this.nativeCrs = nativeCrs; this.timeZone = timeZone; - this.nullValue = nullValue; this.jsonColumns = new LinkedHashMap<>(); this.isPatch = nullValue.isPresent(); - this.trace = LOGGER.isTraceEnabled(); } @Modifiable @@ -102,6 +99,7 @@ public void onStart(ModifiableContext context) public void onEnd(ModifiableContext context) {} @Override + @SuppressWarnings("PMD.NullAssignment") public void onFeatureStart(ModifiableContext context) { currentFeature = ModifiableFeatureDataSql.create().setMapping(mapping); currentFeature.addRow(mapping.getMainTable()); @@ -110,12 +108,16 @@ public void onFeatureStart(ModifiableContext co jsonColumns.clear(); } currentArrayJunctionTable = null; - if (trace) LOGGER.trace("onFeatureStart: {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onFeatureStart: {}", context.pathAsString()); + } } @Override public void onFeatureEnd(ModifiableContext context) { - if (trace) LOGGER.trace("onFeatureEnd: {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onFeatureEnd: {}", context.pathAsString()); + } if (currentJsonColumn != null) { try { @@ -128,33 +130,34 @@ public void onFeatureEnd(ModifiableContext cont } } - currentFeature - .getRows() - .forEach( - row -> { - if (trace) - LOGGER.trace("push: {} {}", row.first().getFullPathAsString(), row.second()); - }); + if (LOGGER.isTraceEnabled()) { + currentFeature + .getRows() + .forEach( + row -> LOGGER.trace("push: {} {}", row.first().getFullPathAsString(), row.second())); + } push(currentFeature); } @Override public void onObjectStart(ModifiableContext context) { - if (trace) LOGGER.trace("onObjectStart: {}", context.pathAsString()); - - mapping.getMainSchema().getAllObjects().stream() - .filter(schema -> Objects.equals(schema.getFullPath(), context.path())) - .findFirst() - .ifPresent( - schema -> { - if (trace) LOGGER.trace("onObjectStart: {} {}", context.pathAsString(), schema); - }); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onObjectStart: {}", context.pathAsString()); + + mapping.getMainSchema().getAllObjects().stream() + .filter(schema -> Objects.equals(schema.getFullPath(), context.path())) + .findFirst() + .ifPresent( + schema -> LOGGER.trace("onObjectStart: {} {}", context.pathAsString(), schema)); + } Optional tableSchema = mapping.getTableForObject(context.pathAsString()); if (tableSchema.isPresent()) { - if (trace) LOGGER.trace("onObjectStart: table found for {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onObjectStart: table found for {}", context.pathAsString()); + } currentFeature.addRow(tableSchema.get()); return; @@ -166,21 +169,29 @@ public void onObjectStart(ModifiableContext con if (column.isPresent() && checkJson(column.get())) { currentJson.openObject(context.path()); - if (trace) LOGGER.trace("onObjectStart: JSON {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onObjectStart: JSON {}", context.pathAsString()); + } return; } - if (trace) LOGGER.warn("onObjectStart: no table found for {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onObjectStart: no table found for {}", context.pathAsString()); + } } @Override public void onObjectEnd(ModifiableContext context) { - if (trace) LOGGER.trace("onObjectEnd: {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onObjectEnd: {}", context.pathAsString()); + } Optional tableSchema = mapping.getTableForObject(context.pathAsString()); if (tableSchema.isPresent()) { - if (trace) LOGGER.trace("onObjectEnd: table found for {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onObjectEnd: table found for {}", context.pathAsString()); + } currentFeature.closeRow(tableSchema.get()); return; @@ -192,16 +203,22 @@ public void onObjectEnd(ModifiableContext conte if (column.isPresent() && checkJson(column.get())) { currentJson.closeObject(context.path()); - if (trace) LOGGER.trace("onObjectEnd: JSON {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onObjectEnd: JSON {}", context.pathAsString()); + } return; } - if (trace) LOGGER.warn("onObjectEnd: no table found for {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onObjectEnd: no table found for {}", context.pathAsString()); + } } @Override public void onArrayStart(ModifiableContext context) { - if (trace) LOGGER.trace("onArrayStart: {} {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onArrayStart: {}", context.pathAsString()); + } mapping .getColumnForValue(context.pathAsString(), MappingRule.Scope.W) @@ -210,7 +227,9 @@ public void onArrayStart(ModifiableContext cont if (checkJson(column)) { currentJson.openArray(context.path()); - if (trace) LOGGER.trace("onArrayStart: JSON {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onArrayStart: JSON {}", context.pathAsString()); + } return; } // VALUE_ARRAY whose value column lives on a junction table other than the row at the @@ -219,22 +238,27 @@ public void onArrayStart(ModifiableContext cont // junctions get a row per member via onObjectStart. if (!currentFeature.isCurrent(column.first())) { currentArrayJunctionTable = column.first(); - if (trace) + if (LOGGER.isTraceEnabled()) { LOGGER.trace( "onArrayStart: junction {} {}", context.pathAsString(), column.first().getFullPathAsString()); + } } }, () -> { - if (trace) - LOGGER.warn("onArrayStart: JSON {} not found in mapping", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onArrayStart: JSON {} not found in mapping", context.pathAsString()); + } }); } @Override + @SuppressWarnings("PMD.NullAssignment") public void onArrayEnd(ModifiableContext context) { - if (trace) LOGGER.trace("onArrayEnd: {} {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onArrayEnd: {}", context.pathAsString()); + } mapping .getColumnForValue(context.pathAsString(), MappingRule.Scope.W) @@ -243,12 +267,15 @@ public void onArrayEnd(ModifiableContext contex if (checkJson(column)) { currentJson.closeArray(context.path()); - if (trace) LOGGER.trace("onArrayEnd: JSON {}", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onArrayEnd: JSON {}", context.pathAsString()); + } } }, () -> { - if (trace) - LOGGER.warn("onArrayEnd: JSON {} not found in mapping", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onArrayEnd: JSON {} not found in mapping", context.pathAsString()); + } }); currentArrayJunctionTable = null; @@ -258,7 +285,7 @@ public void onArrayEnd(ModifiableContext contex public void onGeometry(ModifiableContext context) { Geometry geometry = context.geometry(); - if (trace) { + if (LOGGER.isTraceEnabled()) { LOGGER.trace("geometry: {} {}", context.pathAsString(), geometry); } @@ -288,13 +315,13 @@ public void onGeometry(ModifiableContext contex currentFeature.addColumn(column.first(), column.second(), value); - if (trace) { + if (LOGGER.isTraceEnabled()) { LOGGER.trace("onGeometry: {} {}", context.pathAsString(), value); } }, () -> { - if (trace) { - LOGGER.warn("onGeometry: {} not found in mapping", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onGeometry: {} not found in mapping", context.pathAsString()); } }); } @@ -325,6 +352,7 @@ private Optional transformerFor(Geometry geometry, EpsgCrs st } @Override + @SuppressWarnings("PMD.CognitiveComplexity") public void onValue(ModifiableContext context) { mapping .getColumnForValue(context.pathAsString(), MappingRule.Scope.W) @@ -337,7 +365,7 @@ public void onValue(ModifiableContext context) if (timeZone.isPresent() && column.second().getType() == Type.DATETIME && Objects.nonNull(value)) { - value = toTimeZone(context.pathAsString(), value, timeZone.get(), trace); + value = toTimeZone(context.pathAsString(), value, timeZone.get()); } if (checkJson(column)) { @@ -345,7 +373,9 @@ public void onValue(ModifiableContext context) // TODO: does this use the sql name or json name? currentJson.addValue(context.path(), value); - if (trace) LOGGER.trace("onValue: JSON {} {}", context.pathAsString(), value); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onValue: JSON {} {}", context.pathAsString(), value); + } return; } @@ -368,10 +398,14 @@ public void onValue(ModifiableContext context) currentFeature.closeRow(column.first()); } - if (trace) LOGGER.trace("onValue: {} {}", context.pathAsString(), value); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onValue: {} {}", context.pathAsString(), value); + } }, () -> { - if (trace) LOGGER.warn("onValue: {} not found in mapping", context.pathAsString()); + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("onValue: {} not found in mapping", context.pathAsString()); + } }); } @@ -388,23 +422,22 @@ public ModifiableContext createContext() { } private boolean checkJson(Tuple column) { - if (column.second().hasOperation(Operation.CONNECTOR)) { - if ("JSON".equals(column.second().getOperationParameter(Operation.CONNECTOR, ""))) { - if (currentJsonColumn == null) { - this.currentJsonColumn = column; - this.jsonColumns.put(currentJsonColumn.second().getPathSegment(), new JsonBuilder()); - this.currentJson = jsonColumns.get(currentJsonColumn.second().getPathSegment()); - this.currentJsonSetter = - currentFeature.addLazyColumn(currentJsonColumn.first(), currentJsonColumn.second()); - } - - return true; + if (column.second().hasOperation(Operation.CONNECTOR) + && "JSON".equals(column.second().getOperationParameter(Operation.CONNECTOR, ""))) { + if (currentJsonColumn == null) { + this.currentJsonColumn = column; + this.jsonColumns.put(currentJsonColumn.second().getPathSegment(), new JsonBuilder()); + this.currentJson = jsonColumns.get(currentJsonColumn.second().getPathSegment()); + this.currentJsonSetter = + currentFeature.addLazyColumn(currentJsonColumn.first(), currentJsonColumn.second()); } + + return true; } return false; } - private static String toTimeZone(String path, String value, ZoneId timeZone, boolean trace) { + private static String toTimeZone(String path, String value, ZoneId timeZone) { try { DateTimeFormatter parser = DateTimeFormatter.ISO_DATE_TIME; @@ -416,14 +449,16 @@ private static String toTimeZone(String path, String value, ZoneId timeZone, boo String newValue = formatter.format(instant) + "Z"; - if (trace) { + if (LOGGER.isTraceEnabled()) { LOGGER.trace( "onValue: {} transformed datetime value from '{}' to '{}'", path, value, newValue); } return newValue; - } catch (Throwable e) { - LOGGER.warn("Error while parsing datetime value for {}: {}", path, e.getMessage()); + } catch (DateTimeException e) { + if (LOGGER.isWarnEnabled()) { + LOGGER.warn("Error while parsing datetime value for {}: {}", path, e.getMessage()); + } } return value; @@ -432,16 +467,16 @@ private static String toTimeZone(String path, String value, ZoneId timeZone, boo private static String toWkt( Geometry geometry, Optional crsTransformer, EpsgCrs storageCrs) { - if (crsTransformer.isPresent()) { - geometry = - geometry.accept( - new CoordinatesTransformer( - ImmutableCrsTransform.of(Optional.empty(), crsTransformer.get()))); - } + Geometry transformedGeometry = + crsTransformer.isPresent() + ? geometry.accept( + new CoordinatesTransformer( + ImmutableCrsTransform.of(Optional.empty(), crsTransformer.get()))) + : geometry; String wkt; try { - wkt = new GeometryEncoderWkt().encode(geometry); + wkt = new GeometryEncoderWkt().encode(transformedGeometry); } catch (IOException e) { throw new IllegalStateException(e); } @@ -449,8 +484,8 @@ private static String toWkt( // TODO: functions from Dialect String result = String.format("ST_GeomFromText('%s',%s)", wkt, storageCrs.getCode()); - if (geometry.getType() == GeometryType.POLYGON - || geometry.getType() == GeometryType.MULTI_POLYGON) { + if (transformedGeometry.getType() == GeometryType.POLYGON + || transformedGeometry.getType() == GeometryType.MULTI_POLYGON) { result = String.format("ST_ForcePolygonCW(%s)", result); } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureMutationsSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureMutationsSql.java index b42b9f4b4..64ad13b08 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureMutationsSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureMutationsSql.java @@ -12,7 +12,6 @@ import de.ii.xtraplatform.features.domain.SchemaBase.Type; import de.ii.xtraplatform.features.domain.Tuple; import de.ii.xtraplatform.features.sql.domain.SqlClient; -import de.ii.xtraplatform.features.sql.domain.SqlPathDefaults; import de.ii.xtraplatform.features.sql.domain.SqlQueryColumn; import de.ii.xtraplatform.features.sql.domain.SqlQueryMapping; import de.ii.xtraplatform.features.sql.domain.SqlQueryOptions; @@ -27,24 +26,15 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.Stream; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class FeatureMutationsSql { - private static final Logger LOGGER = LoggerFactory.getLogger(FeatureMutationsSql.class); - private final Supplier sqlClient; private final FeatureStoreInsertGenerator generator; - private final SqlPathDefaults sqlPathDefaults; - public FeatureMutationsSql( - Supplier sqlClient, - FeatureStoreInsertGenerator generator, - SqlPathDefaults sqlPathDefaults) { + public FeatureMutationsSql(Supplier sqlClient, FeatureStoreInsertGenerator generator) { this.sqlClient = sqlClient; this.generator = generator; - this.sqlPathDefaults = sqlPathDefaults; } public Reactive.Transformer getCreatorFlow( @@ -135,7 +125,7 @@ Supplier>> createInstanceDelete( SqlQueryColumn idColumn = mapping .getColumnForId() - .map(col -> col.second()) + .map(de.ii.xtraplatform.base.domain.util.Tuple::second) .orElseThrow( () -> new IllegalStateException( diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureProviderSqlAuto.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureProviderSqlAuto.java index e03777de1..f27338e57 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureProviderSqlAuto.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureProviderSqlAuto.java @@ -23,6 +23,7 @@ import de.ii.xtraplatform.features.sql.domain.SqlClientBasic; import de.ii.xtraplatform.features.sql.domain.SqlClientBasicFactory; import de.ii.xtraplatform.features.sql.infra.db.SchemaGeneratorSql; +import java.io.IOException; import java.util.AbstractMap; import java.util.AbstractMap.SimpleImmutableEntry; import java.util.Comparator; @@ -44,7 +45,7 @@ public FeatureProviderSqlAuto(SqlClientBasicFactory sqlClientBasicFactory) { @Override public Map check(T entityData) { - return null; + return Map.of(); } @Override @@ -61,25 +62,21 @@ public Map> analyze(T entityData) { data.getId(), getConnectionInfoWith4Connections(data.getConnectionInfo())); - SchemaGeneratorSql schemaGeneratorSql = null; - - try { - schemaGeneratorSql = new SchemaGeneratorSql(sqlClientBasic); - + try (SchemaGeneratorSql schemaGeneratorSql = new SchemaGeneratorSql(sqlClientBasic)) { return schemaGeneratorSql.analyze(); + } catch (IOException e) { + return Map.of(); } finally { - if (Objects.nonNull(schemaGeneratorSql)) { - try { - schemaGeneratorSql.close(); - } catch (Throwable e) { - // ignore - } - try { - sqlClientBasicFactory.dispose(sqlClientBasic); - } catch (Throwable e) { - // ignore - } - } + disposeQuietly(sqlClientBasic); + } + } + + @SuppressWarnings("PMD.AvoidCatchingGenericException") + private void disposeQuietly(SqlClientBasic sqlClientBasic) { + try { + sqlClientBasicFactory.dispose(sqlClientBasic); + } catch (RuntimeException e) { + // ignore } } @@ -109,11 +106,7 @@ private FeatureProviderSqlData generateTypesIfNecessary( data.getId(), getConnectionInfoWith4Connections(data.getConnectionInfo())); - SchemaGeneratorSql schemaGeneratorSql = null; - - try { - schemaGeneratorSql = new SchemaGeneratorSql(sqlClientBasic); - + try (SchemaGeneratorSql schemaGeneratorSql = new SchemaGeneratorSql(sqlClientBasic)) { List featureSchemas = schemaGeneratorSql.generate(types, tracker); Map idCounter = new LinkedHashMap<>(); @@ -139,7 +132,7 @@ private FeatureProviderSqlData generateTypesIfNecessary( .map(Entry::getKey) .orElse("id"); - ImmutableMap typeMap = + Map typeMap = featureSchemas.stream() .map( type -> { @@ -173,19 +166,10 @@ private FeatureProviderSqlData generateTypesIfNecessary( } return builder.build(); + } catch (IOException e) { + throw new IllegalStateException(e); } finally { - if (Objects.nonNull(schemaGeneratorSql)) { - try { - schemaGeneratorSql.close(); - } catch (Throwable e) { - // ignore - } - try { - sqlClientBasicFactory.dispose(sqlClientBasic); - } catch (Throwable e) { - // ignore - } - } + disposeQuietly(sqlClientBasic); } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureProviderSqlFactory.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureProviderSqlFactory.java index f0277bf25..5a3ccc2b8 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureProviderSqlFactory.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureProviderSqlFactory.java @@ -15,7 +15,6 @@ import de.ii.xtraplatform.entities.domain.AutoEntityFactory; import de.ii.xtraplatform.entities.domain.EntityData; import de.ii.xtraplatform.entities.domain.EntityDataBuilder; -import de.ii.xtraplatform.entities.domain.EntityFactory; import de.ii.xtraplatform.entities.domain.PersistentEntity; import de.ii.xtraplatform.entities.domain.ValidationResult.MODE; import de.ii.xtraplatform.features.domain.ConnectorFactory; @@ -58,9 +57,9 @@ @Singleton @AutoBind +@SuppressWarnings("PMD.CouplingBetweenObjects") public class FeatureProviderSqlFactory - extends AbstractEntityFactory - implements EntityFactory { + extends AbstractEntityFactory { private static final Logger LOGGER = LoggerFactory.getLogger(FeatureProviderSqlFactory.class); @@ -159,18 +158,21 @@ public Optional auto() { } @Override + @SuppressWarnings("PMD.AvoidCatchingGenericException") public EntityData hydrateData(EntityData entityData) { - FeatureProviderSqlData data = (FeatureProviderSqlData) entityData; - if (skipHydration) { return entityData; } + FeatureProviderSqlData data = (FeatureProviderSqlData) entityData; + try { if (data.isAuto()) { - LOGGER.info( - "Feature provider with id '{}' is in auto mode, generating configuration ...", - data.getId()); + if (LOGGER.isInfoEnabled()) { + LOGGER.info( + "Feature provider with id '{}' is in auto mode, generating configuration ...", + data.getId()); + } ConnectionInfoSql connectionInfo = data.getConnectionInfo(); @@ -179,12 +181,11 @@ public EntityData hydrateData(EntityData entityData) { if (!schemas.isEmpty()) { Map> schemaTables = new LinkedHashMap<>(); + Map> allTables = tables; - for (String schema : schemas) { - if (tables.containsKey(schema)) { - schemaTables.put(schema, tables.get(schema)); - } - } + schemas.stream() + .filter(allTables::containsKey) + .forEach(schema -> schemaTables.put(schema, allTables.get(schema))); tables = schemaTables; } @@ -211,7 +212,7 @@ public EntityData hydrateData(EntityData entityData) { return data; - } catch (Throwable e) { + } catch (RuntimeException e) { LogContext.error( LOGGER, e, "Feature provider with id '{}' could not be started", data.getId()); } @@ -231,7 +232,8 @@ private FeatureProviderSqlData applyTypesResolver( while (resolver.needsResolving(types)) { types = resolver.resolve(types); - if (++rounds >= resolver.maxRounds()) { + rounds++; + if (rounds >= resolver.maxRounds()) { resolver.maxRoundsWarning().ifPresent(LOGGER::warn); break; } @@ -245,6 +247,7 @@ private FeatureProviderSqlData applyTypesResolver( } @AssistedFactory + @FunctionalInterface public interface ProviderSqlFactoryAssisted extends FactoryAssisted { @Override diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureQueryEncoderSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureQueryEncoderSql.java index 9fd955923..f4d2e6f11 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureQueryEncoderSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FeatureQueryEncoderSql.java @@ -17,7 +17,6 @@ import de.ii.xtraplatform.features.domain.MultiFeatureQuery; import de.ii.xtraplatform.features.domain.Query; import de.ii.xtraplatform.features.domain.SchemaBase; -import de.ii.xtraplatform.features.domain.SchemaBase.Type; import de.ii.xtraplatform.features.domain.SortKey; import de.ii.xtraplatform.features.domain.Tuple; import de.ii.xtraplatform.features.domain.TypeQuery; @@ -25,8 +24,6 @@ import de.ii.xtraplatform.features.sql.domain.ImmutableSqlQueryBatch; import de.ii.xtraplatform.features.sql.domain.ImmutableSqlQueryOptions; import de.ii.xtraplatform.features.sql.domain.ImmutableSqlQuerySet; -import de.ii.xtraplatform.features.sql.domain.SchemaSql.PropertyTypeInfo; -import de.ii.xtraplatform.features.sql.domain.SqlDialect; import de.ii.xtraplatform.features.sql.domain.SqlQueryBatch; import de.ii.xtraplatform.features.sql.domain.SqlQueryColumn; import de.ii.xtraplatform.features.sql.domain.SqlQueryMapping; @@ -43,31 +40,24 @@ import java.util.stream.IntStream; import java.util.stream.Stream; import org.apache.commons.lang3.function.TriFunction; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class FeatureQueryEncoderSql implements FeatureQueryEncoder { - private static final Logger LOGGER = LoggerFactory.getLogger(FeatureQueryEncoderSql.class); - private final Map> allQueryTemplates; private final Map> allQueryTemplatesMutations; private final int chunkSize; - private final SqlDialect sqlDialect; private final boolean geometryAsWkb; private final boolean computeNumberMatched; public FeatureQueryEncoderSql( Map> allQueryTemplates, Map> allQueryTemplatesMutations, - QueryGeneratorSettings queryGeneratorSettings, - SqlDialect sqlDialect) { + QueryGeneratorSettings queryGeneratorSettings) { this.allQueryTemplates = allQueryTemplates; this.allQueryTemplatesMutations = allQueryTemplatesMutations; this.chunkSize = queryGeneratorSettings.getChunkSize(); this.geometryAsWkb = queryGeneratorSettings.getGeometryAsWkb(); this.computeNumberMatched = queryGeneratorSettings.getComputeNumberMatched(); - this.sqlDialect = sqlDialect; } // TODO: add cql2 classes @@ -185,6 +175,7 @@ private SqlQueryBatch encode( .withQuerySets(querySets); } + @SuppressWarnings("PMD.CognitiveComplexity") private SqlQuerySet createQuerySet( SqlQueryTemplates queryTemplates, long limit, @@ -306,15 +297,4 @@ private List transformSortKeys(List sortKeys, SqlQueryMapping }) .collect(ImmutableList.toImmutableList()); } - - private boolean typeIsSortable(PropertyTypeInfo typeInfo) { - if (typeInfo.getInArray()) { - return false; - } - Type t = typeInfo.getType(); - if (typeInfo.getType() == Type.VALUE) { - t = typeInfo.getValueType().orElse(Type.STRING); - } - return t == Type.STRING || t == Type.INTEGER || t == Type.FLOAT || t == Type.FEATURE_REF; - } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FilterEncoderSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FilterEncoderSql.java index 8c595e3f7..d9e0a2fb2 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FilterEncoderSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/FilterEncoderSql.java @@ -40,7 +40,6 @@ import de.ii.xtraplatform.cql.domain.InResultSet; import de.ii.xtraplatform.cql.domain.IsNull; import de.ii.xtraplatform.cql.domain.Like; -import de.ii.xtraplatform.cql.domain.LogicalOperation; import de.ii.xtraplatform.cql.domain.Not; import de.ii.xtraplatform.cql.domain.Operand; import de.ii.xtraplatform.cql.domain.Property; @@ -93,6 +92,12 @@ import org.slf4j.LoggerFactory; import org.threeten.extra.Interval; +@SuppressWarnings({ + "PMD.CouplingBetweenObjects", + "PMD.TooManyStaticImports", + "PMD.GodClass", + "PMD.CyclomaticComplexity" +}) public class FilterEncoderSql { private static final Logger LOGGER = LoggerFactory.getLogger(FilterEncoderSql.class); @@ -100,6 +105,41 @@ public class FilterEncoderSql { static final String ROW_NUMBER = "row_number"; static final Splitter ARRAY_SPLITTER = Splitter.on(",").trimResults().omitEmptyStrings(); + // output column alias of every result-set CTE; consumers reference it as `SELECT FROM + // ` + /** Stable name of the single value column projected by a result-set producer. */ + public static final String RESULT_SET_VALUE_COLUMN = "rs_value"; + + private static final String CTE_VALUE_COL = RESULT_SET_VALUE_COLUMN; + private static final String DYNAMIC_REF_TYPE = "DYNAMIC"; + + private static final String PLACEHOLDER_1 = "%1$s"; + private static final String PLACEHOLDER_2 = "%2$s"; + private static final String PLACEHOLDER_PAIR = "%%1$s%1$s%%2$s"; + private static final String PLACEHOLDER_1_OPEN = "%1$s("; + private static final String PLACEHOLDER_2_CLOSE = ")%2$s"; + private static final String SQL_WHERE = " WHERE "; + private static final String FORMAT_QUALIFIED_COLUMN = "%s.%s"; + private static final String FORMAT_COALESCE = "COALESCE(%s,%s)"; + private static final String FORMAT_COLLATE = "%s COLLATE \"%s\""; + private static final String ERROR_UNKNOWN_PROPERTY = "Filter is invalid. Unknown property: %s"; + private static final String ERROR_ACCENTI_UNSUPPORTED = "ACCENTI() is not supported by this API."; + private static final String ROUTE_SEGMENT = "_route_"; + private static final String JSON_SUB_DECODER = "JSON"; + private static final String REGEX_STRIP_QUOTES = "^\"|\"$"; + private static final String HALF_BOUNDED_MIN = "'..'"; + private static final String FORMAT_OPERATOR_VALUE = " %s %s"; + private static final String DUMMY_PLACEHOLDER = "DUMMY"; + private static final String FORMAT_VALUE_OPERATOR_VALUE = "%s %s %s"; + private static final String SQL_SELECT_START = "(SELECT"; + private static final String FORMAT_INTERVAL = "(%s,%s)"; + private static final String ERROR_ARRAY_BOTH_PROPERTIES = + "Array predicates with property references on both sides are not supported."; + private static final String REGEX_STRIP_BRACKETS = "\\[|\\]"; + private static final String SQL_TRUE = "1=1"; + private static final String SQL_FALSE = "1=0"; + private static final String FORMAT_ARRAY_IN_GROUP_BY = " IN %1$s GROUP BY %2$s.%3$s"; + private final EpsgCrs nativeCrs; private final SqlDialect sqlDialect; private final CrsTransformerFactory crsTransformerFactory; @@ -166,8 +206,8 @@ public FilterEncoderSql( this.coordinatesTransformer = this::transformCoordinatesIfNecessary; } - private Optional renderCustomFunction( - de.ii.xtraplatform.cql.domain.Function function, List children) { + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) + private Optional renderCustomFunction(Function function, List children) { CustomFunction customFunction = customFunctions.get(function.getName().toUpperCase(Locale.ROOT)); if (Objects.isNull(customFunction)) { @@ -217,7 +257,7 @@ private Optional renderCustomFunction( String prefix = expression.substring(0, markerIndex); String suffix = expression.substring(markerIndex + marker.length()); - String result = String.format(anchorExpression, "%1$s" + prefix, suffix + "%2$s"); + String result = String.format(anchorExpression, PLACEHOLDER_1 + prefix, suffix + PLACEHOLDER_2); // BOOLEAN-returning functions are top-level predicates — finalize the subquery template // by replacing the remaining %1$s/%2$s placeholders with empty strings. @@ -225,7 +265,7 @@ private Optional renderCustomFunction( // fill in the placeholders itself. if (!customFunction.getReturns().isEmpty() && "BOOLEAN".equalsIgnoreCase(customFunction.getReturns().get(0))) { - result = result.replace("%1$s", "").replace("%2$s", ""); + result = result.replace(PLACEHOLDER_1, "").replace(PLACEHOLDER_2, ""); } return Optional.of(result); } @@ -251,14 +291,14 @@ private Optional resolveExpression(CustomFunction customFunction) { } private boolean operandHasSelectForTemplate(String expression) { - return expression.contains("%1$s") && expression.contains("%2$s"); + return expression.contains(PLACEHOLDER_1) && expression.contains(PLACEHOLDER_2); } private String reduceSelectToColumnForTemplate(String expression) { if (operandHasSelectForTemplate(expression)) { return String.format( - expression.contains(" WHERE ") - ? expression.substring(expression.indexOf(" WHERE ") + 7, expression.length() - 1) + expression.contains(SQL_WHERE) + ? expression.substring(expression.indexOf(SQL_WHERE) + 7, expression.length() - 1) : expression, "", ""); @@ -266,13 +306,6 @@ private String reduceSelectToColumnForTemplate(String expression) { return expression; } - // output column alias of every result-set CTE; consumers reference it as `SELECT FROM - // ` - /** Stable name of the single value column projected by a result-set producer. */ - public static final String RESULT_SET_VALUE_COLUMN = "rs_value"; - - private static final String CTE_VALUE_COL = RESULT_SET_VALUE_COLUMN; - /** * Collects the result-set subqueries of one top-level {@code inResultSet} predicate as named, * materialized CTEs so that each result set is evaluated exactly once instead of being @@ -281,7 +314,7 @@ private String reduceSelectToColumnForTemplate(String expression) { * be referenced by the parent. */ private static final class CteCollector { - private final LinkedHashMap ctes = new LinkedHashMap<>(); + private final Map ctes = new LinkedHashMap<>(); private final Map namesBySet = new java.util.HashMap<>(); private int counter; @@ -290,7 +323,8 @@ String register(String setName, Supplier bodySupplier) { if (existing != null) { return existing; } - String name = "_rs_" + (counter++) + "_" + setName.replaceAll("[^A-Za-z0-9_]", "_"); + String name = "_rs_" + counter + "_" + setName.replaceAll("[^A-Za-z0-9_]", "_"); + counter++; namesBySet.put(setName, name); // building the body registers any nested result sets first, preserving dependency order String body = bodySupplier.get(); @@ -364,7 +398,8 @@ String resultSetProducerSelect( valueTable.getRelations().isEmpty() ? valueTable : valueTable.getRelations().get(0); String join = JoinGenerator.getJoins(valueTable, aliases, this); String valueColumn = - String.format("%s.%s", aliases.get(aliases.size() - 1), setColumn.second().getName()); + String.format( + FORMAT_QUALIFIED_COLUMN, aliases.get(aliases.size() - 1), setColumn.second().getName()); Optional tableFilter = producerMapping.getMainTable().getFilter().map(filter -> (Cql2Expression) filter); @@ -377,7 +412,7 @@ String resultSetProducerSelect( effectiveFilter .map( filter -> - " WHERE " + SQL_WHERE + prepareExpression(filter) .accept(new CqlToSql2(producerMapping, collector))) .orElse(""); @@ -461,23 +496,26 @@ private CqlNode prepareExpression(Cql2Expression cqlFilter) { private Geometry transformCoordinatesIfNecessary( Geometry geometry, Optional sourceCrs) { - if (sourceCrs.isPresent() && !Objects.equals(sourceCrs.get(), nativeCrs)) { - Optional transformer = - crsTransformerFactory.getTransformer(sourceCrs.get(), nativeCrs, true); - if (transformer.isPresent()) { - Geometry transformed = - geometry.accept( - new CoordinatesTransformer( - ImmutableCrsTransform.of(Optional.empty(), transformer.get()))); - if (Objects.isNull(transformed)) { - throw new IllegalArgumentException( - String.format("Filter is invalid. Geometry cannot be transformed: %s", geometry)); - } + if (sourceCrs.isEmpty() || Objects.equals(sourceCrs.get(), nativeCrs)) { + return geometry; + } - return transformed; - } + Optional transformer = + crsTransformerFactory.getTransformer(sourceCrs.get(), nativeCrs, true); + if (transformer.isEmpty()) { + return geometry; } - return geometry; + + Geometry transformed = + geometry.accept( + new CoordinatesTransformer( + ImmutableCrsTransform.of(Optional.empty(), transformer.get()))); + if (Objects.isNull(transformed)) { + throw new IllegalArgumentException( + String.format("Filter is invalid. Geometry cannot be transformed: %s", geometry)); + } + + return transformed; } public Optional encodeRelationFilter( @@ -545,8 +583,6 @@ public Optional encodeRelationFilter2( return Optional.of(encodeNested(null, mergedFilter, table.get(), true)); } - private static final String DYNAMIC_REF_TYPE = "DYNAMIC"; - /** The valid target types of a property in a mapping, or empty if they are not constrained. */ private Optional> targetTypes(SqlQueryMapping mapping, String propertyName) { FeatureSchema schema = @@ -563,15 +599,16 @@ private Optional> targetTypes(SqlQueryMapping mapping, String proper * {@code type} sub-property (case 3). Empty when the target type is not constrained (case 4) or * any branch is open. */ + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) Optional> validTargetTypes(FeatureSchema schema) { if (Objects.isNull(schema)) { return Optional.empty(); } List members = - !schema.getConcat().isEmpty() - ? schema.getConcat() - : !schema.getCoalesce().isEmpty() ? schema.getCoalesce() : List.of(); + schema.getConcat().isEmpty() + ? (schema.getCoalesce().isEmpty() ? List.of() : schema.getCoalesce()) + : schema.getConcat(); if (!members.isEmpty()) { Set types = new LinkedHashSet<>(); for (FeatureSchema member : members) { @@ -614,6 +651,7 @@ private static Predicate getPropertyNameMatcher( && Objects.equals(propertyName, property.getSourcePath().get())); } + @SuppressWarnings({"PMD.GodClass", "PMD.CyclomaticComplexity", "PMD.TooManyMethods"}) private class CqlToSql extends CqlToText { private final SchemaSql rootSchema; @@ -638,7 +676,7 @@ protected SchemaSql getTable( .orElseThrow( () -> new IllegalArgumentException( - String.format("Filter is invalid. Unknown property: %s", propertyName))); + String.format(ERROR_UNKNOWN_PROPERTY, propertyName))); } return rootSchema.getAllObjects().stream() .filter( @@ -655,19 +693,20 @@ protected SchemaSql getTable( .orElseThrow( () -> new IllegalArgumentException( - String.format("Filter is invalid. Unknown property: %s", propertyName))); + String.format(ERROR_UNKNOWN_PROPERTY, propertyName))); } protected Tuple> getQualifiedColumn( SchemaSql table, String propertyName, String alias, boolean allowColumnFallback) { // TODO: support nested mapping filters - if (Objects.equals(table.getParentPath(), ImmutableList.of("_route_")) + if (Objects.equals(table.getParentPath(), ImmutableList.of(ROUTE_SEGMENT)) && "node".equals(propertyName)) { - return Tuple.of("_route_" + propertyName, Optional.empty()); + return Tuple.of(ROUTE_SEGMENT + propertyName, Optional.empty()); } - if (Objects.equals(table.getParentPath(), ImmutableList.of("_route_")) + if (Objects.equals(table.getParentPath(), ImmutableList.of(ROUTE_SEGMENT)) && "source".equals(propertyName)) { - return Tuple.of(String.format("%s.%s", alias, propertyName), Optional.empty()); + return Tuple.of( + String.format(FORMAT_QUALIFIED_COLUMN, alias, propertyName), Optional.empty()); } return table.getProperties().stream() .filter(getPropertyNameMatcher(propertyName, false)) @@ -678,12 +717,14 @@ protected Tuple> getQualifiedColumn( return Tuple.of( mapToSubDecoder(alias, column, propertyName), column.getSubDecoder()); } - String qualifiedColumn = String.format("%s.%s", alias, column.getName()); + String qualifiedColumn = + String.format(FORMAT_QUALIFIED_COLUMN, alias, column.getName()); if (column.isTemporal()) { - if (column.getType() == DATE) + if (column.getType() == DATE) { return Tuple.of( sqlDialect.applyToDate(qualifiedColumn, column.getFormat()), Optional.empty()); + } return Tuple.of( sqlDialect.applyToDatetime(qualifiedColumn, column.getFormat()), Optional.empty()); @@ -702,14 +743,15 @@ protected Tuple> getQualifiedColumn( ? Optional.of( Tuple.of( String.format( - "%s.%s", - alias, propertyName.substring(propertyName.lastIndexOf('.') + 1)), + FORMAT_QUALIFIED_COLUMN, + alias, + propertyName.substring(propertyName.lastIndexOf('.') + 1)), Optional.empty())) : Optional.empty()) .orElseThrow( () -> new IllegalArgumentException( - String.format("Filter is invalid. Unknown property: %s", propertyName))); + String.format(ERROR_UNKNOWN_PROPERTY, propertyName))); } private Optional getPropertyFromSubDecoder(SchemaSql schema, String propertyName) { @@ -719,7 +761,7 @@ private Optional getPropertyFromSubDecoder(SchemaSql schema, String p } private String mapToSubDecoder(String alias, SchemaSql column, String propertyName) { - if (column.getSubDecoder().filter("JSON"::equals).isPresent()) { + if (column.getSubDecoder().filter(JSON_SUB_DECODER::equals).isPresent()) { PropertyTypeInfo typeInfo = column.getSubDecoderTypes().get(propertyName); return sqlDialect.applyToJsonValue( alias, column.getName(), column.getSubDecoderPaths().get(propertyName), typeInfo); @@ -734,7 +776,7 @@ private String mapToSubDecoder(String alias, SchemaSql column, String propertyNa @Override public String visit(Property property, List children) { // strip double quotes from the property name - String propertyName = property.getName().replaceAll("^\"|\"$", ""); + String propertyName = property.getName().replaceAll(REGEX_STRIP_QUOTES, ""); boolean allowColumnFallback = !propertyName.contains("."); SchemaSql table = getTable(propertyName, false, allowColumnFallback); @@ -753,7 +795,9 @@ public String visit(Property property, List children) { boolean ignoreInstanceFilter = true; Optional userFilter; Optional userFilterTable = Optional.empty(); - if (!property.getNestedFilters().isEmpty()) { + if (property.getNestedFilters().isEmpty()) { + userFilter = Optional.empty(); + } else { Optional> nestedFilter = property.getNestedFilters().entrySet().stream().findFirst(); userFilter = nestedFilter.map(Entry::getValue); @@ -764,8 +808,6 @@ public String visit(Property property, List children) { } else { userFilterTable = Optional.ofNullable(getTable(userFilterPropertyName, false, false)); } - } else { - userFilter = Optional.empty(); } String join = @@ -778,7 +820,9 @@ public String visit(Property property, List children) { ignoreInstanceFilter, true, FilterEncoderSql.this); - if (!join.isEmpty()) join += " "; + if (!join.isEmpty()) { + join += " "; + } // When the predicate needs no sub-table join, its operand is a column reachable directly from // the main table (aliased A). Emit it as a direct conjunct instead of a redundant @@ -788,9 +832,10 @@ public String visit(Property property, List children) { // array // traversal is genuinely required (join non-empty) — there it is load-bearing for // cardinality. - if (join.isEmpty() && !Objects.equals(table.getParentPath(), ImmutableList.of("_route_"))) { + if (join.isEmpty() + && !Objects.equals(table.getParentPath(), ImmutableList.of(ROUTE_SEGMENT))) { return String.format( - "%%1$s%1$s%%2$s", + PLACEHOLDER_PAIR, getQualifiedColumn(table, propertyName, "A", allowColumnFallback).first()); } @@ -803,6 +848,7 @@ public String visit(Property property, List children) { qualifiedColumn); } + @SuppressWarnings("PMD.CyclomaticComplexity") private String getUserFilterPropertyName(Cql2Expression userFilter) { CqlNode nestedFilter = userFilter; Operand operand = null; @@ -821,7 +867,7 @@ private String getUserFilterPropertyName(Cql2Expression userFilter) { } if (operand instanceof Property) { return ((Property) operand).getName(); - } else if (operand instanceof de.ii.xtraplatform.cql.domain.Function) { + } else if (operand instanceof Function) { return operand.accept(this); } throw new IllegalArgumentException("unsupported nested filter"); @@ -833,59 +879,66 @@ public String visit(de.ii.xtraplatform.cql.domain.Interval interval, List children) { if (casei.getValue() instanceof ScalarLiteral) { - return children.get(0).toLowerCase(); + return children.get(0).toLowerCase(Locale.ROOT); } - if (children.get(0).contains("%1$s") && children.get(0).contains("%2$s")) { - return String.format(children.get(0), "%1$sLOWER(", ")%2$s"); + if (children.get(0).contains(PLACEHOLDER_1) && children.get(0).contains(PLACEHOLDER_2)) { + return String.format(children.get(0), "%1$sLOWER(", PLACEHOLDER_2_CLOSE); } return String.format("LOWER(%s)", children.get(0)); } @@ -904,21 +957,24 @@ public String visit(de.ii.xtraplatform.cql.domain.Casei casei, List chil @Override public String visit(de.ii.xtraplatform.cql.domain.Accenti accenti, List children) { if (accenti.getValue() instanceof ScalarLiteral) { - if (Objects.nonNull(accentiCollation)) - return String.format("%s COLLATE \"%s\"", children.get(0), accentiCollation); - throw new IllegalArgumentException("ACCENTI() is not supported by this API."); + if (Objects.nonNull(accentiCollation)) { + return String.format(FORMAT_COLLATE, children.get(0), accentiCollation); + } + throw new IllegalArgumentException(ERROR_ACCENTI_UNSUPPORTED); } if (Objects.nonNull(accentiCollation)) { - if (children.get(0).contains("%1$s") && children.get(0).contains("%2$s")) { - return children.get(0).replace("%2$s", " COLLATE \"" + accentiCollation + "\"%2$s"); + if (children.get(0).contains(PLACEHOLDER_1) && children.get(0).contains(PLACEHOLDER_2)) { + return children + .get(0) + .replace(PLACEHOLDER_2, " COLLATE \"" + accentiCollation + "\"" + PLACEHOLDER_2); } - return String.format("%s COLLATE \"%s\"", children.get(0), accentiCollation); + return String.format(FORMAT_COLLATE, children.get(0), accentiCollation); } - throw new IllegalArgumentException("ACCENTI() is not supported by this API."); + throw new IllegalArgumentException(ERROR_ACCENTI_UNSUPPORTED); } @Override - public String visit(de.ii.xtraplatform.cql.domain.Function function, List children) { + public String visit(Function function, List children) { Optional customExpression = renderCustomFunction(function, children); if (customExpression.isPresent()) { return customExpression.get(); @@ -928,35 +984,37 @@ public String visit(de.ii.xtraplatform.cql.domain.Function function, List processBinary(List operands, List children) { + private List processBinary(List children) { // The two operands may be either a property reference or a literal. // If there is at least one property reference, that fragment will // be used as the basis (mainExpression). If the other operand is @@ -977,14 +1035,15 @@ private List processBinary(List operands, List processTernary(List operands, List children) { + @SuppressWarnings("PMD.CognitiveComplexity") + private List processTernary(List children) { // The three operands may be either a property reference or a literal. // If there is at least one property reference, that fragment will // be used as the basis (mainExpression). If another operand is @@ -998,8 +1057,12 @@ private List processTernary(List operands, List childre boolean op2hasSelect = operandHasSelect(secondExpression); boolean op3hasSelect = operandHasSelect(thirdExpression); if (op1hasSelect) { - if (op2hasSelect) secondExpression = reduceSelectToColumn(children.get(1)); - if (op3hasSelect) thirdExpression = reduceSelectToColumn(children.get(2)); + if (op2hasSelect) { + secondExpression = reduceSelectToColumn(children.get(1)); + } + if (op3hasSelect) { + thirdExpression = reduceSelectToColumn(children.get(2)); + } } else { // the unusual case that a literal is on the left side if (op2hasSelect && !op3hasSelect) { @@ -1017,7 +1080,7 @@ private List processTernary(List operands, List childre thirdExpression = reduceSelectToColumn(children.get(2)); } else if (!op2hasSelect && !op3hasSelect) { // special case of three literals, we need to build the SQL expression - mainExpression = String.format("%%1$s%1$s%%2$s", children.get(0)); + mainExpression = String.format(PLACEHOLDER_PAIR, children.get(0)); } } @@ -1054,9 +1117,9 @@ public String visit(BinaryScalarOperation scalarOperation, List children String operator = SCALAR_OPERATORS.get(scalarOperation.getClass()); - List expressions = processBinary(scalarOperation.getArgs(), children); + List expressions = processBinary(children); - String operation = String.format(" %s %s", operator, expressions.get(1)); + String operation = String.format(FORMAT_OPERATOR_VALUE, operator, expressions.get(1)); return String.format(expressions.get(0), "", operation); } @@ -1064,16 +1127,17 @@ public String visit(BinaryScalarOperation scalarOperation, List children public String visit(Like like, List children) { String operator = SCALAR_OPERATORS.get(like.getClass()); - List expressions = processBinary(like.getArgs(), children); + List expressions = processBinary(children); // we may need to change the second expression String secondExpression = expressions.get(1); - String string = sqlDialect.applyToString("DUMMY"); - String functionStart = string.substring(0, string.indexOf("DUMMY")); - String functionEnd = string.substring(string.indexOf("DUMMY") + 5); + String string = sqlDialect.applyToString(DUMMY_PLACEHOLDER); + String functionStart = string.substring(0, string.indexOf(DUMMY_PLACEHOLDER)); + String functionEnd = string.substring(string.indexOf(DUMMY_PLACEHOLDER) + 5); - String operation = String.format("%s %s %s", functionEnd, operator, secondExpression); + String operation = + String.format(FORMAT_VALUE_OPERATOR_VALUE, functionEnd, operator, secondExpression); return String.format(expressions.get(0), functionStart, operation); } @@ -1084,13 +1148,15 @@ public String visit(In in, List children) { String mainExpression = children.get(0); if (!operandHasSelect(mainExpression)) { // special case of a literal, we need to build the SQL expression - mainExpression = String.format("%%1$s%1$s%%2$s", mainExpression); + mainExpression = String.format(PLACEHOLDER_PAIR, mainExpression); } // mainExpression is either a literal value or a SELECT expression String operation = String.format( - " %s %s", operator, String.join(", ", children.subList(1, children.size()))); + FORMAT_OPERATOR_VALUE, + operator, + String.join(", ", children.subList(1, children.size()))); return String.format(mainExpression, "", operation); } @@ -1099,17 +1165,19 @@ public String visit(IsNull isNull, List children) { String operator = SCALAR_OPERATORS.get(isNull.getClass()); String mainExpression = children.get(0); - if (!operandHasSelect(mainExpression)) { + if (operandHasSelect(mainExpression)) { + if (mainExpression.contains(SQL_SELECT_START)) { + // The property needs a join, so the operand is an EXISTS-style semi-join + // (A.id IN (SELECT ... WHERE )) built from INNER joins. Testing the joined + // column for NULL inside that subquery can never match: a feature without related + // rows contributes no subquery rows at all. "Property has no value" is the negation + // of "property has some value" (NOT EXISTS); the outer operand (A.) is + // never null, so the negation is exact. + return String.format("NOT (%s)", String.format(mainExpression, "", " IS NOT NULL")); + } + } else { // special case of a literal, we need to build the SQL expression - mainExpression = String.format("%%1$s%1$s%%2$s", mainExpression); - } else if (mainExpression.contains("(SELECT")) { - // The property needs a join, so the operand is an EXISTS-style semi-join - // (A.id IN (SELECT ... WHERE )) built from INNER joins. Testing the joined - // column for NULL inside that subquery can never match: a feature without related rows - // contributes no subquery rows at all. "Property has no value" is the negation of - // "property has some value" (NOT EXISTS); the outer operand (A.) is never - // null, so the negation is exact. - return String.format("NOT (%s)", String.format(mainExpression, "", " IS NOT NULL")); + mainExpression = String.format(PLACEHOLDER_PAIR, mainExpression); } // mainExpression is either a literal value or a SELECT expression @@ -1121,10 +1189,7 @@ public String visit(IsNull isNull, List children) { public String visit(Between between, List children) { String operator = SCALAR_OPERATORS.get(between.getClass()); - Scalar op1 = between.getValue().get(); - Scalar op2 = between.getLower().get(); - Scalar op3 = between.getUpper().get(); - List expressions = processTernary(ImmutableList.of(op1, op2, op3), children); + List expressions = processTernary(children); String operation = String.format(" %s %s AND %s", operator, expressions.get(1), expressions.get(2)); @@ -1134,54 +1199,58 @@ public String visit(Between between, List children) { @Override public String visit(BinaryTemporalOperation temporalOperation, List children) { String operator = sqlDialect.getTemporalOperator(temporalOperation.getTemporalOperator()); - if (Objects.isNull(operator)) + if (Objects.isNull(operator)) { throw new IllegalStateException( String.format("unexpected temporal operator: %s", temporalOperation.getClass())); + } - Temporal op1 = (Temporal) temporalOperation.getArgs().get(0); - Temporal op2 = (Temporal) temporalOperation.getArgs().get(1); + Temporal op1 = temporalOperation.getArgs().get(0); + Temporal op2 = temporalOperation.getArgs().get(1); + List resolvedChildren = children; + + // if op1 is a Function, nothing to do here, this was handled in the interval() function if (op1 instanceof Property) { // need to change "column" to "(column,column)" - children = + resolvedChildren = ImmutableList.of( - replaceColumnWithInterval(children.get(0), reduceSelectToColumn(children.get(0))), - children.get(1)); + replaceColumnWithInterval( + resolvedChildren.get(0), reduceSelectToColumn(resolvedChildren.get(0))), + resolvedChildren.get(1)); } else if (op1 instanceof TemporalLiteral) { // need to construct "(start, end)" where start and end are identical for an instant and end // is exclusive otherwise - children = + resolvedChildren = ImmutableList.of( String.format( "(%s, %s)", getStartAsString((TemporalLiteral) op1), getEndExclusiveAsString((TemporalLiteral) op1)), - children.get(1)); - } else if (op1 instanceof Function) { - // nothing to do here, this was handled in the interval() function + resolvedChildren.get(1)); } + // if op2 is a Function, nothing to do here, this was handled in the interval() function if (op2 instanceof Property) { // need to change "column" to "(column,column)" - children = + resolvedChildren = ImmutableList.of( - children.get(0), - replaceColumnWithInterval(children.get(1), reduceSelectToColumn(children.get(1)))); - } else if (op2 instanceof TemporalLiteral) { - if (((TemporalLiteral) op2).getType() == Function.class) { - // nothing to do, this was handled in the temporal literal - } else { - // we have a Java interval and need to construct "(start, end)" where start and end are - // identical for an instant and end is exclusive otherwise - children = ImmutableList.of(children.get(0), getInterval((TemporalLiteral) op2)); - } - } else if (op2 instanceof Function) { - // nothing to do here, this was handled in the interval() function - } - - List expressions = processBinary(ImmutableList.of(op1, op2), children); + resolvedChildren.get(0), + replaceColumnWithInterval( + resolvedChildren.get(1), reduceSelectToColumn(resolvedChildren.get(1)))); + } else if (op2 instanceof TemporalLiteral + && ((TemporalLiteral) op2).getType() != Function.class) { + // we have a Java interval and need to construct "(start, end)" where start and end are + // identical for an instant and end is exclusive otherwise; if it is a Function, nothing to + // do, this was handled in the temporal literal + resolvedChildren = + ImmutableList.of(resolvedChildren.get(0), getInterval((TemporalLiteral) op2)); + } + + List expressions = processBinary(resolvedChildren); return String.format( - expressions.get(0), "", String.format(" %s %s", operator, expressions.get(1))); + expressions.get(0), + "", + String.format(FORMAT_OPERATOR_VALUE, operator, expressions.get(1))); } /** @@ -1192,7 +1261,8 @@ public String visit(BinaryTemporalOperation temporalOperation, List chil * @return PostgreSQL interval */ private String getInterval(TemporalLiteral literal) { - return String.format("(%s,%s)", getStartAsString(literal), getEndExclusiveAsString(literal)); + return String.format( + FORMAT_INTERVAL, getStartAsString(literal), getEndExclusiveAsString(literal)); } private Object getStart(TemporalLiteral literal) { @@ -1215,10 +1285,11 @@ private Object getStart(TemporalLiteral literal) { private String getStartAsString(TemporalLiteral literal) { Object start = getStart(literal); - if (start instanceof Instant && start == Instant.MIN) + if (start instanceof Instant && start == Instant.MIN) { return sqlDialect.applyToDatetimeLiteral(sqlDialect.applyToInstantMin()); - else if (start instanceof LocalDate) + } else if (start instanceof LocalDate) { return sqlDialect.applyToDateLiteral(DateTimeFormatter.ISO_DATE.format((LocalDate) start)); + } return sqlDialect.applyToDatetimeLiteral(start.toString()); } @@ -1242,10 +1313,11 @@ private Object getEndExclusive(TemporalLiteral literal) { private String getEndExclusiveAsString(TemporalLiteral literal) { Object end = getEndExclusive(literal); - if (end instanceof Instant && end == Instant.MAX) + if (end instanceof Instant && end == Instant.MAX) { return sqlDialect.applyToDatetimeLiteral(sqlDialect.applyToInstantMax()); - else if (end instanceof LocalDate) + } else if (end instanceof LocalDate) { return sqlDialect.applyToDateLiteral(DateTimeFormatter.ISO_DATE.format((LocalDate) end)); + } return sqlDialect.applyToDatetimeLiteral(end.toString()); } @@ -1258,7 +1330,7 @@ public String visit(BinarySpatialOperation spatialOperation, List childr String match = sqlDialect.getSpatialOperatorMatch(spatialOperation.getSpatialOperator()); - List expressions = processBinary(spatialOperation.getArgs(), children); + List expressions = processBinary(children); return String.format( expressions.get(0), @@ -1272,11 +1344,15 @@ public String visit(BinarySpatialOperation spatialOperation, List childr @Override public String visit(TemporalLiteral temporalLiteral, List children) { if (temporalLiteral.getType() == Instant.class) { - Instant instant = ((Instant) temporalLiteral.getValue()); + Instant instant = (Instant) temporalLiteral.getValue(); String literal; - if (instant == Instant.MIN) literal = sqlDialect.applyToInstantMin(); - else if (instant == Instant.MAX) literal = sqlDialect.applyToInstantMax(); - else literal = ((Instant) temporalLiteral.getValue()).toString(); + if (instant == Instant.MIN) { + literal = sqlDialect.applyToInstantMin(); + } else if (instant == Instant.MAX) { + literal = sqlDialect.applyToInstantMax(); + } else { + literal = instant.toString(); + } return sqlDialect.applyToDatetimeLiteral(literal); } else if (temporalLiteral.getType() == Interval.class) { // this can only occur in the T_INTERSECTS() operator @@ -1291,7 +1367,7 @@ public String visit(TemporalLiteral temporalLiteral, List children) { Operand arg2 = interval.getArgs().get(1); assert arg2 instanceof TemporalLiteral; return String.format( - "(%s,%s)", + FORMAT_INTERVAL, getStartAsString((TemporalLiteral) arg1), getEndExclusiveAsString((TemporalLiteral) arg2)); } else if (temporalLiteral.getType() == LocalDate.class) { @@ -1301,7 +1377,7 @@ public String visit(TemporalLiteral temporalLiteral, List children) { // here we do not know, if we are Instant.MIN (first argument) or // Instant.MAX (second argument); so, we use a placeholder that we // then process in the interval() function - return "'..'"; + return HALF_BOUNDED_MIN; } throw new IllegalStateException("unsupported temporal SQL literal: " + temporalLiteral); } @@ -1318,15 +1394,13 @@ public String visit(Bbox bbox, List children) { Polygon.of( List.of( PositionList.of( - Axes.XY, - new double[] { - c.get(0), c.get(1), c.get(2), c.get(1), c.get(2), c.get(3), c.get(0), - c.get(3), c.get(0), c.get(1) - }))); + Axes.XY, c.get(0), c.get(1), c.get(2), c.get(1), c.get(2), c.get(3), c.get(0), + c.get(3), c.get(0), c.get(1)))); return visit(GeometryNode.of(polygon), ImmutableList.of()); } @Override + @SuppressWarnings({"PMD.NcssCount", "PMD.CognitiveComplexity", "PMD.NPathComplexity"}) public String visit(BinaryArrayOperation arrayOperation, List children) { // The two operands may be either a property reference or a literal. // If there is at least one property reference, that fragment will @@ -1340,10 +1414,8 @@ public String visit(BinaryArrayOperation arrayOperation, List children) boolean notInverse = true; if (op1hasSelect) { if (op2hasSelect) { - // TODO - throw new IllegalArgumentException( - "Array predicates with property references on both sides are not supported."); - // secondExpression = reduceSelectToColumn(children.get(1)); + // not yet supported; would need reduceSelectToColumn to combine both sides + throw new IllegalArgumentException(ERROR_ARRAY_BOTH_PROPERTIES); } } else { // the unusual case that a literal is on the left side @@ -1356,35 +1428,33 @@ public String visit(BinaryArrayOperation arrayOperation, List children) } else { // literal op literal, we can decide here List firstOp = - ARRAY_SPLITTER.splitToList(mainExpression.replaceAll("\\[|\\]", "")); + ARRAY_SPLITTER.splitToList(mainExpression.replaceAll(REGEX_STRIP_BRACKETS, "")); List secondOp = - ARRAY_SPLITTER.splitToList(secondExpression.replaceAll("\\[|\\]", "")); + ARRAY_SPLITTER.splitToList(secondExpression.replaceAll(REGEX_STRIP_BRACKETS, "")); switch (arrayOperation.getArrayOperator()) { case A_CONTAINS: // each item of the second array must be in the first array - return secondOp.stream() - .allMatch(item -> firstOp.stream().anyMatch(item2 -> item.equals(item2))) - ? "1=1" - : "1=0"; + return secondOp.stream().allMatch(item -> firstOp.stream().anyMatch(item::equals)) + ? SQL_TRUE + : SQL_FALSE; case A_EQUALS: // items must be identical - if (firstOp.size() != secondOp.size()) return "1=0"; - return secondOp.stream() - .allMatch(item -> firstOp.stream().anyMatch(item2 -> item.equals(item2))) - ? "1=1" - : "1=0"; + if (firstOp.size() != secondOp.size()) { + return SQL_FALSE; + } + return secondOp.stream().allMatch(item -> firstOp.stream().anyMatch(item::equals)) + ? SQL_TRUE + : SQL_FALSE; case A_OVERLAPS: // at least one common element - return secondOp.stream() - .anyMatch(item -> firstOp.stream().anyMatch(item2 -> item.equals(item2))) - ? "1=1" - : "1=0"; + return secondOp.stream().anyMatch(item -> firstOp.stream().anyMatch(item::equals)) + ? SQL_TRUE + : SQL_FALSE; case A_CONTAINEDBY: // each item of the first array must be in the second array - return firstOp.stream() - .allMatch(item -> secondOp.stream().anyMatch(item2 -> item.equals(item2))) - ? "1=1" - : "1=0"; + return firstOp.stream().allMatch(item -> secondOp.stream().anyMatch(item::equals)) + ? SQL_TRUE + : SQL_FALSE; } throw new IllegalArgumentException( "unsupported array operator: " + arrayOperation.getArrayOperator()); @@ -1392,9 +1462,8 @@ public String visit(BinaryArrayOperation arrayOperation, List children) } if (op1hasSelect && op2hasSelect) { - // TODO property op property - throw new IllegalArgumentException( - "Array predicates with property references on both sides are not supported."); + // property op property is not supported + throw new IllegalArgumentException(ERROR_ARRAY_BOTH_PROPERTIES); } // property op literal @@ -1418,8 +1487,10 @@ public String visit(BinaryArrayOperation arrayOperation, List children) String arrayQuery = elementCount == 1 ? String.format( - " IN %1$s GROUP BY %2$s.%3$s", - secondExpression, aliases.get(0), rootSchema.getSortKey().get()) + FORMAT_ARRAY_IN_GROUP_BY, + secondExpression, + aliases.get(0), + rootSchema.getSortKey().get()) : String.format( " IN %1$s GROUP BY %2$s.%3$s HAVING count(distinct %4$s) = %5$s", secondExpression, @@ -1441,8 +1512,10 @@ public String visit(BinaryArrayOperation arrayOperation, List children) } else if (arrayOperation.getArrayOperator() == A_OVERLAPS) { String arrayQuery = String.format( - " IN %1$s GROUP BY %2$s.%3$s", - secondExpression, aliases.get(0), rootSchema.getSortKey().get()); + FORMAT_ARRAY_IN_GROUP_BY, + secondExpression, + aliases.get(0), + rootSchema.getSortKey().get()); return String.format(mainExpression, "", arrayQuery); } else if (notInverse ? arrayOperation.getArrayOperator() == A_CONTAINEDBY @@ -1457,7 +1530,7 @@ public String visit(BinaryArrayOperation arrayOperation, List children) return String.format(mainExpression, "", arrayQuery); } } else { - if (qualifiedColumn.second().filter("JSON"::equals).isPresent()) { + if (qualifiedColumn.second().filter(JSON_SUB_DECODER::equals).isPresent()) { String jsonValueArray = secondExpression.replaceAll("'", "\"").replace('(', '[').replace(')', ']'); if (notInverse @@ -1500,18 +1573,12 @@ public String visit(ArrayLiteral arrayLiteral, List children) { } @Override - public String visit(LogicalOperation logicalOperation, List children) { - String operator = LOGICAL_OPERATORS.get(logicalOperation.getClass()); - - return super.visit(logicalOperation, children); - } - - @Override + @SuppressWarnings("PMD.CognitiveComplexity") public String visit(Not not, List children) { String operator = LOGICAL_OPERATORS.get(not.getClass()); String operation = children.get(0); - if (operation.contains("(SELECT")) { + if (operation.contains(SQL_SELECT_START)) { // The child predicate is (or contains) an EXISTS-style semi-join on a joined property // (A.id IN (SELECT ... WHERE )). The string surgery below would push the // negation into the subquery, negating the inner predicate (exists a related row that @@ -1521,7 +1588,7 @@ public String visit(Not not, List children) { // null, so NOT (...) is the exact logical negation. return String.format("%s (%s)", operator, operation); } - Integer pos = null; + int pos = -1; Cql2Expression arg = not.getArgs().get(0); if (arg instanceof In) { // replace last IN with NOT IN @@ -1535,9 +1602,7 @@ public String visit(Not not, List children) { } else if (arg instanceof IsNull) { // replace last IS NULL with IS NOT NULL pos = operation.lastIndexOf(" IS NULL"); - if (pos == -1) { - pos = null; - } else { + if (pos != -1) { pos += 3; } } else if (arg instanceof BinaryScalarOperation @@ -1545,19 +1610,19 @@ public String visit(Not not, List children) { || arg instanceof BinarySpatialOperation || arg instanceof BinaryTemporalOperation) { // replace last WHERE with WHERE NOT - pos = operation.lastIndexOf(" WHERE "); - if (pos == -1) { - pos = null; - } else { + pos = operation.lastIndexOf(SQL_WHERE); + if (pos != -1) { pos += 6; } } - if (pos != null) { + if (pos != -1) { int length = operation.length(); return String.format( - "%s %s %s", - operation.substring(0, pos), operator, operation.substring(pos + 1, length)); + FORMAT_VALUE_OPERATOR_VALUE, + operation.substring(0, pos), + operator, + operation.substring(pos + 1, length)); } return super.visit(not, children); @@ -1565,11 +1630,11 @@ public String visit(Not not, List children) { @Override public String visit(BooleanValue2 booleanValue, List children) { - return Boolean.TRUE.equals(booleanValue.getValue()) ? "1=1" : "1=0"; + return Boolean.TRUE.equals(booleanValue.getValue()) ? SQL_TRUE : SQL_FALSE; } } - private class CqlToSqlNested extends CqlToSql { + private final class CqlToSqlNested extends CqlToSql { private final SchemaSql schema; private final boolean isUserFilter; @@ -1584,17 +1649,17 @@ private CqlToSqlNested(SchemaSql schema, boolean isUserFilter) { .map(element -> element.replaceAll("\\{.*?\\}", "").replaceAll("\\[.*?\\]", "")) .collect(Collectors.toList()); this.allowedColumnPrefixes = new ArrayList<>(); - String current = ""; - for (int i = 0; i < parentTables.size(); i++) { - current += parentTables.get(i) + "."; - allowedColumnPrefixes.add(current); + StringBuilder current = new StringBuilder(); + for (String parentTable : parentTables) { + current.append(parentTable).append('.'); + allowedColumnPrefixes.add(current.toString()); } } @Override public String visit(Property property, List children) { // strip double quotes from the property name - String propertyName = property.getName().replaceAll("^\"|\"$", ""); + String propertyName = property.getName().replaceAll(REGEX_STRIP_QUOTES, ""); boolean hasPrefix = propertyName.contains("."); String prefix = hasPrefix ? propertyName.substring(0, propertyName.lastIndexOf('.') + 1) : ""; boolean hasAllowedPrefix = hasPrefix && allowedColumnPrefixes.contains(prefix); @@ -1609,14 +1674,15 @@ public String visit(Property property, List children) { .first(); // TODO: support nested mapping filters - if (qualifiedColumn.startsWith("_route_")) { - qualifiedColumn = "A." + qualifiedColumn.replace("_route_", ""); + if (qualifiedColumn.startsWith(ROUTE_SEGMENT)) { + qualifiedColumn = "A." + qualifiedColumn.replace(ROUTE_SEGMENT, ""); } - return String.format("%%1$s%1$s%%2$s", qualifiedColumn); + return String.format(PLACEHOLDER_PAIR, qualifiedColumn); } } + @SuppressWarnings("PMD.TooManyMethods") private class CqlToSql2 extends CqlToText { private final SqlQueryMapping mapping; @@ -1643,7 +1709,7 @@ protected FeatureSchema getSchema( .orElseThrow( () -> new IllegalArgumentException( - String.format("Filter is invalid. Unknown property: %s", propertyName))); + String.format(ERROR_UNKNOWN_PROPERTY, propertyName))); } return mapping .getSchemaForValue(propertyName) @@ -1651,7 +1717,7 @@ protected FeatureSchema getSchema( .orElseThrow( () -> new IllegalArgumentException( - String.format("Filter is invalid. Unknown property: %s", propertyName))); + String.format(ERROR_UNKNOWN_PROPERTY, propertyName))); } protected de.ii.xtraplatform.base.domain.util.Tuple @@ -1665,7 +1731,7 @@ protected FeatureSchema getSchema( .orElseThrow( () -> new IllegalArgumentException( - String.format("Filter is invalid. Unknown property: %s", propertyName))); + String.format(ERROR_UNKNOWN_PROPERTY, propertyName))); } return mapping .getColumnForValue(propertyName) @@ -1680,7 +1746,7 @@ protected FeatureSchema getSchema( .orElseThrow( () -> new IllegalArgumentException( - String.format("Filter is invalid. Unknown property: %s", propertyName))); + String.format(ERROR_UNKNOWN_PROPERTY, propertyName))); } // TODO: move to SqlQueryMapping? @@ -1692,13 +1758,14 @@ protected Tuple> getQualifiedColumn( boolean allowColumnFallback) { // TODO: why is this needed? what would a clean solution look like? (original: support nested // mapping filters) - if (Objects.equals(table.getParentPath(), ImmutableList.of("_route_")) + if (Objects.equals(table.getParentPath(), ImmutableList.of(ROUTE_SEGMENT)) && "node".equals(propertyName)) { - return Tuple.of("_route_" + propertyName, Optional.empty()); + return Tuple.of(ROUTE_SEGMENT + propertyName, Optional.empty()); } - if (Objects.equals(table.getParentPath(), ImmutableList.of("_route_")) + if (Objects.equals(table.getParentPath(), ImmutableList.of(ROUTE_SEGMENT)) && "source".equals(propertyName)) { - return Tuple.of(String.format("%s.%s", alias, propertyName), Optional.empty()); + return Tuple.of( + String.format(FORMAT_QUALIFIED_COLUMN, alias, propertyName), Optional.empty()); } return Optional.ofNullable(column) .map( @@ -1708,7 +1775,8 @@ protected Tuple> getQualifiedColumn( mapToSubDecoder(alias, col, propertyName, table.getColumnPath(col)), col.getOperationParameter(Operation.CONNECTOR)); } - String qualifiedColumn = String.format("%s.%s", alias, col.getName()); + String qualifiedColumn = + String.format(FORMAT_QUALIFIED_COLUMN, alias, col.getName()); if (col.getType() == DATE) { return Tuple.of( sqlDialect.applyToDate( @@ -1729,19 +1797,23 @@ protected Tuple> getQualifiedColumn( ? Optional.of( Tuple.of( String.format( - "%s.%s", - alias, propertyName.substring(propertyName.lastIndexOf('.') + 1)), + FORMAT_QUALIFIED_COLUMN, + alias, + propertyName.substring(propertyName.lastIndexOf('.') + 1)), Optional.empty())) : Optional.empty()) .orElseThrow( () -> new IllegalArgumentException( - String.format("Filter is invalid. Unknown property: %s", propertyName))); + String.format(ERROR_UNKNOWN_PROPERTY, propertyName))); } private String mapToSubDecoder( String alias, SqlQueryColumn column, String propertyName, List columnPath) { - if (column.getOperationParameter(Operation.CONNECTOR).filter("JSON"::equals).isPresent()) { + if (column + .getOperationParameter(Operation.CONNECTOR) + .filter(JSON_SUB_DECODER::equals) + .isPresent()) { FeatureSchema schema = mapping.getSchemaForValue(propertyName).orElseThrow(); String path = mapping.getPathInConnector(schema); boolean inArray = mapping.isInConnectedArray(schema); @@ -1771,7 +1843,7 @@ private String mapToSubDecoder( @Override public String visit(Property property, List children) { // strip double quotes from the property name - String propertyName = property.getName().replaceAll("^\"|\"$", ""); + String propertyName = property.getName().replaceAll(REGEX_STRIP_QUOTES, ""); boolean allowColumnFallback = !propertyName.contains("."); de.ii.xtraplatform.base.domain.util.Tuple table = getTableColumn(propertyName, false, allowColumnFallback); @@ -1793,7 +1865,9 @@ public String visit(Property property, List children) { boolean ignoreInstanceFilter = true; Optional userFilter; Optional userFilterTable = Optional.empty(); - if (!property.getNestedFilters().isEmpty()) { + if (property.getNestedFilters().isEmpty()) { + userFilter = Optional.empty(); + } else { Optional> nestedFilter = property.getNestedFilters().entrySet().stream().findFirst(); userFilter = nestedFilter.map(Entry::getValue); @@ -1808,8 +1882,6 @@ public String visit(Property property, List children) { Optional.ofNullable(getTableColumn(userFilterPropertyName, false, false)) .map(de.ii.xtraplatform.base.domain.util.Tuple::first); } - } else { - userFilter = Optional.empty(); } String join = @@ -1821,7 +1893,9 @@ public String visit(Property property, List children) { ignoreInstanceFilter, false, FilterEncoderSql.this); - if (!join.isEmpty()) join += " "; + if (!join.isEmpty()) { + join += " "; + } // When the predicate needs no sub-table join, its operand is a column reachable directly from // the main table (aliased A). Emit it as a direct conjunct instead of a redundant @@ -1832,9 +1906,9 @@ public String visit(Property property, List children) { // traversal is genuinely required (join non-empty) — there it is load-bearing for // cardinality. if (join.isEmpty() - && !Objects.equals(table.first().getParentPath(), ImmutableList.of("_route_"))) { + && !Objects.equals(table.first().getParentPath(), ImmutableList.of(ROUTE_SEGMENT))) { return String.format( - "%%1$s%1$s%%2$s", + PLACEHOLDER_PAIR, getQualifiedColumn( table.first(), table.second(), propertyName, "A", allowColumnFallback) .first()); @@ -1867,7 +1941,7 @@ private String getUserFilterPropertyName(Cql2Expression userFilter) { } if (operand instanceof Property) { return ((Property) operand).getName(); - } else if (operand instanceof de.ii.xtraplatform.cql.domain.Function) { + } else if (operand instanceof Function) { return operand.accept(this); } throw new IllegalArgumentException("unsupported nested filter"); @@ -1879,59 +1953,66 @@ public String visit(de.ii.xtraplatform.cql.domain.Interval interval, List children) { if (casei.getValue() instanceof ScalarLiteral) { - return children.get(0).toLowerCase(); + return children.get(0).toLowerCase(Locale.ROOT); } - if (children.get(0).contains("%1$s") && children.get(0).contains("%2$s")) { - return String.format(children.get(0), "%1$sLOWER(", ")%2$s"); + if (children.get(0).contains(PLACEHOLDER_1) && children.get(0).contains(PLACEHOLDER_2)) { + return String.format(children.get(0), "%1$sLOWER(", PLACEHOLDER_2_CLOSE); } return String.format("LOWER(%s)", children.get(0)); } @@ -1950,21 +2031,24 @@ public String visit(de.ii.xtraplatform.cql.domain.Casei casei, List chil @Override public String visit(de.ii.xtraplatform.cql.domain.Accenti accenti, List children) { if (accenti.getValue() instanceof ScalarLiteral) { - if (Objects.nonNull(accentiCollation)) - return String.format("%s COLLATE \"%s\"", children.get(0), accentiCollation); - throw new IllegalArgumentException("ACCENTI() is not supported by this API."); + if (Objects.nonNull(accentiCollation)) { + return String.format(FORMAT_COLLATE, children.get(0), accentiCollation); + } + throw new IllegalArgumentException(ERROR_ACCENTI_UNSUPPORTED); } if (Objects.nonNull(accentiCollation)) { - if (children.get(0).contains("%1$s") && children.get(0).contains("%2$s")) { - return children.get(0).replace("%2$s", " COLLATE \"" + accentiCollation + "\"%2$s"); + if (children.get(0).contains(PLACEHOLDER_1) && children.get(0).contains(PLACEHOLDER_2)) { + return children + .get(0) + .replace(PLACEHOLDER_2, " COLLATE \"" + accentiCollation + "\"" + PLACEHOLDER_2); } - return String.format("%s COLLATE \"%s\"", children.get(0), accentiCollation); + return String.format(FORMAT_COLLATE, children.get(0), accentiCollation); } - throw new IllegalArgumentException("ACCENTI() is not supported by this API."); + throw new IllegalArgumentException(ERROR_ACCENTI_UNSUPPORTED); } @Override - public String visit(de.ii.xtraplatform.cql.domain.Function function, List children) { + public String visit(Function function, List children) { Optional customExpression = renderCustomFunction(function, children); if (customExpression.isPresent()) { return customExpression.get(); @@ -1974,35 +2058,37 @@ public String visit(de.ii.xtraplatform.cql.domain.Function function, List processBinary(List operands, List children) { + private List processBinary(List children) { // The two operands may be either a property reference or a literal. // If there is at least one property reference, that fragment will // be used as the basis (mainExpression). If the other operand is @@ -2023,14 +2109,15 @@ private List processBinary(List operands, List processTernary(List operands, List children) { + @SuppressWarnings("PMD.CognitiveComplexity") + private List processTernary(List children) { // The three operands may be either a property reference or a literal. // If there is at least one property reference, that fragment will // be used as the basis (mainExpression). If another operand is @@ -2044,8 +2131,12 @@ private List processTernary(List operands, List childre boolean op2hasSelect = operandHasSelect(secondExpression); boolean op3hasSelect = operandHasSelect(thirdExpression); if (op1hasSelect) { - if (op2hasSelect) secondExpression = reduceSelectToColumn(children.get(1)); - if (op3hasSelect) thirdExpression = reduceSelectToColumn(children.get(2)); + if (op2hasSelect) { + secondExpression = reduceSelectToColumn(children.get(1)); + } + if (op3hasSelect) { + thirdExpression = reduceSelectToColumn(children.get(2)); + } } else { // the unusual case that a literal is on the left side if (op2hasSelect && !op3hasSelect) { @@ -2063,7 +2154,7 @@ private List processTernary(List operands, List childre thirdExpression = reduceSelectToColumn(children.get(2)); } else if (!op2hasSelect && !op3hasSelect) { // special case of three literals, we need to build the SQL expression - mainExpression = String.format("%%1$s%1$s%%2$s", children.get(0)); + mainExpression = String.format(PLACEHOLDER_PAIR, children.get(0)); } } @@ -2093,9 +2184,9 @@ public String visit(BinaryScalarOperation scalarOperation, List children String operator = SCALAR_OPERATORS.get(scalarOperation.getClass()); - List expressions = processBinary(scalarOperation.getArgs(), children); + List expressions = processBinary(children); - String operation = String.format(" %s %s", operator, expressions.get(1)); + String operation = String.format(FORMAT_OPERATOR_VALUE, operator, expressions.get(1)); return String.format(expressions.get(0), "", operation); } @@ -2131,7 +2222,7 @@ private String encodeInResultSet(InResultSet inResultSet, String mainExpression) // concat/coalesce, or a type sub-property with a constant or enum; otherwise they are // unconstrained and the check is skipped (no false negatives). String consumerProperty = - ((Property) inResultSet.getArgs().get(0)).getName().replaceAll("^\"|\"$", ""); + ((Property) inResultSet.getArgs().get(0)).getName().replaceAll(REGEX_STRIP_QUOTES, ""); Optional> consumerTargets = targetTypes(mapping, consumerProperty); Optional> setTypes = inResultSet.getProducerValues().isPresent() @@ -2197,16 +2288,17 @@ private String encodeInResultSet(InResultSet inResultSet, String mainExpression) public String visit(Like like, List children) { String operator = SCALAR_OPERATORS.get(like.getClass()); - List expressions = processBinary(like.getArgs(), children); + List expressions = processBinary(children); // we may need to change the second expression String secondExpression = expressions.get(1); - String string = sqlDialect.applyToString("DUMMY"); - String functionStart = string.substring(0, string.indexOf("DUMMY")); - String functionEnd = string.substring(string.indexOf("DUMMY") + 5); + String string = sqlDialect.applyToString(DUMMY_PLACEHOLDER); + String functionStart = string.substring(0, string.indexOf(DUMMY_PLACEHOLDER)); + String functionEnd = string.substring(string.indexOf(DUMMY_PLACEHOLDER) + 5); - String operation = String.format("%s %s %s", functionEnd, operator, secondExpression); + String operation = + String.format(FORMAT_VALUE_OPERATOR_VALUE, functionEnd, operator, secondExpression); return String.format(expressions.get(0), functionStart, operation); } @@ -2217,13 +2309,15 @@ public String visit(In in, List children) { String mainExpression = children.get(0); if (!operandHasSelect(mainExpression)) { // special case of a literal, we need to build the SQL expression - mainExpression = String.format("%%1$s%1$s%%2$s", mainExpression); + mainExpression = String.format(PLACEHOLDER_PAIR, mainExpression); } // mainExpression is either a literal value or a SELECT expression String operation = String.format( - " %s %s", operator, String.join(", ", children.subList(1, children.size()))); + FORMAT_OPERATOR_VALUE, + operator, + String.join(", ", children.subList(1, children.size()))); return String.format(mainExpression, "", operation); } @@ -2232,17 +2326,19 @@ public String visit(IsNull isNull, List children) { String operator = SCALAR_OPERATORS.get(isNull.getClass()); String mainExpression = children.get(0); - if (!operandHasSelect(mainExpression)) { + if (operandHasSelect(mainExpression)) { + if (mainExpression.contains(SQL_SELECT_START)) { + // The property needs a join, so the operand is an EXISTS-style semi-join + // (A.id IN (SELECT ... WHERE )) built from INNER joins. Testing the joined + // column for NULL inside that subquery can never match: a feature without related + // rows contributes no subquery rows at all. "Property has no value" is the negation + // of "property has some value" (NOT EXISTS); the outer operand (A.) is + // never null, so the negation is exact. + return String.format("NOT (%s)", String.format(mainExpression, "", " IS NOT NULL")); + } + } else { // special case of a literal, we need to build the SQL expression - mainExpression = String.format("%%1$s%1$s%%2$s", mainExpression); - } else if (mainExpression.contains("(SELECT")) { - // The property needs a join, so the operand is an EXISTS-style semi-join - // (A.id IN (SELECT ... WHERE )) built from INNER joins. Testing the joined - // column for NULL inside that subquery can never match: a feature without related rows - // contributes no subquery rows at all. "Property has no value" is the negation of - // "property has some value" (NOT EXISTS); the outer operand (A.) is never - // null, so the negation is exact. - return String.format("NOT (%s)", String.format(mainExpression, "", " IS NOT NULL")); + mainExpression = String.format(PLACEHOLDER_PAIR, mainExpression); } // mainExpression is either a literal value or a SELECT expression @@ -2254,10 +2350,7 @@ public String visit(IsNull isNull, List children) { public String visit(Between between, List children) { String operator = SCALAR_OPERATORS.get(between.getClass()); - Scalar op1 = between.getValue().get(); - Scalar op2 = between.getLower().get(); - Scalar op3 = between.getUpper().get(); - List expressions = processTernary(ImmutableList.of(op1, op2, op3), children); + List expressions = processTernary(children); String operation = String.format(" %s %s AND %s", operator, expressions.get(1), expressions.get(2)); @@ -2267,54 +2360,58 @@ public String visit(Between between, List children) { @Override public String visit(BinaryTemporalOperation temporalOperation, List children) { String operator = sqlDialect.getTemporalOperator(temporalOperation.getTemporalOperator()); - if (Objects.isNull(operator)) + if (Objects.isNull(operator)) { throw new IllegalStateException( String.format("unexpected temporal operator: %s", temporalOperation.getClass())); + } + + Temporal op1 = temporalOperation.getArgs().get(0); + Temporal op2 = temporalOperation.getArgs().get(1); - Temporal op1 = (Temporal) temporalOperation.getArgs().get(0); - Temporal op2 = (Temporal) temporalOperation.getArgs().get(1); + List resolvedChildren = children; + // if op1 is a Function, nothing to do here, this was handled in the interval() function if (op1 instanceof Property) { // need to change "column" to "(column,column)" - children = + resolvedChildren = ImmutableList.of( - replaceColumnWithInterval(children.get(0), reduceSelectToColumn(children.get(0))), - children.get(1)); + replaceColumnWithInterval( + resolvedChildren.get(0), reduceSelectToColumn(resolvedChildren.get(0))), + resolvedChildren.get(1)); } else if (op1 instanceof TemporalLiteral) { // need to construct "(start, end)" where start and end are identical for an instant and end // is exclusive otherwise - children = + resolvedChildren = ImmutableList.of( String.format( "(%s, %s)", getStartAsString((TemporalLiteral) op1), getEndExclusiveAsString((TemporalLiteral) op1)), - children.get(1)); - } else if (op1 instanceof Function) { - // nothing to do here, this was handled in the interval() function + resolvedChildren.get(1)); } + // if op2 is a Function, nothing to do here, this was handled in the interval() function if (op2 instanceof Property) { // need to change "column" to "(column,column)" - children = + resolvedChildren = ImmutableList.of( - children.get(0), - replaceColumnWithInterval(children.get(1), reduceSelectToColumn(children.get(1)))); - } else if (op2 instanceof TemporalLiteral) { - if (((TemporalLiteral) op2).getType() == Function.class) { - // nothing to do, this was handled in the temporal literal - } else { - // we have a Java interval and need to construct "(start, end)" where start and end are - // identical for an instant and end is exclusive otherwise - children = ImmutableList.of(children.get(0), getInterval((TemporalLiteral) op2)); - } - } else if (op2 instanceof Function) { - // nothing to do here, this was handled in the interval() function - } - - List expressions = processBinary(ImmutableList.of(op1, op2), children); + resolvedChildren.get(0), + replaceColumnWithInterval( + resolvedChildren.get(1), reduceSelectToColumn(resolvedChildren.get(1)))); + } else if (op2 instanceof TemporalLiteral + && ((TemporalLiteral) op2).getType() != Function.class) { + // we have a Java interval and need to construct "(start, end)" where start and end are + // identical for an instant and end is exclusive otherwise; if it is a Function, nothing to + // do, this was handled in the temporal literal + resolvedChildren = + ImmutableList.of(resolvedChildren.get(0), getInterval((TemporalLiteral) op2)); + } + + List expressions = processBinary(resolvedChildren); return String.format( - expressions.get(0), "", String.format(" %s %s", operator, expressions.get(1))); + expressions.get(0), + "", + String.format(FORMAT_OPERATOR_VALUE, operator, expressions.get(1))); } /** @@ -2325,7 +2422,8 @@ public String visit(BinaryTemporalOperation temporalOperation, List chil * @return PostgreSQL interval */ private String getInterval(TemporalLiteral literal) { - return String.format("(%s,%s)", getStartAsString(literal), getEndExclusiveAsString(literal)); + return String.format( + FORMAT_INTERVAL, getStartAsString(literal), getEndExclusiveAsString(literal)); } private Object getStart(TemporalLiteral literal) { @@ -2348,10 +2446,11 @@ private Object getStart(TemporalLiteral literal) { private String getStartAsString(TemporalLiteral literal) { Object start = getStart(literal); - if (start instanceof Instant && start == Instant.MIN) + if (start instanceof Instant && start == Instant.MIN) { return sqlDialect.applyToDatetimeLiteral(sqlDialect.applyToInstantMin()); - else if (start instanceof LocalDate) + } else if (start instanceof LocalDate) { return sqlDialect.applyToDateLiteral(DateTimeFormatter.ISO_DATE.format((LocalDate) start)); + } return sqlDialect.applyToDatetimeLiteral(start.toString()); } @@ -2375,10 +2474,11 @@ private Object getEndExclusive(TemporalLiteral literal) { private String getEndExclusiveAsString(TemporalLiteral literal) { Object end = getEndExclusive(literal); - if (end instanceof Instant && end == Instant.MAX) + if (end instanceof Instant && end == Instant.MAX) { return sqlDialect.applyToDatetimeLiteral(sqlDialect.applyToInstantMax()); - else if (end instanceof LocalDate) + } else if (end instanceof LocalDate) { return sqlDialect.applyToDateLiteral(DateTimeFormatter.ISO_DATE.format((LocalDate) end)); + } return sqlDialect.applyToDatetimeLiteral(end.toString()); } @@ -2391,7 +2491,7 @@ public String visit(BinarySpatialOperation spatialOperation, List childr String match = sqlDialect.getSpatialOperatorMatch(spatialOperation.getSpatialOperator()); - List expressions = processBinary(spatialOperation.getArgs(), children); + List expressions = processBinary(children); return String.format( expressions.get(0), @@ -2405,11 +2505,15 @@ public String visit(BinarySpatialOperation spatialOperation, List childr @Override public String visit(TemporalLiteral temporalLiteral, List children) { if (temporalLiteral.getType() == Instant.class) { - Instant instant = ((Instant) temporalLiteral.getValue()); + Instant instant = (Instant) temporalLiteral.getValue(); String literal; - if (instant == Instant.MIN) literal = sqlDialect.applyToInstantMin(); - else if (instant == Instant.MAX) literal = sqlDialect.applyToInstantMax(); - else literal = ((Instant) temporalLiteral.getValue()).toString(); + if (instant == Instant.MIN) { + literal = sqlDialect.applyToInstantMin(); + } else if (instant == Instant.MAX) { + literal = sqlDialect.applyToInstantMax(); + } else { + literal = instant.toString(); + } return sqlDialect.applyToDatetimeLiteral(literal); } else if (temporalLiteral.getType() == Interval.class) { // this can only occur in the T_INTERSECTS() operator @@ -2424,7 +2528,7 @@ public String visit(TemporalLiteral temporalLiteral, List children) { Operand arg2 = interval.getArgs().get(1); assert arg2 instanceof TemporalLiteral; return String.format( - "(%s,%s)", + FORMAT_INTERVAL, getStartAsString((TemporalLiteral) arg1), getEndExclusiveAsString((TemporalLiteral) arg2)); } else if (temporalLiteral.getType() == LocalDate.class) { @@ -2434,7 +2538,7 @@ public String visit(TemporalLiteral temporalLiteral, List children) { // here we do not know, if we are Instant.MIN (first argument) or // Instant.MAX (second argument); so, we use a placeholder that we // then process in the interval() function - return "'..'"; + return HALF_BOUNDED_MIN; } throw new IllegalStateException("unsupported temporal SQL literal: " + temporalLiteral); } @@ -2451,15 +2555,13 @@ public String visit(Bbox bbox, List children) { Polygon.of( List.of( PositionList.of( - Axes.XY, - new double[] { - c.get(0), c.get(1), c.get(2), c.get(1), c.get(2), c.get(3), c.get(0), - c.get(3), c.get(0), c.get(1) - }))); + Axes.XY, c.get(0), c.get(1), c.get(2), c.get(1), c.get(2), c.get(3), c.get(0), + c.get(3), c.get(0), c.get(1)))); return visit(GeometryNode.of(polygon), ImmutableList.of()); } @Override + @SuppressWarnings({"PMD.NcssCount", "PMD.CognitiveComplexity", "PMD.NPathComplexity"}) public String visit(BinaryArrayOperation arrayOperation, List children) { // The two operands may be either a property reference or a literal. // If there is at least one property reference, that fragment will @@ -2474,8 +2576,7 @@ public String visit(BinaryArrayOperation arrayOperation, List children) if (op1hasSelect) { if (op2hasSelect) { // TODO - throw new IllegalArgumentException( - "Array predicates with property references on both sides are not supported."); + throw new IllegalArgumentException(ERROR_ARRAY_BOTH_PROPERTIES); // secondExpression = reduceSelectToColumn(children.get(1)); } } else { @@ -2489,35 +2590,33 @@ public String visit(BinaryArrayOperation arrayOperation, List children) } else { // literal op literal, we can decide here List firstOp = - ARRAY_SPLITTER.splitToList(mainExpression.replaceAll("\\[|\\]", "")); + ARRAY_SPLITTER.splitToList(mainExpression.replaceAll(REGEX_STRIP_BRACKETS, "")); List secondOp = - ARRAY_SPLITTER.splitToList(secondExpression.replaceAll("\\[|\\]", "")); + ARRAY_SPLITTER.splitToList(secondExpression.replaceAll(REGEX_STRIP_BRACKETS, "")); switch (arrayOperation.getArrayOperator()) { case A_CONTAINS: // each item of the second array must be in the first array - return secondOp.stream() - .allMatch(item -> firstOp.stream().anyMatch(item2 -> item.equals(item2))) - ? "1=1" - : "1=0"; + return secondOp.stream().allMatch(item -> firstOp.stream().anyMatch(item::equals)) + ? SQL_TRUE + : SQL_FALSE; case A_EQUALS: // items must be identical - if (firstOp.size() != secondOp.size()) return "1=0"; - return secondOp.stream() - .allMatch(item -> firstOp.stream().anyMatch(item2 -> item.equals(item2))) - ? "1=1" - : "1=0"; + if (firstOp.size() != secondOp.size()) { + return SQL_FALSE; + } + return secondOp.stream().allMatch(item -> firstOp.stream().anyMatch(item::equals)) + ? SQL_TRUE + : SQL_FALSE; case A_OVERLAPS: // at least one common element - return secondOp.stream() - .anyMatch(item -> firstOp.stream().anyMatch(item2 -> item.equals(item2))) - ? "1=1" - : "1=0"; + return secondOp.stream().anyMatch(item -> firstOp.stream().anyMatch(item::equals)) + ? SQL_TRUE + : SQL_FALSE; case A_CONTAINEDBY: // each item of the first array must be in the second array - return firstOp.stream() - .allMatch(item -> secondOp.stream().anyMatch(item2 -> item.equals(item2))) - ? "1=1" - : "1=0"; + return firstOp.stream().allMatch(item -> secondOp.stream().anyMatch(item::equals)) + ? SQL_TRUE + : SQL_FALSE; } throw new IllegalArgumentException( "unsupported array operator: " + arrayOperation.getArrayOperator()); @@ -2526,8 +2625,7 @@ public String visit(BinaryArrayOperation arrayOperation, List children) if (op1hasSelect && op2hasSelect) { // TODO property op property - throw new IllegalArgumentException( - "Array predicates with property references on both sides are not supported."); + throw new IllegalArgumentException(ERROR_ARRAY_BOTH_PROPERTIES); } // property op literal @@ -2553,8 +2651,10 @@ public String visit(BinaryArrayOperation arrayOperation, List children) String arrayQuery = elementCount == 1 ? String.format( - " IN %1$s GROUP BY %2$s.%3$s", - secondExpression, aliases.get(0), mapping.getMainTable().getSortKey()) + FORMAT_ARRAY_IN_GROUP_BY, + secondExpression, + aliases.get(0), + mapping.getMainTable().getSortKey()) : String.format( " IN %1$s GROUP BY %2$s.%3$s HAVING count(distinct %4$s) = %5$s", secondExpression, @@ -2576,8 +2676,10 @@ public String visit(BinaryArrayOperation arrayOperation, List children) } else if (arrayOperation.getArrayOperator() == A_OVERLAPS) { String arrayQuery = String.format( - " IN %1$s GROUP BY %2$s.%3$s", - secondExpression, aliases.get(0), mapping.getMainTable().getSortKey()); + FORMAT_ARRAY_IN_GROUP_BY, + secondExpression, + aliases.get(0), + mapping.getMainTable().getSortKey()); return String.format(mainExpression, "", arrayQuery); } else if (notInverse ? arrayOperation.getArrayOperator() == A_CONTAINEDBY @@ -2592,7 +2694,7 @@ public String visit(BinaryArrayOperation arrayOperation, List children) return String.format(mainExpression, "", arrayQuery); } } else { - if (qualifiedColumn.second().filter("JSON"::equals).isPresent()) { + if (qualifiedColumn.second().filter(JSON_SUB_DECODER::equals).isPresent()) { String jsonValueArray = secondExpression.replaceAll("'", "\"").replace('(', '[').replace(')', ']'); if (notInverse @@ -2635,18 +2737,12 @@ public String visit(ArrayLiteral arrayLiteral, List children) { } @Override - public String visit(LogicalOperation logicalOperation, List children) { - String operator = LOGICAL_OPERATORS.get(logicalOperation.getClass()); - - return super.visit(logicalOperation, children); - } - - @Override + @SuppressWarnings("PMD.CognitiveComplexity") public String visit(Not not, List children) { String operator = LOGICAL_OPERATORS.get(not.getClass()); String operation = children.get(0); - if (operation.contains("(SELECT")) { + if (operation.contains(SQL_SELECT_START)) { // The child predicate is (or contains) an EXISTS-style semi-join on a joined property // (A.id IN (SELECT ... WHERE )). The string surgery below would push the // negation into the subquery, negating the inner predicate (exists a related row that @@ -2656,7 +2752,7 @@ public String visit(Not not, List children) { // null, so NOT (...) is the exact logical negation. return String.format("%s (%s)", operator, operation); } - Integer pos = null; + int pos = -1; Cql2Expression arg = not.getArgs().get(0); if (arg instanceof In) { // replace last IN with NOT IN @@ -2670,9 +2766,7 @@ public String visit(Not not, List children) { } else if (arg instanceof IsNull) { // replace last IS NULL with IS NOT NULL pos = operation.lastIndexOf(" IS NULL"); - if (pos == -1) { - pos = null; - } else { + if (pos != -1) { pos += 3; } } else if (arg instanceof BinaryScalarOperation @@ -2680,19 +2774,19 @@ public String visit(Not not, List children) { || arg instanceof BinarySpatialOperation || arg instanceof BinaryTemporalOperation) { // replace last WHERE with WHERE NOT - pos = operation.lastIndexOf(" WHERE "); - if (pos == -1) { - pos = null; - } else { + pos = operation.lastIndexOf(SQL_WHERE); + if (pos != -1) { pos += 6; } } - if (pos != null) { + if (pos != -1) { int length = operation.length(); return String.format( - "%s %s %s", - operation.substring(0, pos), operator, operation.substring(pos + 1, length)); + FORMAT_VALUE_OPERATOR_VALUE, + operation.substring(0, pos), + operator, + operation.substring(pos + 1, length)); } return super.visit(not, children); @@ -2700,11 +2794,11 @@ public String visit(Not not, List children) { @Override public String visit(BooleanValue2 booleanValue, List children) { - return Boolean.TRUE.equals(booleanValue.getValue()) ? "1=1" : "1=0"; + return Boolean.TRUE.equals(booleanValue.getValue()) ? SQL_TRUE : SQL_FALSE; } } - private class CqlToSqlNested2 extends CqlToSql2 { + private final class CqlToSqlNested2 extends CqlToSql2 { private final List tablePath; private final boolean isUserFilter; @@ -2718,17 +2812,17 @@ private CqlToSqlNested2( List parentTables = tablePath.subList(0, tablePath.size() - 1).stream().map(SqlQueryTable::getName).toList(); this.allowedColumnPrefixes = new ArrayList<>(); - String current = ""; - for (int i = 0; i < parentTables.size(); i++) { - current += parentTables.get(i) + "."; - allowedColumnPrefixes.add(current); + StringBuilder current = new StringBuilder(); + for (String parentTable : parentTables) { + current.append(parentTable).append('.'); + allowedColumnPrefixes.add(current.toString()); } } @Override public String visit(Property property, List children) { // strip double quotes from the property name - String propertyName = property.getName().replaceAll("^\"|\"$", ""); + String propertyName = property.getName().replaceAll(REGEX_STRIP_QUOTES, ""); /*boolean hasPrefix = propertyName.contains("."); String prefix = hasPrefix ? propertyName.substring(0, propertyName.lastIndexOf(".") + 1) : ""; boolean hasAllowedPrefix = hasPrefix && allowedColumnPrefixes.contains(prefix); @@ -2756,36 +2850,16 @@ public String visit(Property property, List children) { String qualifiedColumn = getQualifiedColumn(propertyName); // TODO: support nested mapping filters - if (qualifiedColumn.startsWith("_route_")) { - qualifiedColumn = "A." + qualifiedColumn.replace("_route_", ""); - } - - return String.format("%%1$s%1$s%%2$s", qualifiedColumn); - } - - // TODO: columns do not have to be defined in the mapping - // TODO: how to handle table prefixes? - private de.ii.xtraplatform.base.domain.util.Tuple getTableColumn( - String column, boolean hasPrefix, String prefix) { - for (int i = tablePath.size() - 1; i >= 0; i--) { - SqlQueryTable table = tablePath.get(i); - if (table instanceof SqlQuerySchema) { - SqlQuerySchema schema = (SqlQuerySchema) table; - /*if (schema.getColumnNames().contains(column)) { - return de.ii.xtraplatform.base.domain.util.Tuple.of( - schema, schema.getColumnNames().indexOf(column)); - }*/ - } + if (qualifiedColumn.startsWith(ROUTE_SEGMENT)) { + qualifiedColumn = "A." + qualifiedColumn.replace(ROUTE_SEGMENT, ""); } - throw new IllegalStateException("unknown table column: " + column); + return String.format(PLACEHOLDER_PAIR, qualifiedColumn); } private String getQualifiedColumn(String propertyName) { boolean hasPrefix = propertyName.contains("."); String prefix = hasPrefix ? propertyName.substring(0, propertyName.lastIndexOf('.') + 1) : ""; - String column = - hasPrefix ? propertyName.substring(propertyName.lastIndexOf('.') + 1) : propertyName; if (hasPrefix && !allowedColumnPrefixes.contains(prefix)) { throw new IllegalStateException( @@ -2798,7 +2872,9 @@ private String getQualifiedColumn(String propertyName) { ? aliases.get(allowedColumnPrefixes.indexOf(prefix)) : aliases.get(aliases.size() - 1); - return String.format("%s.%s", alias, column); + String column = + hasPrefix ? propertyName.substring(propertyName.lastIndexOf('.') + 1) : propertyName; + return String.format(FORMAT_QUALIFIED_COLUMN, alias, column); } } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/JoinGenerator.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/JoinGenerator.java index 91dd30b5c..23b3d3c9f 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/JoinGenerator.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/JoinGenerator.java @@ -20,7 +20,9 @@ import java.util.stream.IntStream; import java.util.stream.Stream; -public class JoinGenerator { +public final class JoinGenerator { + + private JoinGenerator() {} public static String getJoins( SchemaSql table, @@ -73,8 +75,12 @@ public static String getJoins( table.getRelation().stream()) .filter(t -> !t.getTargetField().equals(userFilterTargetField)) .flatMap( - relation -> - toJoins(relation, aliasesIterator, relationFilters.get(i[0]++), instanceFilter)) + relation -> { + int index = i[0]; + i[0]++; + return toJoins( + relation, aliasesIterator, relationFilters.get(index), instanceFilter); + }) .collect(Collectors.joining(" ")); return String.format( "%1$s%3$s%2$s", @@ -114,8 +120,12 @@ public static String getJoins( table.getRelations().stream() .filter(t -> !t.getTargetField().equals(userFilterTargetField)) .flatMap( - relation -> - toJoins(relation, aliasesIterator, relationFilters.get(i[0]++), instanceFilter)) + relation -> { + int index = i[0]; + i[0]++; + return toJoins( + relation, aliasesIterator, relationFilters.get(index), instanceFilter); + }) .collect(Collectors.joining(" ")); return String.format( "%1$s%3$s%2$s", diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/MutationSchemaBuilderSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/MutationSchemaBuilderSql.java index a64554cdb..0f0b45e0f 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/MutationSchemaBuilderSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/MutationSchemaBuilderSql.java @@ -100,27 +100,27 @@ private SchemaSql switchRelation(SchemaSql parent, SchemaSql child, List newChildPath = - !parent.getRelation().isEmpty() - ? Lists.newArrayList(Iterables.concat(parent.getParentPath(), childRelation.asPath())) - : ImmutableList.of(child.getName()); + parent.getRelation().isEmpty() + ? ImmutableList.of(child.getName()) + : Lists.newArrayList(Iterables.concat(parent.getParentPath(), childRelation.asPath())); - newChild = replaceInParentPath(!parent.getRelation().isEmpty() ? 1 : 0, newChildPath, newChild); + newChild = replaceInParentPath(parent.getRelation().isEmpty() ? 0 : 1, newChildPath, newChild); // TODO: rebuild mainTable without relation, change nested parentPaths // TODO: make schema child of mainTable, change nested parentPaths Optional newParentRelation = - !parent.getRelation().isEmpty() - ? Optional.of( + parent.getRelation().isEmpty() + ? Optional.empty() + : Optional.of( new ImmutableSqlRelation.Builder() .from(parent.getRelation().get(0)) .sourceContainer(parent.getRelation().get(0).getSourceContainer()) .sourceField(parent.getRelation().get(0).getSourceField()) .targetContainer(childRelation.getTargetContainer()) .targetField(childRelation.getTargetField()) - .build()) - : Optional.empty(); + .build()); List newParentPath = newParentRelation.isPresent() ? parent.getParentPath() : ImmutableList.of(child.getName()); diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/MutationSchemaDeriver.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/MutationSchemaDeriver.java index fbebf7cae..22297cb91 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/MutationSchemaDeriver.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/MutationSchemaDeriver.java @@ -46,8 +46,7 @@ public MutationSchemaDeriver(PathParserSql pathParser, SqlPathParser pathParser3 public SchemaSql create(List path, FeatureSchema targetSchema) { List path2 = path.stream() - .map( - path1 -> !targetSchema.isValue() ? pathParser3.tablePathWithDefaults(path1) : path1) + .map(path1 -> targetSchema.isValue() ? path1 : pathParser3.tablePathWithDefaults(path1)) .collect(Collectors.toList()); if (LOGGER.isTraceEnabled()) { @@ -73,7 +72,7 @@ public SchemaSql create(String path, FeatureSchema targetSchema) { } if (targetSchema.isValue()) { - de.ii.xtraplatform.features.sql.domain.SqlPath strings = pathParser3.parseColumnPath(path); + pathParser3.parseColumnPath(path); } return null; @@ -86,8 +85,6 @@ public List createParents( String path = JOINER.join(child.getFullPath()); if (!shouldIgnore(parentParentPath)) { path = parentParentPath + "/" + path; - } else { - boolean br = true; } Optional sqlPath = pathParser.parse(path, child.isValue()); @@ -97,11 +94,6 @@ public List createParents( throw new IllegalArgumentException("Parse error for SQL path: " + path); } - List tablePathAsList = - ReverseSchemaDeriver.SPLITTER.splitToList(sqlPath.get().getTablePath()); - - boolean hasRelation = tablePathAsList.size() > 1; // (isRoot ? 1 : 0); - List relations = ImmutableList .of(); // TODO hasRelation ? pathParser.toRelations(tablePathAsList, ImmutableMap.of()) @@ -209,7 +201,9 @@ public SchemaSql prependToSourcePath(String parentSourcePath, SchemaSql schema) @Override public String ignore() { - return IGNORE + ignoreCounter++; + String result = IGNORE + ignoreCounter; + ignoreCounter++; + return result; } private boolean shouldIgnore(String path) { diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/PathParserSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/PathParserSql.java index e8aab4a03..dc5892abd 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/PathParserSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/PathParserSql.java @@ -8,7 +8,6 @@ package de.ii.xtraplatform.features.sql.app; import com.google.common.collect.ImmutableList; -import de.ii.xtraplatform.cql.domain.Cql; import de.ii.xtraplatform.cql.domain.Cql2Expression; import de.ii.xtraplatform.features.domain.FeatureStoreRelation; import de.ii.xtraplatform.features.domain.ImmutableFeatureStoreRelation; @@ -26,19 +25,13 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.Stream; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class PathParserSql { - private static final Logger LOGGER = LoggerFactory.getLogger(PathParserSql.class); - private final SqlPathSyntax syntax; - private final Cql cql; - public PathParserSql(SqlPathSyntax syntax, Cql cql) { + public PathParserSql(SqlPathSyntax syntax) { this.syntax = syntax; - this.cql = cql; } public Optional parse(String path, boolean isColumn) { @@ -72,8 +65,6 @@ public Optional parse(String path, boolean isColumn) { List tablePathAsList = syntax.asList(tablePath); boolean isRoot = tablePathAsList.size() == 1; boolean isJunction = syntax.isJunctionTable(tablePathAsList.get(tablePathAsList.size() - 1)); - Optional queryable = - syntax.getQueryableFlag(flags).map(q -> q.replaceAll("\\[", "").replaceAll("]", "")); boolean isSpatial = syntax.getSpatialFlag(flags); try { @@ -89,9 +80,8 @@ public Optional parse(String path, boolean isColumn) { .queryable("" /*queryable.get()*/) .isSpatial(isSpatial) .build()); - } catch (Throwable e) { + } catch (IllegalStateException e) { // invalid path - boolean br = true; } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/QuerySchemaDeriver.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/QuerySchemaDeriver.java index 765f79279..e69e9bca4 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/QuerySchemaDeriver.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/QuerySchemaDeriver.java @@ -36,6 +36,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +@SuppressWarnings({"PMD.CouplingBetweenObjects", "PMD.GodClass"}) public class QuerySchemaDeriver implements MappedSchemaDeriver { private final SqlPathParser pathParser; @@ -73,6 +74,7 @@ public boolean hasRootPath(FeatureSchema sourceSchema) { } @Override + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity", "PMD.NPathComplexity"}) public SchemaSql create( FeatureSchema targetSchema, SqlPath path, @@ -120,15 +122,17 @@ public SchemaSql create( Type type = isConnected && !isExpression ? Type.STRING : targetSchema.getType(); Optional valueType = targetSchema.getValueType(); - if (!targetSchema.getConcat().isEmpty()) { - if (!relations.isEmpty()) { - sortKey = Optional.empty(); - } else if (type == Type.VALUE_ARRAY) { - type = valueType.orElse(Type.STRING); - valueType = Optional.empty(); - } else if (targetSchema.isFeature() && type == Type.OBJECT_ARRAY) { - type = Type.OBJECT; - } + boolean hasConcat = !targetSchema.getConcat().isEmpty(); + if (hasConcat && relations.isEmpty() && type == Type.VALUE_ARRAY) { + type = valueType.orElse(Type.STRING); + valueType = Optional.empty(); + } else if (hasConcat + && relations.isEmpty() + && targetSchema.isFeature() + && type == Type.OBJECT_ARRAY) { + type = Type.OBJECT; + } else if (hasConcat && !relations.isEmpty()) { + sortKey = Optional.empty(); } Builder builder = @@ -237,6 +241,7 @@ private static SubConnector getSubDecoders( return new SubConnector(subDecoderPaths, subDecoderTypes); } + @SuppressWarnings("PMD.CognitiveComplexity") private static Map getSubConnectors( List properties, FeatureSchema targetSchema, SqlPath path, boolean nestedArray) { Map> subConnectorPaths = new HashMap<>(); @@ -332,7 +337,6 @@ private List createTableParents( if (newProperties.stream().anyMatch(p -> !p.getRelation().isEmpty())) { newProperties = createTableParents(newProperties, targetSchema, relations, sortKeys); - boolean br = true; } boolean isArray = entry.getValue().stream().anyMatch(SchemaBase::isArray); @@ -362,9 +366,9 @@ private List createTableParents( .sortKeyUnique(tablePath.getSortKeyUnique()) .primaryKey(tablePath.getPrimaryKey()) .sourcePath( - !targetSchema.isFeature() - ? Optional.of(targetSchema.getName()) - : Optional.empty()) + targetSchema.isFeature() + ? Optional.empty() + : Optional.of(targetSchema.getName())) .build()); }) .collect(Collectors.toList()); @@ -393,16 +397,16 @@ private static Function> adopt( prop.getSourcePath() .map( sourcePath -> - !targetSchema.isFeature() - ? targetSchema.getName() + "." + sourcePath - : sourcePath)) + targetSchema.isFeature() + ? sourcePath + : targetSchema.getName() + "." + sourcePath)) .sourcePaths( prop.getSourcePaths().stream() .map( sourcePath -> - !targetSchema.isFeature() - ? targetSchema.getName() + "." + sourcePath - : sourcePath) + targetSchema.isFeature() + ? sourcePath + : targetSchema.getName() + "." + sourcePath) .collect(Collectors.toList())) .build()); }; @@ -417,17 +421,8 @@ private static Stream adjustSourcePathsAndKeys( List childRelations = hasValueSiblings ? propertiesByRelation.getKey() - : !relations.isEmpty() - ? propertiesByRelation.getKey().stream() - .map( - rel -> - new ImmutableSqlRelation.Builder() - .from(rel) - .sourceSortKey(Optional.empty()) - .sourcePrimaryKey(Optional.empty()) - .build()) - .collect(Collectors.toList()) - : propertiesByRelation.getKey().size() > 1 + : relations.isEmpty() + ? propertiesByRelation.getKey().size() > 1 ? Stream.concat( Stream.of(propertiesByRelation.getKey().get(0)), propertiesByRelation @@ -442,7 +437,16 @@ private static Stream adjustSourcePathsAndKeys( .sourcePrimaryKey(Optional.empty()) .build())) .collect(Collectors.toList()) - : propertiesByRelation.getKey(); + : propertiesByRelation.getKey() + : propertiesByRelation.getKey().stream() + .map( + rel -> + new ImmutableSqlRelation.Builder() + .from(rel) + .sourceSortKey(Optional.empty()) + .sourcePrimaryKey(Optional.empty()) + .build()) + .collect(Collectors.toList()); List childSortKeys = Stream.concat( @@ -530,54 +534,54 @@ private static Map, List> groupByRelation( Collectors.groupingBy( SchemaSql::getRelation, LinkedHashMap::new, Collectors.toList())); - Map, List> groupedByRelation2 = - groupedByRelation.entrySet().stream() - .flatMap( - entry -> { - List relation1 = entry.getKey(); - List mergeable = - groupedByRelation.entrySet().stream() - .filter( - entry2 -> - relation1.size() > 0 - && relation1.size() < entry2.getKey().size() - && Objects.equals( - relation1, entry2.getKey().subList(0, relation1.size()))) - .flatMap(entry2 -> entry2.getValue().stream()) - /*.map( - prop -> - new Builder() - .from(prop) - .relation( - prop.getRelation() - .subList(relation1.size(), prop.getRelation().size())) - .addAllParentPath( - prop.getRelation().subList(0, relation1.size()).stream() - .flatMap(s -> s.asPath().stream()) - .collect(Collectors.toList())) - .build())*/ - .collect(Collectors.toList()); - - if (!mergeable.isEmpty()) { - List newProps = - Stream.concat(entry.getValue().stream(), mergeable.stream()) - .collect(Collectors.toList()); - - return Stream.of(new SimpleImmutableEntry<>(relation1, newProps)); - } else if (groupedByRelation.keySet().stream() - .anyMatch( - relation2 -> - relation2.size() > 0 - && relation2.size() < relation1.size() + return groupedByRelation.entrySet().stream() + .flatMap( + entry -> { + List relation1 = entry.getKey(); + List mergeable = + groupedByRelation.entrySet().stream() + .filter( + entry2 -> + !relation1.isEmpty() + && relation1.size() < entry2.getKey().size() && Objects.equals( - relation2, relation1.subList(0, relation2.size())))) { - return Stream.empty(); - } - - return Stream.of(new SimpleImmutableEntry<>(relation1, entry.getValue())); - }) - .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); - return groupedByRelation2; + relation1, entry2.getKey().subList(0, relation1.size()))) + .flatMap(entry2 -> entry2.getValue().stream()) + /*.map( + prop -> + new Builder() + .from(prop) + .relation( + prop.getRelation() + .subList(relation1.size(), prop.getRelation().size())) + .addAllParentPath( + prop.getRelation().subList(0, relation1.size()).stream() + .flatMap(s -> s.asPath().stream()) + .collect(Collectors.toList())) + .build())*/ + .collect(Collectors.toList()); + + if (mergeable.isEmpty()) { + if (groupedByRelation.keySet().stream() + .anyMatch( + relation2 -> + !relation2.isEmpty() + && relation2.size() < relation1.size() + && Objects.equals( + relation2, relation1.subList(0, relation2.size())))) { + return Stream.empty(); + } + } else { + List newProps = + Stream.concat(entry.getValue().stream(), mergeable.stream()) + .collect(Collectors.toList()); + + return Stream.of(new SimpleImmutableEntry<>(relation1, newProps)); + } + + return Stream.of(new SimpleImmutableEntry<>(relation1, entry.getValue())); + }) + .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); } private static PropertyTypeInfo getTypeInfo(SchemaSql column, boolean nestedArray) { @@ -621,7 +625,7 @@ private static List prefixSourcePath(List schemas, String private static List adjustParentSortKeys( List schemas, List parentSortKeys) { - ArrayList keys = new ArrayList<>(parentSortKeys); + List keys = new ArrayList<>(parentSortKeys); return schemas.stream() .map( diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/ResultSetMaterializer.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/ResultSetMaterializer.java index 9f5b963ca..cc7d6e4f9 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/ResultSetMaterializer.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/ResultSetMaterializer.java @@ -51,6 +51,7 @@ * (when the dialect supports it); otherwise it is left unmaterialized and falls back to the inline * (CTE) re-evaluation. */ +@SuppressWarnings({"PMD.CouplingBetweenObjects", "PMD.GodClass"}) public class ResultSetMaterializer { private static final Logger LOGGER = LoggerFactory.getLogger(ResultSetMaterializer.class); @@ -94,6 +95,12 @@ public MultiFeatureQuery materialize(MultiFeatureQuery query) { * oversized result sets into {@code createdTables}. The caller owns their lifecycle and must * {@link #dropTables(java.util.Collection) drop} them once the query's stream has completed. */ + @SuppressWarnings({ + "PMD.CognitiveComplexity", + "PMD.CyclomaticComplexity", + "PMD.NPathComplexity", + "PMD.AvoidInstantiatingObjectsInLoops" + }) public MultiFeatureQuery materialize(MultiFeatureQuery query, List createdTables) { Map sets = new LinkedHashMap<>(); for (SubQuery subQuery : query.getQueries()) { @@ -318,7 +325,8 @@ private void materializeTable( } /** Drops the given result-set tables, best effort. Safe to call with an empty collection. */ - public void dropTables(java.util.Collection tables) { + @SuppressWarnings("PMD.AvoidCatchingGenericException") + public void dropTables(Collection tables) { for (String table : tables) { try { sqlClient.get().run(dialect.dropResultSetTable(table), SqlQueryOptions.ddl()).join(); @@ -347,6 +355,7 @@ private enum Truth { UNKNOWN } + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) private static Truth truth(CqlNode node, Map> materialized) { if (node instanceof InResultSet) { List values = materialized.get(((InResultSet) node).getSetName()); @@ -415,7 +424,7 @@ private static Object coerce(Object value, SchemaBase.Type type) { } /** Records the {@link InResultSet} nodes encountered while traversing a filter. */ - private static class Collector extends CqlVisitorCopy { + private static final class Collector extends CqlVisitorCopy { private final List found = new ArrayList<>(); @Override @@ -435,6 +444,7 @@ private static class ApplyMaterialized extends CqlVisitorCopy { ApplyMaterialized( Map> materialized, Map materializedTables) { + super(); this.materialized = materialized; this.materializedTables = materializedTables; } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlClientBasicFactoryDefault.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlClientBasicFactoryDefault.java index 3f9ef8dd7..31eb4fcbd 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlClientBasicFactoryDefault.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlClientBasicFactoryDefault.java @@ -36,17 +36,14 @@ public SqlClientBasic create( if (!connector.isConnected()) { connectorFactory.disposeConnector(connector); - RuntimeException connectionError = - connector - .getConnectionError() - .map( - throwable -> - throwable instanceof RuntimeException - ? (RuntimeException) throwable - : new RuntimeException(throwable)) - .orElse(new IllegalStateException("unknown reason")); - - throw connectionError; + throw connector + .getConnectionError() + .map( + throwable -> + throwable instanceof RuntimeException + ? (RuntimeException) throwable + : new IllegalStateException(throwable)) + .orElse(new IllegalStateException("unknown reason")); } return new SqlClientBasicDefault( @@ -62,7 +59,7 @@ public void dispose(SqlClientBasic sqlClient) { } } - private static class SqlClientBasicDefault implements SqlClientBasic { + private static final class SqlClientBasicDefault implements SqlClientBasic { private final SqlConnector connector; private final SqlDbmsAdapter dbmsAdapter; private final SqlDialect dialect; diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlClientBasicFactorySimple.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlClientBasicFactorySimple.java index ab6a3510f..09491d95c 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlClientBasicFactorySimple.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlClientBasicFactorySimple.java @@ -46,6 +46,7 @@ public SqlClientBasic create( } @Override + @SuppressWarnings("PMD.CloseResource") public void dispose(SqlClientBasic sqlClient) { if (sqlClient instanceof SqlClientBasicSimple) { for (Connection connection : ((SqlClientBasicSimple) sqlClient).connections) { @@ -66,7 +67,7 @@ public void dispose(SqlClientBasic sqlClient) { } } - private static class SqlClientBasicSimple implements SqlClientBasic { + private static final class SqlClientBasicSimple implements SqlClientBasic { private final DataSource dataSource; private final ConnectionInfoSql connectionInfo; private final SqlDbmsAdapter dbmsAdapter; @@ -93,7 +94,7 @@ public Connection getConnection() { dataSource.getConnection( connectionInfo.getUser().orElse(""), SqlConnectorRx.getPassword(connectionInfo)); } catch (SQLException e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } this.connections.add(connection); diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlInsertGenerator2.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlInsertGenerator2.java index b89e25131..bc3e93f1a 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlInsertGenerator2.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlInsertGenerator2.java @@ -9,7 +9,6 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; -import de.ii.xtraplatform.crs.domain.CrsTransformerFactory; import de.ii.xtraplatform.crs.domain.EpsgCrs; import de.ii.xtraplatform.features.domain.SchemaBase; import de.ii.xtraplatform.features.domain.SchemaBase.Type; @@ -31,24 +30,17 @@ import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * @author zahnen */ public class SqlInsertGenerator2 implements FeatureStoreInsertGenerator { - private static final Logger LOGGER = LoggerFactory.getLogger(SqlInsertGenerator2.class); + private static final String FORMAT_QUALIFIED_COLUMN = "%s.%s"; - private final EpsgCrs nativeCrs; - private final CrsTransformerFactory crsTransformerFactory; private final SqlPathDefaults sqlOptions; - public SqlInsertGenerator2( - EpsgCrs nativeCrs, CrsTransformerFactory crsTransformerFactory, SqlPathDefaults sqlOptions) { - this.nativeCrs = nativeCrs; - this.crsTransformerFactory = crsTransformerFactory; + public SqlInsertGenerator2(SqlPathDefaults sqlOptions) { this.sqlOptions = sqlOptions; } @@ -57,6 +49,7 @@ SqlPathDefaults getSqlOptions() { } @Override + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity", "PMD.NPathComplexity"}) public Supplier>> createInsert( FeatureDataSql feature, SqlQuerySchema schema, @@ -111,27 +104,29 @@ public Supplier>> createInsert( List sortKeys = new ArrayList<>(); - if (parentRelation.isPresent()) { - // TODO: is this merged? - if (schema.isOne2One() - && Objects.equals( - parentRelation.get().getSortKey(), parentRelation.get().getSourceField())) { - // TODO fullPath, sortKey - sortKeys.add( - 0, - String.format( - "%s.%s", parentRelation.get().getName(), parentRelation.get().getSortKey())); - if (!columns2.contains(primaryKey)) { - columns2.add(0, primaryKey); - } - - } else if (schema.isOne2N()) { - sortKeys.add( - 0, - String.format( - "%s.%s", parentRelation.get().getName(), parentRelation.get().getSourceField())); - columns2.add(0, parentRelation.get().getTargetField()); + // TODO: is this merged? + if (parentRelation.isPresent() + && schema.isOne2One() + && Objects.equals( + parentRelation.get().getSortKey(), parentRelation.get().getSourceField())) { + // TODO fullPath, sortKey + sortKeys.add( + 0, + String.format( + FORMAT_QUALIFIED_COLUMN, + parentRelation.get().getName(), + parentRelation.get().getSortKey())); + if (!columns2.contains(primaryKey)) { + columns2.add(0, primaryKey); } + } else if (parentRelation.isPresent() && schema.isOne2N()) { + sortKeys.add( + 0, + String.format( + FORMAT_QUALIFIED_COLUMN, + parentRelation.get().getName(), + parentRelation.get().getSourceField())); + columns2.add(0, parentRelation.get().getTargetField()); } String tableName = schema.getName(); @@ -177,10 +172,10 @@ public Supplier>> createInsert( valueOverrides, schema.getStaticInserts()); - if (!values.isEmpty()) { - values = "VALUES (" + values + ")"; - } else { + if (values.isEmpty()) { values = "DEFAULT VALUES"; + } else { + values = "VALUES (" + values + ")"; } String query = @@ -220,9 +215,11 @@ public Supplier>> createJunctionInsert( String columnNames = String.format("%s,%s", joins.get(0).getTargetField(), joins.get(1).getSourceField()); String sourceIdColumn = - String.format("%s.%s", joins.get(0).getName(), joins.get(0).getSourceField()); + String.format( + FORMAT_QUALIFIED_COLUMN, joins.get(0).getName(), joins.get(0).getSourceField()); String targetIdColumn = - String.format("%s.%s", joins.get(1).getTarget(), joins.get(1).getTargetField()); + String.format( + FORMAT_QUALIFIED_COLUMN, joins.get(1).getTarget(), joins.get(1).getTargetField()); Optional parentRow = feature.getRow(schema.getParentPath(), parentRows.subList(0, 1)); @@ -275,7 +272,9 @@ public Supplier>> createForeignKeyUpdate( String column = joins.get(0).getSourceField(); String columnKey = joins.get(0).getTargetField(); String idColumn = schema.getSortKey(); // TODO: primary key - String idKey = String.format("%s.%s", table, joins.get(0).getSortKey()); // TODO: primary key + String idKey = + String.format( + FORMAT_QUALIFIED_COLUMN, table, joins.get(0).getSortKey()); // TODO: primary key Optional parentRow = feature.getRow(schema.getParentPath(), parentRows.subList(0, 1)); diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlLiterals.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlLiterals.java index c98a74466..99a85709b 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlLiterals.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlLiterals.java @@ -58,7 +58,7 @@ static String integer(String value) { try { return new BigDecimal(value.trim()).toBigIntegerExact().toString(); } catch (NumberFormatException | ArithmeticException e) { - throw new IllegalArgumentException("not a valid integer value: '" + value + "'"); + throw new IllegalArgumentException("not a valid integer value: '" + value + "'", e); } } @@ -68,10 +68,11 @@ static String number(String value) { // SQL dialects. return new BigDecimal(value.trim()).toPlainString(); } catch (NumberFormatException e) { - throw new IllegalArgumentException("not a valid number value: '" + value + "'"); + throw new IllegalArgumentException("not a valid number value: '" + value + "'", e); } } + @SuppressWarnings("PMD.CyclomaticComplexity") static String bool(String value) { String normalized = value.trim().toLowerCase(Locale.ROOT); switch (normalized) { diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMappingDeriver.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMappingDeriver.java index 0c492c4d3..26f21b6bd 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMappingDeriver.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMappingDeriver.java @@ -22,7 +22,6 @@ import de.ii.xtraplatform.features.sql.domain.ImmutableSqlQueryColumn; import de.ii.xtraplatform.features.sql.domain.ImmutableSqlQueryJoin; import de.ii.xtraplatform.features.sql.domain.ImmutableSqlQueryMapping; -import de.ii.xtraplatform.features.sql.domain.ImmutableSqlQuerySchema; import de.ii.xtraplatform.features.sql.domain.ImmutableSqlQuerySchema.Builder; import de.ii.xtraplatform.features.sql.domain.SqlPath; import de.ii.xtraplatform.features.sql.domain.SqlPath.JoinType; @@ -39,6 +38,7 @@ import java.util.Optional; import java.util.stream.Stream; +@SuppressWarnings({"PMD.GodClass", "PMD.CyclomaticComplexity"}) public class SqlMappingDeriver { private final SqlPathParser pathParser; @@ -49,6 +49,12 @@ public SqlMappingDeriver(SqlPathParser pathParser, QueryGeneratorSettings queryG this.queryGeneration = queryGeneration; } + @SuppressWarnings({ + "PMD.AvoidInstantiatingObjectsInLoops", + "PMD.CognitiveComplexity", + "PMD.CyclomaticComplexity", + "PMD.NPathComplexity" + }) public List derive(List mappingRules, FeatureSchema schema) { List schemas = new ArrayList<>(); List> previous = new ArrayList<>(); @@ -145,6 +151,7 @@ public List derive(List mappingRules, FeatureSchem return mappings; } + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity", "PMD.NPathComplexity"}) private void addToMapping( FeatureSchema schema, MappingRule tableRule, @@ -242,6 +249,7 @@ private void addToMapping( } } + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) private void addToMapping( FeatureSchema schema, ImmutableSqlQueryMapping.Builder mapping, @@ -253,32 +261,33 @@ private void addToMapping( SqlQuerySchema querySchema, boolean isWritable) { if ("$".equals(column.getTarget())) { - if (column1.hasOperation(SqlQueryColumn.Operation.CONNECTOR)) { - List connectedSchemas = - includeSchema - ? getConnectedSchemas(schema, column1.getPathSegment(), "", false) - : List.of(); - - for (FeatureSchema p : connectedSchemas) { - if (isWritable) { - mapping.putWritableTables(p.getFullPathAsString(), querySchema); - mapping.putWritableColumns(p.getFullPathAsString(), column1); - seenWritableProperties.add(p.getFullPathAsString()); - } - if (!seenProperties.contains(p.getFullPathAsString())) { - mapping.putValueTables(p.getFullPathAsString(), querySchema); - mapping.putValueColumns(p.getFullPathAsString(), column1); - mapping.putValueSchemas(p.getFullPathAsString(), p); - seenProperties.add(p.getFullPathAsString()); - } + if (!column1.hasOperation(SqlQueryColumn.Operation.CONNECTOR)) { + return; + } + + List connectedSchemas = + includeSchema + ? getConnectedSchemas(schema, column1.getPathSegment(), "", false) + : List.of(); + + for (FeatureSchema p : connectedSchemas) { + if (isWritable) { + mapping.putWritableTables(p.getFullPathAsString(), querySchema); + mapping.putWritableColumns(p.getFullPathAsString(), column1); + seenWritableProperties.add(p.getFullPathAsString()); + } + if (!seenProperties.contains(p.getFullPathAsString())) { + mapping.putValueTables(p.getFullPathAsString(), querySchema); + mapping.putValueColumns(p.getFullPathAsString(), column1); + mapping.putValueSchemas(p.getFullPathAsString(), p); + seenProperties.add(p.getFullPathAsString()); } } } else { String target = column.getTarget(); - FeatureSchema propertySchema = null; if (includeSchema) { - propertySchema = + FeatureSchema propertySchema = schema.getAllNestedProperties().stream() .filter(property -> matches(column, property)) .findFirst() @@ -391,25 +400,22 @@ private SqlQuerySchema derive( List> previous) { SqlPath sqlPath = pathParser.parseFullTablePath(table.getSource()); - ImmutableSqlQuerySchema querySchema = - new Builder() - .name(sqlPath.getName()) - .pathSegment(sqlPath.asPath()) - .sortKey(sqlPath.getSortKey()) - .sortKeyUnique(sqlPath.getSortKeyUnique()) - .primaryKey(sqlPath.getPrimaryKey()) - .filter(sqlPath.getFilter().map(expr -> (Operation) expr)) - .columns(columns.stream().map(column -> getColumn(schema, column)).toList()) - .filterColumns( - filterColumnRules.stream().map(column1 -> getColumn(schema, column1)).toList()) - .writableColumns( - writableColumnRules.stream().map(column1 -> getColumn(schema, column1)).toList()) - .relations(getJoins(sqlPath, previous)) - .staticInserts(sqlPath.getStaticInserts()) - .role(table.getRole()) - .build(); - - return querySchema; + return new Builder() + .name(sqlPath.getName()) + .pathSegment(sqlPath.asPath()) + .sortKey(sqlPath.getSortKey()) + .sortKeyUnique(sqlPath.getSortKeyUnique()) + .primaryKey(sqlPath.getPrimaryKey()) + .filter(sqlPath.getFilter().map(expr -> (Operation) expr)) + .columns(columns.stream().map(column -> getColumn(schema, column)).toList()) + .filterColumns( + filterColumnRules.stream().map(column1 -> getColumn(schema, column1)).toList()) + .writableColumns( + writableColumnRules.stream().map(column1 -> getColumn(schema, column1)).toList()) + .relations(getJoins(sqlPath, previous)) + .staticInserts(sqlPath.getStaticInserts()) + .role(table.getRole()) + .build(); } private static List getJoins(SqlPath path, List> previous) { @@ -492,6 +498,7 @@ private static boolean matches(MappingRule column, FeatureSchema schema) { return false; } + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity", "PMD.NPathComplexity"}) private Map getColumnOperations( MappingRule column, SqlPath sqlPath, Optional propertySchema) { Map operations = new LinkedHashMap<>(); @@ -501,7 +508,7 @@ private Map getColumnOperations( SqlQueryColumn.Operation.CONSTANT, new String[] {sqlPath.getConstantValue().get()}); } - if (sqlPath.getGenerated().isPresent() && sqlPath.getGenerated().get() == false) { + if (sqlPath.getGenerated().isPresent() && !sqlPath.getGenerated().get()) { operations.put(SqlQueryColumn.Operation.DO_NOT_GENERATE, new String[] {}); } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMultiplicityTracker.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMultiplicityTracker.java index ca7a625c8..7912ab3a7 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMultiplicityTracker.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMultiplicityTracker.java @@ -34,7 +34,6 @@ public SqlMultiplicityTracker(List> multiTables) { this.currentIds = new HashMap<>(); this.currentMultiplicities = new HashMap<>(); this.children = new LinkedHashMap<>(); - ; // TODO: test with geoval multiTables.forEach( @@ -63,6 +62,7 @@ public void reset() { } @Override + @SuppressWarnings({"PMD.AvoidInstantiatingObjectsInLoops", "PMD.CognitiveComplexity"}) public void track(List path, List> ids) { int multiplicityIndex = 0; boolean increased = false; @@ -90,7 +90,7 @@ public void track(List path, List> ids) { currentMultiplicities.putIfAbsent(table, 1); } - children.putIfAbsent(table, new HashSet<>()); + children.computeIfAbsent(table, ignored -> new HashSet<>()); if (multiplicityIndex > 0) { parentTables.forEach(parent -> children.get(parent).add(table)); } @@ -106,7 +106,9 @@ public void track(List path, List> ids) { .get(increasedMultiplicityKey) .forEach( child -> { - if (!parentTables.contains(child)) currentMultiplicities.remove(child); + if (!parentTables.contains(child)) { + currentMultiplicities.remove(child); + } }); } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java index e54f7fa8a..b5ff239fd 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlMutationSession.java @@ -55,10 +55,31 @@ * FeatureMutationsSql} to derive insert/delete statements and executes them sequentially on the * underlying {@link SqlSession} so that all mutations participate in one transaction. */ +@SuppressWarnings({ + "PMD.AvoidCatchingGenericException", + "PMD.CouplingBetweenObjects", + "PMD.GodClass", + "PMD.CyclomaticComplexity", + "PMD.TooManyMethods" +}) public class SqlMutationSession implements FeatureTransactions.Session { private static final Logger LOGGER = LoggerFactory.getLogger(SqlMutationSession.class); + private static final String SQL_EQ = " = "; + private static final String SQL_NULL = "NULL"; + private static final String SQL_AND = " AND "; + private static final String SQL_IS_NULL = " IS NULL"; + private static final String SQL_UPDATE = "UPDATE "; + private static final String SQL_SET = " SET "; + private static final String SQL_WHERE = " WHERE "; + private static final String SQL_RETURNING = " RETURNING "; + private static final String SQL_SELECT = "SELECT "; + private static final String SQL_FROM = " FROM "; + private static final String SQL_INSERT_INTO = "INSERT INTO "; + private static final String ERROR_FEATURE_TYPE = "Feature type '"; + private static final String ERROR_IN_COLLECTION = "' in collection '"; + private final SqlSession sqlSession; private final Map> queryMappings; private final FeatureMutationsSql featureMutationsSql; @@ -177,6 +198,7 @@ public FeatureTransactions.MutationResult createFeatures( // the end-col predicate narrows to the just-inserted open version (versioned collections // bind `PRIMARY_INTERVAL_END`; plain collections skip the predicate). Role overrides whose // column IS writable have already landed via the INSERT path and are skipped here. + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) private void applyPostInsertRoleOverrides( SqlQueryMapping mapping, List ids, Map overrides) { if (ids.isEmpty()) { @@ -200,7 +222,7 @@ private void applyPostInsertRoleOverrides( } if (postUpdateTable == null) { postUpdateTable = table; - } else if (postUpdateTable != table) { + } else if (!Objects.equals(postUpdateTable, table)) { // All deferred overrides for a single feature must target the same table (typically the // main table). A future role binding on a sub-table would need a separate UPDATE. continue; @@ -224,22 +246,22 @@ private void applyPostInsertRoleOverrides( } setClause .append(d.first().getName()) - .append(" = ") - .append(d.second() == null ? "NULL" : formatRoleOverrideValue(d.first(), d.second())); + .append(SQL_EQ) + .append(d.second() == null ? SQL_NULL : formatRoleOverrideValue(d.first(), d.second())); } String tableName = postUpdateTable.getName(); String idColName = idColumn.get().second().getName(); String endPredicate = - endColumn.map(t -> " AND " + t.second().getName() + " IS NULL").orElse(""); + endColumn.map(t -> SQL_AND + t.second().getName() + SQL_IS_NULL).orElse(""); for (String id : ids) { String sql = - "UPDATE " + SQL_UPDATE + tableName - + " SET " + + SQL_SET + setClause - + " WHERE " + + SQL_WHERE + idColName - + " = " + + SQL_EQ + sqlString(id) + endPredicate + ";"; @@ -307,6 +329,7 @@ public FeatureTransactions.MutationResult updateFeature( // 409-style conflict. When `expectedStart` is present, the WHERE also requires `startCol = // expectedStart` — an If-Unmodified-Since-style check that the caller maps to a 412 on miss. @Override + @SuppressWarnings("PMD.CyclomaticComplexity") public FeatureTransactions.MutationResult retireFeature( String featureType, String featureId, @@ -324,7 +347,7 @@ public FeatureTransactions.MutationResult retireFeature( return builder .error( new IllegalStateException( - "Feature type '" + ERROR_FEATURE_TYPE + featureType + "' has no PRIMARY_INTERVAL_END role column; cannot retire.")) .build(); @@ -335,7 +358,7 @@ public FeatureTransactions.MutationResult retireFeature( return builder .error( new IllegalStateException( - "Feature type '" + ERROR_FEATURE_TYPE + featureType + "' has no PRIMARY_INTERVAL_START role column; cannot enforce" + " no-backdating during retire.")) @@ -347,7 +370,7 @@ public FeatureTransactions.MutationResult retireFeature( return builder .error( new IllegalStateException( - "Feature type '" + featureType + "' has no id column; cannot retire.")) + ERROR_FEATURE_TYPE + featureType + "' has no id column; cannot retire.")) .build(); } @@ -359,7 +382,7 @@ public FeatureTransactions.MutationResult retireFeature( return builder .error( new IllegalStateException( - "Feature type '" + ERROR_FEATURE_TYPE + featureType + "' has id / PRIMARY_INTERVAL_START / PRIMARY_INTERVAL_END on more" + " than one table; retirement requires all three on the main table.")) @@ -376,7 +399,7 @@ public FeatureTransactions.MutationResult retireFeature( // role to a column on the main table, set it to the retirement timestamp — which is also // the new version's start in retire-and-insert flows. Opt-in: no SUCCESSOR_INTERVAL_START // role on the schema means no SET clause is added. - StringBuilder setClause = new StringBuilder(endColumnName).append(" = ").append(tsLiteral); + StringBuilder setClause = new StringBuilder(endColumnName).append(SQL_EQ).append(tsLiteral); Optional> successorColumn = mapping.getColumnForRole(SchemaBase.Role.SUCCESSOR_INTERVAL_START); if (successorColumn.isPresent() @@ -384,7 +407,7 @@ public FeatureTransactions.MutationResult retireFeature( setClause .append(", ") .append(successorColumn.get().second().getName()) - .append(" = ") + .append(SQL_EQ) .append(tsLiteral); } @@ -396,9 +419,9 @@ public FeatureTransactions.MutationResult retireFeature( // 412 on miss (composite-id convention). StringBuilder where = new StringBuilder(idColumnName) - .append(" = ") + .append(SQL_EQ) .append(sqlString(featureId)) - .append(" AND ") + .append(SQL_AND) .append(endColumnName) .append(" IS NULL AND ") .append(startColumnName) @@ -406,19 +429,19 @@ public FeatureTransactions.MutationResult retireFeature( .append(tsLiteral); if (expectedStart.isPresent()) { where - .append(" AND ") + .append(SQL_AND) .append(startColumnName) - .append(" = ") + .append(SQL_EQ) .append(sqlString(expectedStart.get().toString())); } String sql = - "UPDATE " + SQL_UPDATE + mainTableName - + " SET " + + SQL_SET + setClause - + " WHERE " + + SQL_WHERE + where - + " RETURNING " + + SQL_RETURNING + idColumnName + ";"; try { @@ -451,7 +474,7 @@ public FeatureTransactions.MutationResult assertNoConflictingVersion( return builder .error( new IllegalStateException( - "Feature type '" + ERROR_FEATURE_TYPE + featureType + "' has no id column; cannot run the versioned-insert pre-flight.")) .build(); @@ -461,9 +484,9 @@ public FeatureTransactions.MutationResult assertNoConflictingVersion( String sql = "SELECT 1 FROM " + mainTableName - + " WHERE " + + SQL_WHERE + idCol - + " = " + + SQL_EQ + sqlString(featureId) + " LIMIT 1;"; try { @@ -474,7 +497,7 @@ public FeatureTransactions.MutationResult assertNoConflictingVersion( new IllegalArgumentException( "Cannot create feature id '" + featureId - + "' in collection '" + + ERROR_IN_COLLECTION + featureType + "': a version of this feature already exists (use Replace or Update to" + " add a new version).")) @@ -509,15 +532,15 @@ public Optional getOpenVersionStart(String featureType, String featureId return Optional.empty(); } String sql = - "SELECT " + SQL_SELECT + startColumn.get().second().getName() - + " FROM " + + SQL_FROM + mainTableName - + " WHERE " + + SQL_WHERE + idColumn.get().second().getName() - + " = " + + SQL_EQ + sqlString(featureId) - + " AND " + + SQL_AND + endColumn.get().second().getName() + " IS NULL LIMIT 1;"; try { @@ -599,13 +622,13 @@ public FeatureTransactions.MutationResult patchOpenVersion( return builder .error( new IllegalStateException( - "Feature type '" + ERROR_FEATURE_TYPE + featureType + "' has no PRIMARY_INTERVAL_END role column; cannot patch open version.")) .build(); } String endColumnName = endColumn.get().second().getName(); - String extra = " AND " + endColumnName + " IS NULL"; + StringBuilder extra = new StringBuilder(SQL_AND).append(endColumnName).append(SQL_IS_NULL); Optional> startColumn = mapping.getColumnForRole(SchemaBase.Role.PRIMARY_INTERVAL_START); @@ -621,7 +644,7 @@ public FeatureTransactions.MutationResult patchOpenVersion( if (resolved.isPresent() && Objects.equals(resolved.get().second().getName(), endColumnName)) { String endLiteral = encodeLiteral(resolved.get().second(), u.getValue(), crs); - extra = extra + " AND " + startColumnName + " < " + endLiteral; + extra.append(SQL_AND).append(startColumnName).append(" < ").append(endLiteral); break; } } @@ -629,12 +652,16 @@ public FeatureTransactions.MutationResult patchOpenVersion( // equal the value the client encoded in the rid's suffix. Otherwise the UPDATE matches 0 // rows and the caller maps that to a 412 Precondition Failed. if (expectedStart.isPresent()) { - extra = - extra + " AND " + startColumnName + " = " + sqlString(expectedStart.get().toString()); + extra + .append(SQL_AND) + .append(startColumnName) + .append(SQL_EQ) + .append(sqlString(expectedStart.get().toString())); } } - return patchInternal(featureType, featureId, updates, crs, extra, "open version of feature"); + return patchInternal( + featureType, featureId, updates, crs, extra.toString(), "open version of feature"); } // Versioned Update CLONE_AND_PATCH: create a new version of the open row, carry @@ -652,6 +679,7 @@ public FeatureTransactions.MutationResult patchOpenVersion( // DELETE+INSERT junction path. // An empty result on step 1 → caller maps to 409 (or 412 when `expectedStart` was present). @Override + @SuppressWarnings({"PMD.NcssCount", "PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) public FeatureTransactions.MutationResult cloneAndPatchFeature( String featureType, String featureId, @@ -675,7 +703,7 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( return builder .error( new IllegalStateException( - "Feature type '" + ERROR_FEATURE_TYPE + featureType + "' is missing ID / PRIMARY_INTERVAL_START / PRIMARY_INTERVAL_END role" + " columns; cannot clone-and-patch.")) @@ -688,7 +716,7 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( return builder .error( new IllegalStateException( - "Feature type '" + ERROR_FEATURE_TYPE + featureType + "' has id / PRIMARY_INTERVAL_START / PRIMARY_INTERVAL_END on more than" + " one table; clone-and-patch requires all three on the main table.")) @@ -713,22 +741,23 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( // Step 1: capture the open row's surrogate PK (and reject early if no open row matches). StringBuilder findPk = - new StringBuilder("SELECT ") + new StringBuilder(128) + .append(SQL_SELECT) .append(pkColumnName) - .append(" FROM ") + .append(SQL_FROM) .append(mainTableName) - .append(" WHERE ") + .append(SQL_WHERE) .append(idColumnName) - .append(" = ") + .append(SQL_EQ) .append(sqlString(featureId)) - .append(" AND ") + .append(SQL_AND) .append(endColumnName) - .append(" IS NULL"); + .append(SQL_IS_NULL); if (expectedStart.isPresent()) { findPk - .append(" AND ") + .append(SQL_AND) .append(startColumnName) - .append(" = ") + .append(SQL_EQ) .append(sqlString(expectedStart.get().toString())); } findPk.append(" LIMIT 1;"); @@ -751,13 +780,13 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( try { List startRows = sqlSession.runReturning( - "SELECT " + SQL_SELECT + startColumnName - + " FROM " + + SQL_FROM + mainTableName - + " WHERE " + + SQL_WHERE + pkColumnName - + " = " + + SQL_EQ + oldPkLit + ";"); if (!startRows.isEmpty()) { @@ -798,16 +827,16 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( continue; } if (role.get() == SchemaBase.Role.PRIMARY_INTERVAL_END) { - selectExprs.add("NULL"); + selectExprs.add(SQL_NULL); continue; } if (role.get() == SchemaBase.Role.PREDECESSOR_INTERVAL_START) { - selectExprs.add(oldStart.map(SqlMutationSession::sqlString).orElse("NULL")); + selectExprs.add(oldStart.map(SqlMutationSession::sqlString).orElse(SQL_NULL)); continue; } if (role.get() == SchemaBase.Role.SUCCESSOR_INTERVAL_START) { // The new row is open — no successor yet. - selectExprs.add("NULL"); + selectExprs.add(SQL_NULL); continue; } } @@ -815,19 +844,19 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( selectExprs.add(patchLit != null ? patchLit : "m." + name); } String cloneMainSql = - "INSERT INTO " + SQL_INSERT_INTO + mainTableName + " (" + String.join(", ", insertCols) + ") SELECT " + String.join(", ", selectExprs) - + " FROM " + + SQL_FROM + mainTableName + " m WHERE m." + pkColumnName - + " = " + + SQL_EQ + oldPkLit - + " RETURNING " + + SQL_RETURNING + pkColumnName + ";"; List newPkRows; @@ -842,7 +871,7 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( new IllegalStateException( "Clone-and-patch of feature id '" + featureId - + "' in collection '" + + ERROR_IN_COLLECTION + featureType + "' did not return a new row PK; clone INSERT must have inserted 0 rows.")) .build(); @@ -868,30 +897,30 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( } // Step 4: retire OLD. Same guard as retireFeature — no-backdating + must be the open row. - StringBuilder retireSet = new StringBuilder(endColumnName).append(" = ").append(tsLiteral); + StringBuilder retireSet = new StringBuilder(endColumnName).append(SQL_EQ).append(tsLiteral); if (successorOnMain) { retireSet .append(", ") .append(successorColumn.get().second().getName()) - .append(" = ") + .append(SQL_EQ) .append(tsLiteral); } String retireSql = - "UPDATE " + SQL_UPDATE + mainTableName - + " SET " + + SQL_SET + retireSet - + " WHERE " + + SQL_WHERE + pkColumnName - + " = " + + SQL_EQ + oldPkLit - + " AND " + + SQL_AND + endColumnName + " IS NULL AND " + startColumnName + " < " + tsLiteral - + " RETURNING " + + SQL_RETURNING + pkColumnName + ";"; List retiredRows; @@ -906,7 +935,7 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( new IllegalStateException( "Clone-and-patch of feature id '" + featureId - + "' in collection '" + + ERROR_IN_COLLECTION + featureType + "': failed to retire the previous open version (concurrent modification or" + " no-backdating violation).")) @@ -935,7 +964,7 @@ public FeatureTransactions.MutationResult cloneAndPatchFeature( featureId, junctionUpdates, crs, - " AND " + endColumnName + " IS NULL", + SQL_AND + endColumnName + SQL_IS_NULL, "open version of feature"); if (patchResult.getError().isPresent()) { return builder.error(patchResult.getError().get()).build(); @@ -982,17 +1011,17 @@ private void cloneJunctionRows(SqlQuerySchema junction, String oldPkLit, String return; } String sql = - "INSERT INTO " + SQL_INSERT_INTO + junction.getName() + " (" + String.join(", ", insertCols) + ") SELECT " + String.join(", ", selectExprs) - + " FROM " + + SQL_FROM + junction.getName() - + " WHERE " + + SQL_WHERE + fkColumn - + " = " + + SQL_EQ + oldPkLit + ";"; sqlSession.runReturning(sql); @@ -1003,7 +1032,7 @@ private void cloneJunctionRows(SqlQuerySchema junction, String oldPkLit, String // anything that isn't a plain integer. private static String sqlLiteralForPk(String raw) { if (raw == null) { - return "NULL"; + return SQL_NULL; } try { Long.parseLong(raw); @@ -1013,6 +1042,11 @@ private static String sqlLiteralForPk(String raw) { } } + @SuppressWarnings({ + "PMD.AvoidInstantiatingObjectsInLoops", + "PMD.CognitiveComplexity", + "PMD.CyclomaticComplexity" + }) private FeatureTransactions.MutationResult patchInternal( String featureType, String featureId, @@ -1035,7 +1069,7 @@ private FeatureTransactions.MutationResult patchInternal( return builder .error( new IllegalStateException( - "Feature type '" + featureType + "' has no id column; cannot patch in place.")) + ERROR_FEATURE_TYPE + featureType + "' has no id column; cannot patch in place.")) .build(); } SqlQuerySchema mainTable = mapping.getMainTable(); @@ -1045,8 +1079,7 @@ private FeatureTransactions.MutationResult patchInternal( List setClauses = new ArrayList<>(); // Junction-backed updates: ordered by first-touch path so deterministic SQL ordering. - java.util.LinkedHashMap junctionPatches = - new java.util.LinkedHashMap<>(); + Map junctionPatches = new java.util.LinkedHashMap<>(); for (FeatureTransactions.PropertyUpdate update : updates) { String joined = String.join(".", update.getPath()); @@ -1060,7 +1093,7 @@ private FeatureTransactions.MutationResult patchInternal( SqlQueryColumn column = resolved.get().second(); if (Objects.equals(table.getName(), mainTableName)) { String literal = encodeLiteral(column, update.getValue(), crs); - setClauses.add(column.getName() + " = " + literal); + setClauses.add(column.getName() + SQL_EQ + literal); } else if (table.isOne2N()) { JunctionPatch patch = junctionPatches.computeIfAbsent( @@ -1110,16 +1143,16 @@ private FeatureTransactions.MutationResult patchInternal( try { if (!setClauses.isEmpty()) { String sql = - "UPDATE " + SQL_UPDATE + mainTableName - + " SET " + + SQL_SET + String.join(", ", setClauses) - + " WHERE " + + SQL_WHERE + idColumnName - + " = " + + SQL_EQ + idLiteral + extraWherePredicate - + " RETURNING " + + SQL_RETURNING + idColumnName + ";"; List returned = sqlSession.runReturning(sql); @@ -1131,7 +1164,7 @@ private FeatureTransactions.MutationResult patchInternal( + missingTargetLabel + " with id '" + featureId - + "' in collection '" + + ERROR_IN_COLLECTION + featureType + "'.")) .build(); @@ -1149,13 +1182,13 @@ private FeatureTransactions.MutationResult patchInternal( if (setClauses.isEmpty() && !junctionPatches.isEmpty()) { List exists = sqlSession.runReturning( - "SELECT " + SQL_SELECT + idColumnName - + " FROM " + + SQL_FROM + mainTableName - + " WHERE " + + SQL_WHERE + idColumnName - + " = " + + SQL_EQ + idLiteral + extraWherePredicate + ";"); @@ -1167,7 +1200,7 @@ private FeatureTransactions.MutationResult patchInternal( + missingTargetLabel + " with id '" + featureId - + "' in collection '" + + ERROR_IN_COLLECTION + featureType + "'.")) .build(); @@ -1180,6 +1213,7 @@ private FeatureTransactions.MutationResult patchInternal( return builder.build(); } + @SuppressWarnings({"PMD.AvoidInstantiatingObjectsInLoops", "PMD.CyclomaticComplexity"}) private void runJunctionPatch( JunctionPatch patch, String mainTableName, @@ -1199,15 +1233,15 @@ private void runJunctionPatch( String deleteSql = "DELETE FROM " + junctionTable - + " WHERE " + + SQL_WHERE + junctionFk + " IN (SELECT " + parentPk - + " FROM " + + SQL_FROM + mainTableName - + " WHERE " + + SQL_WHERE + idColumnName - + " = " + + SQL_EQ + idLiteral + extraWherePredicate + ");"; @@ -1222,14 +1256,16 @@ private void runJunctionPatch( String valueCol = patch.valueColumn.getName(); StringBuilder valuesList = new StringBuilder(); for (int i = 0; i < patch.values.size(); i++) { - if (i > 0) valuesList.append(", "); + if (i > 0) { + valuesList.append(", "); + } valuesList - .append("(") + .append('(') .append(encodeLiteral(patch.valueColumn, Optional.of(patch.values.get(i)), crs)) - .append(")"); + .append(')'); } String insertSql = - "INSERT INTO " + SQL_INSERT_INTO + junctionTable + " (" + junctionFk @@ -1243,7 +1279,7 @@ private void runJunctionPatch( + valuesList + ") AS v(val) WHERE m." + idColumnName - + " = " + + SQL_EQ + idLiteral + qualifyAliasPredicate(extraWherePredicate, "m") + ";"; @@ -1269,22 +1305,23 @@ private void runJunctionPatch( StringBuilder selectLits = new StringBuilder("m.").append(parentPk); for (String childKey : childKeys) { com.fasterxml.jackson.databind.JsonNode v = element.get(childKey); - selectLits.append(", "); - selectLits.append( - encodeLiteral(patch.objectChildColumns.get(childKey), Optional.ofNullable(v), crs)); + selectLits + .append(", ") + .append( + encodeLiteral(patch.objectChildColumns.get(childKey), Optional.ofNullable(v), crs)); } String insertSql = - "INSERT INTO " + SQL_INSERT_INTO + junctionTable + " (" + cols + ") SELECT " + selectLits - + " FROM " + + SQL_FROM + mainTableName + " m WHERE m." + idColumnName - + " = " + + SQL_EQ + idLiteral + qualifyAliasPredicate(extraWherePredicate, "m") + ";"; @@ -1296,12 +1333,16 @@ private void runJunctionPatch( // references must be qualified with that alias. Single-pass replace works because the predicate // is generated by us (`" AND IS NULL"`); not robust against arbitrary user input. private static String qualifyAliasPredicate(String extraPredicate, String alias) { - if (extraPredicate.isEmpty()) return extraPredicate; + if (extraPredicate.isEmpty()) { + return extraPredicate; + } // Strip leading " AND " and prefix every word-start with the alias. int and = extraPredicate.indexOf("AND "); - if (and < 0) return extraPredicate; + if (and < 0) { + return extraPredicate; + } String rest = extraPredicate.substring(and + 4); - return " AND " + alias + "." + rest; + return SQL_AND + alias + "." + rest; } // Patch state for a junction-backed property. Two modes are encoded in the same record so the @@ -1312,14 +1353,14 @@ private static String qualifyAliasPredicate(String extraPredicate, String alias) private static final class JunctionPatch { final SqlQuerySchema junction; final SqlQueryColumn valueColumn; - final java.util.LinkedHashMap objectChildColumns; + final Map objectChildColumns; final String objectPath; final List values = new ArrayList<>(); private JunctionPatch( SqlQuerySchema junction, SqlQueryColumn valueColumn, - java.util.LinkedHashMap objectChildColumns, + Map objectChildColumns, String objectPath) { this.junction = junction; this.valueColumn = valueColumn; @@ -1333,7 +1374,7 @@ static JunctionPatch valueArray(SqlQuerySchema junction, SqlQueryColumn valueCol static JunctionPatch objectArray( SqlQuerySchema junction, FeatureSchema objectSchema, SqlQueryMapping mapping, String path) { - java.util.LinkedHashMap cols = new java.util.LinkedHashMap<>(); + Map cols = new java.util.LinkedHashMap<>(); for (FeatureSchema child : objectSchema.getProperties()) { if (child.getType() == SchemaBase.Type.OBJECT || child.getType() == SchemaBase.Type.OBJECT_ARRAY) { @@ -1408,7 +1449,7 @@ private String encodeLiteral( Optional valueOpt, EpsgCrs crs) { if (valueOpt.isEmpty() || valueOpt.get().isNull()) { - return "NULL"; + return SQL_NULL; } com.fasterxml.jackson.databind.JsonNode value = valueOpt.get(); if (column.hasOperation(SqlQueryColumn.Operation.WKT) @@ -1420,6 +1461,7 @@ private String encodeLiteral( return SqlLiterals.forType(column.getType(), value.asText()); } + @SuppressWarnings("PMD.CyclomaticComplexity") private String encodeGeometryLiteral( SqlQueryColumn column, com.fasterxml.jackson.databind.JsonNode value, EpsgCrs crs) { if (!value.isObject()) { @@ -1510,7 +1552,7 @@ private static FeatureSchema resolveSchemaByPath(FeatureSchema root, List (FeatureDataSql) feature)); + .via(Transformer.map(feature -> feature)); if (partial) { featureSqlSource = @@ -1698,6 +1740,7 @@ private void writeFeaturesPerFeature( // consumer (so child statements can read them from currentRow.ids), then send all children // across all features through a single sqlSession.run — letting the existing JDBC batch path // collapse the entire tail into one executeBatch. + @SuppressWarnings("PMD.NullAssignment") private void writeFeaturesBatched( List collected, RowCursor rowCursor, @@ -1748,8 +1791,9 @@ private void writeFeaturesBatched( // sqlSession.runReturning so we receive every generated PK in insertion order. Features whose // main SQL doesn't fit the shape (e.g. DEFAULT VALUES, or a different table) are flushed as a // single-row insert via the same path; null-SQL slots are skipped entirely. + @SuppressWarnings("PMD.AvoidInstantiatingObjectsInLoops") private void runMainInsertsGrouped( - String[] mainSqls, Consumer[] consumersRaw, String[] returnedPks) { + String[] mainSqls, Consumer[] consumersRaw, String... returnedPks) { int n = mainSqls.length; int i = 0; while (i < n) { @@ -1760,7 +1804,7 @@ private void runMainInsertsGrouped( MainInsertParts head = splitMainInsert(mainSqls[i]); if (head == null) { // SQL doesn't match the standard shape — execute on its own. - executeAndDispatch(mainSqls[i], consumersRaw, returnedPks, new int[] {i}); + executeAndDispatch(mainSqls[i], consumersRaw, returnedPks, i); i++; continue; } @@ -1790,7 +1834,7 @@ private void runMainInsertsGrouped( @SuppressWarnings("unchecked") private void executeAndDispatch( - String sql, Consumer[] consumersRaw, String[] returnedPks, int[] featureIndices) { + String sql, Consumer[] consumersRaw, String[] returnedPks, int... featureIndices) { List ids = sqlSession.runReturning(sql); for (int k = 0; k < featureIndices.length; k++) { String returned = k < ids.size() ? ids.get(k) : null; @@ -1827,11 +1871,10 @@ private void runChildren(List>>>> c // " RETURNING "). Returns null when the SQL doesn't match — e.g. DEFAULT VALUES, or some other // shape — so the caller can fall back to a single-row execution. private static MainInsertParts splitMainInsert(String sql) { - int retIdx = sql.lastIndexOf(" RETURNING "); + int retIdx = sql.lastIndexOf(SQL_RETURNING); if (retIdx < 0 || !sql.endsWith(";")) { return null; } - String suffix = sql.substring(retIdx); String body = sql.substring(0, retIdx); int valIdx = body.lastIndexOf(" VALUES ("); if (valIdx < 0 || !body.endsWith(")")) { @@ -1840,6 +1883,7 @@ private static MainInsertParts splitMainInsert(String sql) { int prefixEnd = valIdx + " VALUES ".length(); String prefix = sql.substring(0, prefixEnd); String values = body.substring(prefixEnd); + String suffix = sql.substring(retIdx); return new MainInsertParts(prefix, values, suffix); } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlQueryColumnOperations.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlQueryColumnOperations.java index 946e8e4df..447626c92 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlQueryColumnOperations.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlQueryColumnOperations.java @@ -13,8 +13,10 @@ import de.ii.xtraplatform.features.sql.domain.SqlDialect; import de.ii.xtraplatform.features.sql.domain.SqlQueryColumn; import de.ii.xtraplatform.features.sql.domain.SqlQueryColumn.Operation; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.IntStream; public interface SqlQueryColumnOperations { @@ -31,6 +33,7 @@ static String getQualifiedColumnResolved( return getQualifiedColumnResolved(tableAlias, column, sqlDialect, Set.of(), false); } + @SuppressWarnings("PMD.CyclomaticComplexity") static String getQualifiedColumnResolved( String tableAlias, SqlQueryColumn column, @@ -47,13 +50,13 @@ static String getQualifiedColumnResolved( } if (ops.containsKey(Operation.EXPRESSION) && !excludeOperations.contains(Operation.EXPRESSION)) { - final int[] i = {0}; + List expressionParams = column.getOperationParameters(Operation.EXPRESSION); return sqlDialect.applyToExpression( tableAlias, column.getName(), - column.getOperationParameters(Operation.EXPRESSION).stream() - .map(param -> Map.entry("" + i[0]++, param)) - .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)), + IntStream.range(0, expressionParams.size()) + .boxed() + .collect(ImmutableMap.toImmutableMap(String::valueOf, expressionParams::get)), ops.containsKey(Operation.WKT)); } if (ops.containsKey(Operation.WKT) && !excludeOperations.contains(Operation.WKT)) { diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlQueryTemplatesDeriver.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlQueryTemplatesDeriver.java index 9e81376a6..fbfa9b251 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlQueryTemplatesDeriver.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/app/SqlQueryTemplatesDeriver.java @@ -35,6 +35,7 @@ import java.util.stream.IntStream; import java.util.stream.Stream; +@SuppressWarnings({"PMD.CouplingBetweenObjects", "PMD.GodClass"}) public class SqlQueryTemplatesDeriver { private static final String SKEY = "SKEY"; @@ -87,6 +88,7 @@ public SqlQueryTemplates derive(SqlQueryMapping mapping) { .build(); } + @SuppressWarnings("PMD.CognitiveComplexity") MetaQueryTemplate createMetaQueryTemplate(SqlQuerySchema schema, SqlQueryMapping mapping) { return (limit, offset, @@ -109,14 +111,14 @@ MetaQueryTemplate createMetaQueryTemplate(SqlQuerySchema schema, SqlQueryMapping ? virtualTables.get(schema.getName()) : schema.getName(); String table = String.format("%s A", tableName); - String columns = ""; + StringBuilder columns = new StringBuilder(); for (int i = 0; i < additionalSortKeys.size(); i++) { SortKey sortKey = additionalSortKeys.get(i); - columns += getSortColumn("A", sortKey, i) + ", "; + columns.append(getSortColumn("A", sortKey, i)).append(", "); } - columns += getSkeyColumn("A", schema, "", false); + columns.append(getSkeyColumn("A", schema, "", false)); String orderBy = getOrderBy(additionalSortKeys); String minMaxColumns = getMinMaxColumns(additionalSortKeys); @@ -124,7 +126,13 @@ MetaQueryTemplate createMetaQueryTemplate(SqlQuerySchema schema, SqlQueryMapping withNumberReturned ? String.format( "SELECT %6$s, count(*) AS numberReturned FROM (SELECT %2$s FROM %1$s%5$s ORDER BY %3$s%4$s)%7$s", - table, columns, orderBy, limitAndOffsetSql, where, minMaxColumns, asIds) + table, + columns.toString(), + orderBy, + limitAndOffsetSql, + where, + minMaxColumns, + asIds) : sqlDialect.applyToNoTable( String.format( "SELECT NULL AS minKey, NULL AS maxKey, %s AS numberReturned", @@ -232,7 +240,7 @@ private String getTableQuery( String paging = pagingClause.filter(p -> join.isEmpty()).orElse(""); if (!join.isEmpty() && pagingClause.isPresent()) { - String where2 = " WHERE "; + StringBuilder where2 = new StringBuilder(" WHERE "); List aliasesNested = AliasGenerator.getAliases(schema, where.isEmpty() ? 1 : 2); String orderBy = IntStream.range(0, sortFields.size()) @@ -249,7 +257,7 @@ private String getTableQuery( .map(sortField -> sortField.replace("A.", aliasesNested.get(0) + ".")) .map(sortField -> sortField.replaceAll(" AS \\w+", "")) .collect(Collectors.joining(",")); - where2 += + where2.append( String.format( "(A.%3$s IN (SELECT %2$s.%3$s FROM %1$s %2$s%4$s ORDER BY %5$s%6$s))", mainTableName, @@ -259,9 +267,9 @@ private String getTableQuery( .replace("(A.", "(" + aliasesNested.get(0) + ".") .replace(" A.", " " + aliasesNested.get(0) + "."), orderBy, - pagingClause.get()); + pagingClause.get())); - where = where2; + where = where2.toString(); } String orderBy = @@ -288,16 +296,10 @@ private String getTableQuery( * as constrained by the id list and let the SQL generator skip the surrogate-key range guard. */ private static boolean containsIdFilter(Cql2Expression expr) { - if (expr instanceof In && ((In) expr).isIdFilter()) { - return true; - } - if (expr instanceof And) { - return ((And) expr) - .getArgs().stream() - .anyMatch( - arg -> arg instanceof Cql2Expression && containsIdFilter((Cql2Expression) arg)); - } - return false; + return (expr instanceof In && ((In) expr).isIdFilter()) + || (expr instanceof And + && ((And) expr) + .getArgs().stream().anyMatch(SqlQueryTemplatesDeriver::containsIdFilter)); } private Optional toWhereClause( @@ -309,16 +311,16 @@ private Optional toWhereClause( StringBuilder filter = new StringBuilder(); if (minMaxKeys.isPresent() && additionalSortKeys.isEmpty()) { - filter.append("("); + filter.append('('); addMinMaxFilter(filter, alias, keyField, minMaxKeys.get().first(), minMaxKeys.get().second()); - filter.append(")"); + filter.append(')'); } if (additionalFilter.isPresent()) { if (minMaxKeys.isPresent() && additionalSortKeys.isEmpty()) { filter.append(" AND "); } - filter.append("(").append(additionalFilter.get()).append(")"); + filter.append('(').append(additionalFilter.get()).append(')'); } if (filter.length() == 0) { @@ -332,13 +334,13 @@ private StringBuilder addMinMaxFilter( StringBuilder whereClause, String alias, String keyField, Object minKey, Object maxKey) { return whereClause .append(alias) - .append(".") + .append('.') .append(keyField) .append(" >= ") .append(formatLiteral(minKey)) .append(" AND ") .append(alias) - .append(".") + .append('.') .append(keyField) .append(" <= ") .append(formatLiteral(maxKey)); @@ -382,27 +384,27 @@ private String getSkeyColumn( private List getSortFields( SqlQuerySchema schema, List aliases, List additionalSortKeys) { - final int[] i = {0}; Stream customSortKeys = - additionalSortKeys.stream().map(sortKey -> getSortColumn(aliases.get(0), sortKey, i[0]++)); - - if (!schema.getRelations().isEmpty()) { - ListIterator aliasesIterator = aliases.listIterator(); + IntStream.range(0, additionalSortKeys.size()) + .mapToObj(i -> getSortColumn(aliases.get(0), additionalSortKeys.get(i), i)); - List parentSortKeys = List.of(); - - return Stream.of( - customSortKeys, - parentSortKeys.stream(), - getSortKeys(schema.asTablePath(), aliasesIterator, false, parentSortKeys.size()) - .stream()) - .flatMap(s -> s) - .collect(Collectors.toList()); - } else { + if (schema.getRelations().isEmpty()) { return Stream.concat( customSortKeys, Stream.of(getSkeyColumn(aliases.get(0), schema, "", true))) .collect(Collectors.toList()); } + + ListIterator aliasesIterator = aliases.listIterator(); + + List parentSortKeys = List.of(); + + return Stream.of( + customSortKeys, + parentSortKeys.stream(), + getSortKeys(schema.asTablePath(), aliasesIterator, false, parentSortKeys.size()) + .stream()) + .flatMap(s -> s) + .collect(Collectors.toList()); } private Optional getFilter( @@ -427,37 +429,40 @@ private Optional getFilter( } private String getOrderBy(List sortKeys) { - String orderBy = ""; + StringBuilder orderBy = new StringBuilder(32); for (int i = 0; i < sortKeys.size(); i++) { SortKey sortKey = sortKeys.get(i); - orderBy += - CSKEY - + "_" - + i - + (sortKey.getDirection() == Direction.DESCENDING ? " DESC" : "") - + nullOrder - + ", "; + orderBy + .append(CSKEY) + .append('_') + .append(i) + .append(sortKey.getDirection() == Direction.DESCENDING ? " DESC" : "") + .append(nullOrder) + .append(", "); } - orderBy += SKEY; + orderBy.append(SKEY); - return orderBy; + return orderBy.toString(); } private String getMinMaxColumns(List sortKeys) { - String minMaxKeys = ""; - - if (!sortKeys.isEmpty()) { - minMaxKeys += "NULL AS minKey, "; - minMaxKeys += "NULL AS maxKey"; + StringBuilder minMaxKeys = new StringBuilder(48); + + if (sortKeys.isEmpty()) { + minMaxKeys + .append("MIN(") + .append(SKEY) + .append(") AS minKey, MAX(") + .append(SKEY) + .append(") AS maxKey"); } else { - minMaxKeys += "MIN(" + SKEY + ") AS minKey, "; - minMaxKeys += "MAX(" + SKEY + ") AS maxKey"; + minMaxKeys.append("NULL AS minKey, NULL AS maxKey"); } - return minMaxKeys; + return minMaxKeys.toString(); } private List getSortKeys( diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSql.java index bbf7e6893..e0aee5425 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSql.java @@ -23,6 +23,7 @@ import de.ii.xtraplatform.cql.domain.CustomFunction; import de.ii.xtraplatform.crs.domain.BoundingBox; import de.ii.xtraplatform.crs.domain.CrsInfo; +import de.ii.xtraplatform.crs.domain.CrsTransformationException; import de.ii.xtraplatform.crs.domain.CrsTransformerFactory; import de.ii.xtraplatform.crs.domain.EpsgCrs; import de.ii.xtraplatform.crs.domain.OgcCrs; @@ -47,7 +48,6 @@ import de.ii.xtraplatform.features.domain.FeatureExtents; import de.ii.xtraplatform.features.domain.FeatureProvider; import de.ii.xtraplatform.features.domain.FeatureProviderDataV2; -import de.ii.xtraplatform.features.domain.FeatureQueries; import de.ii.xtraplatform.features.domain.FeatureQuery; import de.ii.xtraplatform.features.domain.FeatureQueryEncoder; import de.ii.xtraplatform.features.domain.FeatureSchema; @@ -445,11 +445,15 @@ value = FeatureProviderSql.PROVIDER_SUB_TYPE) }, data = FeatureProviderSqlData.class) +@SuppressWarnings({ + "PMD.CouplingBetweenObjects", + "PMD.GodClass", + "PMD.CyclomaticComplexity", + "PMD.TooManyMethods" +}) public class FeatureProviderSql extends AbstractFeatureProvider - implements FeatureProvider, - FeatureQueries, - FeatureExtents, + implements FeatureExtents, FeatureCrs, FeatureTransactions, MultiFeatureQueries, @@ -459,12 +463,13 @@ public class FeatureProviderSql public static final String ENTITY_SUB_TYPE = "feature/sql"; public static final String PROVIDER_SUB_TYPE = "SQL"; + private static final String CACHE_KEY_STATS = "stats"; private final Cql cql; private final SqlDbmsAdapters dbmsAdapters; private final Map subdecoders; - private final de.ii.xtraplatform.cache.domain.Cache cache; + private final Cache cache; private final Scheduler scheduler; private FeatureQueryEncoderSql queryTransformer; @@ -514,6 +519,11 @@ public FeatureProviderSql( decoderFactories.getConnectorDecoders()); } + @SuppressWarnings({ + "PMD.ExcessiveParameterList", + "PMD.ConstructorCallsOverridableMethod", + "PMD.NullAssignment" + }) protected FeatureProviderSql( CrsTransformerFactory crsTransformerFactory, CrsInfo crsInfo, @@ -551,9 +561,9 @@ protected FeatureProviderSql( this.cql2Functions = List.of(); } - private static PathParserSql createPathParser2(SqlPathDefaults sqlPathDefaults, Cql cql) { + private static PathParserSql createPathParser2(SqlPathDefaults sqlPathDefaults) { SqlPathSyntax syntax = ImmutableSqlPathSyntax.builder().options(sqlPathDefaults).build(); - return new PathParserSql(syntax, cql); + return new PathParserSql(syntax); } private static SqlPathParser createPathParser3( @@ -621,17 +631,15 @@ protected boolean onStartup() throws InterruptedException { Map> allQueryTemplates = queryMappings.entrySet().stream() .map( - entry -> { - final int[] i = {0}; - return new SimpleImmutableEntry<>( - entry.getKey(), - entry.getValue().stream().map(sqlQueryTemplatesDeriver::derive).toList()); - }) + entry -> + new SimpleImmutableEntry<>( + entry.getKey(), + entry.getValue().stream().map(sqlQueryTemplatesDeriver::derive).toList())) .collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue)); this.queryTransformer = new FeatureQueryEncoderSql( - allQueryTemplates, allQueryTemplates, getData().getQueryGeneration(), sqlDialect); + allQueryTemplates, allQueryTemplates, getData().getQueryGeneration()); this.resultSetMaterializer = new ResultSetMaterializer( @@ -649,18 +657,14 @@ protected boolean onStartup() throws InterruptedException { getData().getNativeTimeZone().orElse(ZoneId.of("UTC"))); this.featureMutationsSql = new FeatureMutationsSql( - this::getSqlClient, - new SqlInsertGenerator2( - getData().getNativeCrs().orElse(OgcCrs.CRS84), - crsTransformerFactory, - getData().getSourcePathDefaults()), - getData().getSourcePathDefaults()); - this.pathParser2 = createPathParser2(getData().getSourcePathDefaults(), cql); + this::getSqlClient, new SqlInsertGenerator2(getData().getSourcePathDefaults())); + this.pathParser2 = createPathParser2(getData().getSourcePathDefaults()); return true; } @Override + @SuppressWarnings("PMD.AvoidCatchingGenericException") protected void onStarted() { changes() .addListener( @@ -669,48 +673,57 @@ protected void onStarted() { super.onStarted(); - if (Runtime.getRuntime().availableProcessors() > getStreamRunner().getCapacity()) { + if (Runtime.getRuntime().availableProcessors() > getStreamRunner().getCapacity() + && LOGGER.isInfoEnabled()) { LOGGER.info( "Recommended max connections for optimal performance under load: {}", getMaxQueries() * Runtime.getRuntime().availableProcessors()); } Map> sourceSchema = new LinkedHashMap<>(); + MutationSchemaDeriver mutationSchemaDeriver = + new MutationSchemaDeriver(pathParser2, pathParser3); try { for (FeatureSchema fs : getData().getTypes().values()) { sourceSchema.put( - fs.getName(), - fs.accept(WITH_SCOPE_RECEIVABLE) - .accept(new MutationSchemaDeriver(pathParser2, pathParser3))); + fs.getName(), fs.accept(WITH_SCOPE_RECEIVABLE).accept(mutationSchemaDeriver)); } - } catch (Throwable e) { + } catch (RuntimeException e) { // ignore } - if (Objects.isNull(cronJob) - && Objects.nonNull(getData().getDatasetChanges().getSyncPeriodic())) { - if (getData().getDatasetChanges().isModeCrud() || getData().getDatasetChanges().isModeOff()) { + if (Objects.nonNull(cronJob) + || Objects.isNull(getData().getDatasetChanges().getSyncPeriodic())) { + return; + } + + if (getData().getDatasetChanges().isModeCrud() || getData().getDatasetChanges().isModeOff()) { + if (LOGGER.isWarnEnabled()) { LOGGER.warn( "Periodic dataset sync is not supported in mode '{}'", getData().getDatasetChanges().getMode()); - return; } + return; + } + + if (LOGGER.isDebugEnabled()) { LOGGER.debug( "Scheduling periodic dataset sync: {}", getData().getDatasetChanges().getSyncPeriodic()); - - this.cronJob = - scheduler.schedule( - LogContext.withMdc( - () -> - changes() - .handle( - ImmutableDatasetChange.builder() - .featureTypes(getData().getTypes().keySet()) - .build())), - getData().getDatasetChanges().getSyncPeriodic()); } + + this.cronJob = + scheduler.schedule( + LogContext.withMdc( + () -> + changes() + .handle( + ImmutableDatasetChange.builder() + .featureTypes(getData().getTypes().keySet()) + .build())), + getData().getDatasetChanges().getSyncPeriodic()); } @Override + @SuppressWarnings("PMD.NullAssignment") protected void onReloaded(boolean forceReload) { super.onReloaded(forceReload); @@ -735,6 +748,7 @@ protected void onReloaded(boolean forceReload) { } @Override + @SuppressWarnings("PMD.NullAssignment") protected void onStopped() { super.onStopped(); @@ -757,9 +771,9 @@ protected boolean allowForceReload() { private void clearCache(String type) { LOGGER.debug("Clearing cache for type: {}", type); - cache.del(type, "stats", "count"); - cache.del(type, "stats", "spatial"); - cache.del(type, "stats", "temporal"); + cache.del(type, CACHE_KEY_STATS, "count"); + cache.del(type, CACHE_KEY_STATS, "spatial"); + cache.del(type, CACHE_KEY_STATS, "temporal"); } // TODO: implement auto mode for maxConnections=-1, how to get numberOfQueries in Connector? @@ -825,9 +839,8 @@ protected int getRunnerQueueSize(ConnectionInfo connectionInfo) { } int capacity = maxConnections / maxQueries; // TODO - int queueSize = Math.max(1024, maxConnections * capacity * 2) / maxQueries; // LOGGER.info("RUNNERQ: {} {} {} {}", maxQueries ,maxConnections, capacity, queueSize); - return queueSize; + return Math.max(1024, maxConnections * capacity * 2) / maxQueries; } @Override @@ -966,10 +979,8 @@ private SqlClient getSqlClient() { @Override public boolean supportsMutationsInternal() { - if (!Objects.equals(getData().getConnectionInfo().getDialect(), SqlDbmsPgis.ID)) { - return false; - } - return getData().getDatasetChanges().isModeCrud(); + return Objects.equals(getData().getConnectionInfo().getDialect(), SqlDbmsPgis.ID) + && getData().getDatasetChanges().isModeCrud(); } @Override @@ -988,12 +999,13 @@ public boolean is3dSupported() { } @Override + @SuppressWarnings("PMD.AvoidCatchingGenericException") public long getFeatureCount(String typeName) { if (!queryMappings.containsKey(typeName)) { return -1; } - String[] cacheKey = {typeName, "stats", "count"}; + String[] cacheKey = {typeName, CACHE_KEY_STATS, "count"}; String cacheValidator = getData().getStableHash(); if (cache.hasValid(cacheValidator, cacheKey)) { @@ -1034,12 +1046,13 @@ public long getFeatureCount(String typeName) { } @Override + @SuppressWarnings("PMD.AvoidCatchingGenericException") public Optional getSpatialExtent(String typeName) { if (!queryMappings.containsKey(typeName)) { return Optional.empty(); } - String[] cacheKey = {typeName, "stats", "spatial"}; + String[] cacheKey = {typeName, CACHE_KEY_STATS, "spatial"}; String cacheValidator = getData().getStableHash(); if (cache.hasValid(cacheValidator, cacheKey)) { @@ -1102,19 +1115,20 @@ public Optional getSpatialExtent(String typeName, EpsgCrs crs) { crsTransformer -> { try { return Optional.of(crsTransformer.transformBoundingBox(boundingBox)); - } catch (Exception e) { + } catch (CrsTransformationException e) { return Optional.empty(); } })); } @Override + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.AvoidCatchingGenericException"}) public Optional getTemporalExtent(String typeName) { if (!queryMappings.containsKey(typeName)) { return Optional.empty(); } - String[] cacheKey = {typeName, "stats", "temporal"}; + String[] cacheKey = {typeName, CACHE_KEY_STATS, "temporal"}; String cacheValidator = getData().getStableHash(); if (cache.hasValid(cacheValidator, cacheKey)) { @@ -1197,10 +1211,10 @@ public Optional getTemporalExtent(String typeName) { // (and there are no map keys to get them from) // so to implement this we need the split between cfg and internal schemas private Map> generateSqlQueryMappings() { - String[] cacheKey = {"schema", "sql"}; + /*String[] cacheKey = {"schema", "sql"}; String cacheValidator = getData().getStableHash(); - /*if (cache.hasValid(cacheValidator, cacheKey)) { + if (cache.hasValid(cacheValidator, cacheKey)) { if (LOGGER.isDebugEnabled()) { LOGGER.debug("Using cached sql schemas"); } @@ -1223,19 +1237,15 @@ private Map> generateSqlQueryMappings() { .map(entry -> Map.entry(entry.getKey(), entry.getValue().accept(mappingRulesDeriver))) .collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue)); // LOGGER.debug("AFTER Derive MappingRules"); - Map> mappings = - getData().getTypes().entrySet().stream() - .map( - entry -> - Map.entry( - entry.getKey(), - sqlMappingDeriver.derive( - mappingRules.get(entry.getKey()), entry.getValue()))) - .collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue)); // LOGGER.debug("AFTER Derive SqlQueryMapping"); // cache.put(cacheValidator, mappings, cacheKey); - - return mappings; + return getData().getTypes().entrySet().stream() + .map( + entry -> + Map.entry( + entry.getKey(), + sqlMappingDeriver.derive(mappingRules.get(entry.getKey()), entry.getValue()))) + .collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue)); } @Override @@ -1306,29 +1316,23 @@ public boolean hasGeneratedId(String featureType) { Optional> queryMapping = Optional.ofNullable(queryMappings.get(featureType)); - if (queryMapping.isPresent()) { - return queryMapping.get().stream() - .allMatch( - mapping -> { - if (mapping.getColumnForId().isPresent() && mapping.getSchemaForId().isPresent()) { + return queryMapping.isEmpty() + || queryMapping.get().stream() + .allMatch( + mapping -> { + if (mapping.getColumnForId().isEmpty() || mapping.getSchemaForId().isEmpty()) { + return true; + } String primaryKey = mapping.getColumnForId().get().first().getPrimaryKey(); String idColumn = mapping.getColumnForId().get().second().getName(); - if (!Objects.equals(primaryKey, idColumn)) { - return false; - } - - return !mapping - .getColumnForId() - .get() - .second() - .hasOperation(Operation.DO_NOT_GENERATE); - } - return true; - }); - } - - return true; + return Objects.equals(primaryKey, idColumn) + && !mapping + .getColumnForId() + .get() + .second() + .hasOperation(Operation.DO_NOT_GENERATE); + }); } private MutationResult writeFeatures( @@ -1366,7 +1370,7 @@ private MutationResult writeFeatures( getNativeCrs(), crsTransformerFactory, getData().getNativeTimeZone(), - partial ? Optional.of(FeatureTransactions.PATCH_NULL_VALUE) : Optional.empty())) + partial ? Optional.of(PATCH_NULL_VALUE) : Optional.empty())) .via(Transformer.map(feature -> feature)); if (partial) { @@ -1397,7 +1401,7 @@ private MutationResult writeFeatures( return result.error(error); }) - .handleItem((Builder::addIds)) + .handleItem(Builder::addIds) .handleEnd(Builder::build) .on(getStreamRunner()); @@ -1410,6 +1414,7 @@ protected Query preprocessQuery(Query query) { return preprocessQuery(query, new ArrayList<>()); } + @SuppressWarnings("PMD.CognitiveComplexity") protected Query preprocessQuery(Query query, List resultSetTables) { if (query instanceof FeatureQuery && (((FeatureQuery) query).getFields().size() > 1 @@ -1540,10 +1545,8 @@ public boolean supportsHitsOnly() { @Override public boolean supportsAccenti() { - if (Objects.nonNull(getData().getQueryGeneration())) { - return getData().getQueryGeneration().getAccentiCollation().isPresent(); - } - return false; + return Objects.nonNull(getData().getQueryGeneration()) + && getData().getQueryGeneration().getAccentiCollation().isPresent(); } @Override @@ -1593,10 +1596,8 @@ private List getDialectAwareCustomFunctions(SqlDialect sqlDialec @Override public boolean skipUnusedPipelineSteps() { - if (Objects.nonNull(getData().getQueryProcessing())) { - return getData().getQueryProcessing().getSkipUnusedPipelineSteps(); - } - return false; + return Objects.nonNull(getData().getQueryProcessing()) + && getData().getQueryProcessing().getSkipUnusedPipelineSteps(); } @Override diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSqlData.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSqlData.java index 04f6f1e24..e5d30de5b 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSqlData.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureProviderSqlData.java @@ -11,7 +11,6 @@ import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import de.ii.xtraplatform.docs.DocIgnore; import de.ii.xtraplatform.docs.DocMarker; -import de.ii.xtraplatform.entities.domain.EntityDataBuilder; import de.ii.xtraplatform.entities.domain.EntityDataDefaults; import de.ii.xtraplatform.entities.domain.maptobuilder.BuildableMap; import de.ii.xtraplatform.features.domain.ExtensionConfiguration; @@ -127,8 +126,7 @@ default FeatureProviderSqlData mergeExtensions() { } abstract class Builder - extends FeatureProviderDataV2.Builder - implements EntityDataBuilder { + extends FeatureProviderDataV2.Builder { public abstract ImmutableFeatureProviderSqlData.Builder connectionInfo( ConnectionInfoSql connectionInfo); @@ -315,6 +313,7 @@ && getConnectionInfo().getAssumeExternalChanges()) { */ @Value.Immutable @JsonDeserialize(builder = ImmutableDatasetChangeSettings.Builder.class) + @SuppressWarnings("PMD.ImplicitFunctionalInterface") interface DatasetChangeSettings { /** diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureTokenStatsCollector.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureTokenStatsCollector.java index 6cc878ce5..12f7a0313 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureTokenStatsCollector.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/FeatureTokenStatsCollector.java @@ -17,26 +17,23 @@ import java.time.LocalDate; import java.time.ZoneOffset; import java.time.ZonedDateTime; +import java.time.format.DateTimeParseException; import java.util.Objects; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class FeatureTokenStatsCollector extends FeatureTokenTransformerSql { - private static final Logger LOGGER = LoggerFactory.getLogger(FeatureTokenStatsCollector.class); - private final Builder builder; private final EpsgCrs crs; - private int axis = 0; private int dim = 2; - private Double xmin = null; - private Double ymin = null; - private Double xmax = null; - private Double ymax = null; + private Double xmin; + private Double ymin; + private Double xmax; + private Double ymax; private String start = ""; private String end = ""; public FeatureTokenStatsCollector(Builder builder, EpsgCrs crs) { + super(); this.builder = builder; this.crs = crs; } @@ -71,38 +68,41 @@ private Long parseTemporal(String temporal) { return ZonedDateTime.parse(temporal).toInstant().toEpochMilli(); } return LocalDate.parse(temporal).atStartOfDay().toInstant(ZoneOffset.UTC).toEpochMilli(); - } catch (Throwable e) { + } catch (DateTimeParseException e) { return null; } } @Override + @SuppressWarnings("PMD.CyclomaticComplexity") public void onValue(ModifiableContext context) { - if (Objects.nonNull(context.value())) { - String value = context.value(); - - if (hasRole(context, Role.PRIMARY_INSTANT)) { - if (start.isEmpty() || value.compareTo(start) < 0) { - this.start = value; - } - if (end.isEmpty() || value.compareTo(end) > 0) { - this.end = value; - } - } else if (hasRole(context, Role.PRIMARY_INTERVAL_START)) { - if (start.isEmpty() || value.compareTo(start) < 0) { - this.start = value; - } - } else if (hasRole(context, Role.PRIMARY_INTERVAL_END)) { - if (end.isEmpty() || value.compareTo(end) > 0) { - this.end = value; - } + String value = context.value(); + + if (Objects.isNull(value)) { + super.onValue(context); + return; + } + + if (hasRole(context, Role.PRIMARY_INSTANT)) { + if (start.isEmpty() || value.compareTo(start) < 0) { + this.start = value; + } + if (end.isEmpty() || value.compareTo(end) > 0) { + this.end = value; } + } else if (hasRole(context, Role.PRIMARY_INTERVAL_START) + && (start.isEmpty() || value.compareTo(start) < 0)) { + this.start = value; + } else if (hasRole(context, Role.PRIMARY_INTERVAL_END) + && (end.isEmpty() || value.compareTo(end) > 0)) { + this.end = value; } super.onValue(context); } @Override + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) public void onGeometry(ModifiableContext context) { // Only the primary geometry feeds the spatial extent: secondary geometry properties may // store positions in a different CRS (position variants), which must not be interpreted in diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SchemaMappingSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SchemaMappingSql.java index 52e30010c..6aef8cdbe 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SchemaMappingSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SchemaMappingSql.java @@ -15,6 +15,7 @@ @Value.Immutable @Value.Style(deepImmutablesDetection = true, builder = "new", attributeBuilderDetection = true) +@SuppressWarnings("PMD.ImplicitFunctionalInterface") public interface SchemaMappingSql extends SchemaMappingBase { @Override diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SchemaSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SchemaSql.java index b7b926659..9d037f73f 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SchemaSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SchemaSql.java @@ -151,6 +151,7 @@ default List getSortKeys() { } // TODO: should we do this here? can we derive it from the above? + @SuppressWarnings("PMD.CyclomaticComplexity") default List getSortKeys( ListIterator aliasesIterator, boolean onlyRelations, int keyIndexStart) { ImmutableList.Builder keys = ImmutableList.builder(); diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlConnector.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlConnector.java index cf807662b..8a6370ef0 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlConnector.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlConnector.java @@ -8,7 +8,6 @@ package de.ii.xtraplatform.features.sql.domain; import com.google.common.collect.ImmutableList; -import de.ii.xtraplatform.base.domain.resiliency.Volatile2; import de.ii.xtraplatform.base.domain.util.Tuple; import de.ii.xtraplatform.features.domain.FeatureProviderConnector; import de.ii.xtraplatform.features.sql.domain.ImmutableSqlRowMeta.Builder; @@ -31,7 +30,56 @@ import org.postgresql.util.PSQLState; public interface SqlConnector - extends FeatureProviderConnector, Volatile2 { + extends FeatureProviderConnector { + + // Per-sub-query row buffer for the concurrent single-shot value phase (see getSourceStream). + int UNPAGED_SUBQUERY_PREFETCH = 256; + + String NEWLINE_INDENT = "\n "; + + Function PSQL_CONTEXT = + throwable -> { + if (throwable instanceof PSQLException) { + PSQLException e = (PSQLException) throwable; + String message = + Optional.ofNullable(e.getServerErrorMessage()) + .map( + serverErrorMessage -> { + StringBuilder totalMessage = new StringBuilder(NEWLINE_INDENT); + String msg = serverErrorMessage.getSeverity(); + if (msg != null) { + totalMessage.append(msg).append(": "); + } + msg = serverErrorMessage.getMessage(); + if (msg != null) { + totalMessage.append(msg); + } + msg = serverErrorMessage.getDetail(); + if (msg != null) { + totalMessage.append(NEWLINE_INDENT + "Detail: ").append(msg); + } + + msg = serverErrorMessage.getHint(); + if (msg != null) { + totalMessage.append(NEWLINE_INDENT + "Hint: ").append(msg); + } + msg = String.valueOf(serverErrorMessage.getPosition()); + if (!"0".equals(msg)) { + totalMessage.append(NEWLINE_INDENT + "Position: ").append(msg); + } + msg = serverErrorMessage.getWhere(); + if (msg != null) { + totalMessage.append(NEWLINE_INDENT + "Where: ").append(msg); + } + return totalMessage.toString(); + }) + .orElseGet(e::getMessage); + + return new PSQLException( + "Unexpected SQL query error: " + message, PSQLState.UNKNOWN_STATE, throwable); + } + return throwable; + }; int getMaxConnections(); @@ -80,6 +128,7 @@ public Paging(long limit, long offset, long chunkSize, boolean computeNumberMatc this.noOffset = false; } + @SuppressWarnings("PMD.CyclomaticComplexity") Optional> get(String currentTable) { long found = lastNumberReturned + lastNumberSkipped; @@ -118,11 +167,9 @@ void register(String currentTable, SqlRowMeta metaResult) { } } - // Per-sub-query row buffer for the concurrent single-shot value phase (see getSourceStream). - int UNPAGED_SUBQUERY_PREFETCH = 256; - // TODO: simplify, class SqlQueryRunner, remove options, singleFeature @Override + @SuppressWarnings("PMD.CognitiveComplexity") default Reactive.Source getSourceStream( SqlQueryBatch queryBatch, SqlQueryOptions options) { Paging paging = @@ -185,20 +232,20 @@ default Reactive.Source getSourceStream( rows.get(0).getNumberMatched().isEmpty() && nextRow.getNumberMatched().isEmpty() ? OptionalLong.empty() - : !Objects.equals(rows.get(0).getName(), nextRow.getName()) - ? OptionalLong.of( + : Objects.equals(rows.get(0).getName(), nextRow.getName()) + ? rows.get(0).getNumberMatched() + : OptionalLong.of( rows.get(0).getNumberMatched().orElse(0) - + nextRow.getNumberMatched().orElse(0)) - : rows.get(0).getNumberMatched(); + + nextRow.getNumberMatched().orElse(0)); OptionalLong numberSkipped3 = rows.get(0).getNumberSkipped().isEmpty() && nextRow.getNumberSkipped().isEmpty() ? OptionalLong.empty() - : !Objects.equals(rows.get(0).getName(), nextRow.getName()) - ? OptionalLong.of( + : Objects.equals(rows.get(0).getName(), nextRow.getName()) + ? rows.get(0).getNumberSkipped() + : OptionalLong.of( rows.get(0).getNumberSkipped().orElse(0) - + nextRow.getNumberSkipped().orElse(0)) - : rows.get(0).getNumberSkipped(); + + nextRow.getNumberSkipped().orElse(0)); rows.set( 0, @@ -230,30 +277,32 @@ default Reactive.Source getSourceStream( int fetchSize = unpaged ? (int) queryBatch.getChunkSize() : 0; Function> valuePhase = index -> { - int[] i = {0}; - Source[] sqlRows = + List valueQueries = querySets .get(index) .getValueQueries() .apply(sqlRowMeta, 0L, 0L) - .map( - valueQuery -> + .collect(Collectors.toList()); + Source[] sqlRows = + IntStream.range(0, valueQueries.size()) + .mapToObj( + k -> getSqlClient() .getSourceStream( - valueQuery, + valueQueries.get(k), new ImmutableSqlQueryOptions.Builder() .from(options) .tableSchema( querySets .get(index) .getTableSchemas() - .get(i[0])) + .get(k)) .type( querySets .get(index) .getOptions() .getType()) - .containerPriority(i[0]++) + .containerPriority(k) .queryIndex( querySets.get(index).getQueryIndex()) .fetchSize(fetchSize) @@ -330,12 +379,13 @@ default Reactive.Source getSourceStream( queryBatch.getOffset(), queryBatch.getChunkSize(), false); - int[] i = {0}; if (options.isHitsOnly()) { return Source.single(aggregatedMetaResult); } + int[] i = {0}; + return Source.iterable( IntStream.range(0, querySets.size()) .boxed() @@ -351,7 +401,6 @@ default Reactive.Source getSourceStream( .getTableSchemas() .get(0) .getFullPathAsString(); - int[] j = {0}; if (metaResults.get(index).getNumberReturned() <= 0) { paging2.register(currentTable, metaResults.get(index)); @@ -361,6 +410,7 @@ default Reactive.Source getSourceStream( Optional> maxLimitAndSkipped = paging2.get(currentTable); + int[] j = {0}; Source[] sqlRows = querySets .get(index) @@ -370,28 +420,33 @@ default Reactive.Source getSourceStream( maxLimitAndSkipped.get().first(), maxLimitAndSkipped.get().second()) .map( - valueQuery -> - getSqlClient() - .getSourceStream( - valueQuery, - new ImmutableSqlQueryOptions.Builder() - .from(options) - .tableSchema( - querySets - .get(index) - .getTableSchemas() - .get(j[0]++)) - .type( - querySets - .get(index) - .getOptions() - .getType()) - .containerPriority(i[0]++) - .queryIndex( - querySets - .get(index) - .getQueryIndex()) - .build())) + valueQuery -> { + int tableIndex = j[0]; + j[0]++; + int priority = i[0]; + i[0]++; + return getSqlClient() + .getSourceStream( + valueQuery, + new ImmutableSqlQueryOptions.Builder() + .from(options) + .tableSchema( + querySets + .get(index) + .getTableSchemas() + .get(tableIndex)) + .type( + querySets + .get(index) + .getOptions() + .getType()) + .containerPriority(priority) + .queryIndex( + querySets + .get(index) + .getQueryIndex()) + .build()); + }) .toArray((IntFunction[]>) Source[]::new); paging2.register(currentTable, metaResults.get(index)); @@ -469,48 +524,4 @@ static > Reactive.Source mergeAndSort( } return mergedAndSorted; } - - Function PSQL_CONTEXT = - throwable -> { - if (throwable instanceof PSQLException) { - PSQLException e = (PSQLException) throwable; - String message = - Optional.ofNullable(e.getServerErrorMessage()) - .map( - serverErrorMessage -> { - StringBuilder totalMessage = new StringBuilder("\n "); - String msg = serverErrorMessage.getSeverity(); - if (msg != null) { - totalMessage.append(msg).append(": "); - } - msg = serverErrorMessage.getMessage(); - if (msg != null) { - totalMessage.append(msg); - } - msg = serverErrorMessage.getDetail(); - if (msg != null) { - totalMessage.append("\n ").append("Detail: ").append(msg); - } - - msg = serverErrorMessage.getHint(); - if (msg != null) { - totalMessage.append("\n ").append("Hint: ").append(msg); - } - msg = String.valueOf(serverErrorMessage.getPosition()); - if (!"0".equals(msg)) { - totalMessage.append("\n ").append("Position: ").append(msg); - } - msg = serverErrorMessage.getWhere(); - if (msg != null) { - totalMessage.append("\n ").append("Where: ").append(msg); - } - return totalMessage.toString(); - }) - .orElseGet(e::getMessage); - - return new PSQLException( - "Unexpected SQL query error: " + message, PSQLState.UNKNOWN_STATE, throwable); - } - return throwable; - }; } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialect.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialect.java index 4ba6e8d2f..19c00a3a9 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialect.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialect.java @@ -22,8 +22,24 @@ import java.util.Set; import org.threeten.extra.Interval; +@SuppressWarnings("PMD.TooManyMethods") public interface SqlDialect { + Map SPATIAL_OPERATORS = + new ImmutableMap.Builder() + .put(SpatialFunction.S_EQUALS, "ST_Equals") + .put(SpatialFunction.S_DISJOINT, "ST_Disjoint") + .put(SpatialFunction.S_TOUCHES, "ST_Touches") + .put(SpatialFunction.S_WITHIN, "ST_Within") + .put(SpatialFunction.S_OVERLAPS, "ST_Overlaps") + .put(SpatialFunction.S_CROSSES, "ST_Crosses") + .put(SpatialFunction.S_INTERSECTS, "ST_Intersects") + .put(SpatialFunction.S_CONTAINS, "ST_Contains") + .build(); + + Map SPATIAL_OPERATORS_3D = + new ImmutableMap.Builder().build(); + String getId(); String applyToWkt(String column, boolean forcePolygonCCW, boolean linearizeCurves); @@ -151,19 +167,4 @@ default String applyToExpression( String table, String name, Map subDecoderPaths, boolean spatial) { return name; } - - Map SPATIAL_OPERATORS = - new ImmutableMap.Builder() - .put(SpatialFunction.S_EQUALS, "ST_Equals") - .put(SpatialFunction.S_DISJOINT, "ST_Disjoint") - .put(SpatialFunction.S_TOUCHES, "ST_Touches") - .put(SpatialFunction.S_WITHIN, "ST_Within") - .put(SpatialFunction.S_OVERLAPS, "ST_Overlaps") - .put(SpatialFunction.S_CROSSES, "ST_Crosses") - .put(SpatialFunction.S_INTERSECTS, "ST_Intersects") - .put(SpatialFunction.S_CONTAINS, "ST_Contains") - .build(); - - Map SPATIAL_OPERATORS_3D = - new ImmutableMap.Builder().build(); } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialectGpkg.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialectGpkg.java index 42af4c0b7..18ee6ea7d 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialectGpkg.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialectGpkg.java @@ -24,18 +24,19 @@ import java.util.Optional; import org.threeten.extra.Interval; +@SuppressWarnings("PMD.GodClass") public class SqlDialectGpkg implements SqlDialect { + private static final Splitter BBOX_SPLITTER = + Splitter.onPattern("[(), ]").omitEmptyStrings().trimResults(); + + private QueryGeneratorSettings settings; + @Override public String getId() { return SqlDbmsAdapterGpkg.ID; } - private QueryGeneratorSettings settings; - - private static final Splitter BBOX_SPLITTER = - Splitter.onPattern("[(), ]").omitEmptyStrings().trimResults(); - @Override public String applyToWkt(String column, boolean forcePolygonCCW, boolean linearizeCurves) { if (!forcePolygonCCW) { @@ -129,7 +130,6 @@ public String applyToDatetimeLiteral(String datetime) { public String applyToInstantMin() { return "0001-01-01T00:00:00Z"; } - ; @Override public String applyToInstantMax() { @@ -143,6 +143,7 @@ public String applyToDiameter(String geomExpression, boolean is3d) { } @Override + @SuppressWarnings("PMD.CyclomaticComplexity") public String applyToJsonValue( String alias, String column, String path, PropertyTypeInfo typeInfo) { @@ -168,6 +169,8 @@ public String applyToJsonValue( case FEATURE_REF_ARRAY: throw new IllegalArgumentException( "Arrays as queryables are not supported for GeoPackage feature providers."); + default: + break; } } @@ -190,8 +193,8 @@ private String getCast(Type valueType) { case INTEGER: case BOOLEAN: return "integer"; - default: case STRING: + default: return "text"; } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialectPgis.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialectPgis.java index 97a5e22cd..74ff77739 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialectPgis.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlDialectPgis.java @@ -33,8 +33,22 @@ import java.util.Set; import org.threeten.extra.Interval; +@SuppressWarnings({"PMD.GodClass", "PMD.CyclomaticComplexity"}) public class SqlDialectPgis implements SqlDialect { + private static final Splitter BBOX_SPLITTER = + Splitter.onPattern("[(), ]").omitEmptyStrings().trimResults(); + private static final Map SPATIAL_OPERATORS_3D = + new ImmutableMap.Builder() + .put(SpatialFunction.S_INTERSECTS, "ST_3DIntersects") + .build(); + public static final Map TEMPORAL_OPERATORS = + new ImmutableMap.Builder() + .put( + TemporalFunction.T_INTERSECTS, + "OVERLAPS") // "({start1},{end1}) OVERLAPS ({start2},{end2})" + .build(); + @Override public String getId() { return SqlDbmsPgis.ID; @@ -71,22 +85,9 @@ public String dropResultSetTable(String name) { return String.format("DROP TABLE IF EXISTS %s", name); } - private static final Splitter BBOX_SPLITTER = - Splitter.onPattern("[(), ]").omitEmptyStrings().trimResults(); - private static final Map SPATIAL_OPERATORS_3D = - new ImmutableMap.Builder() - .put(SpatialFunction.S_INTERSECTS, "ST_3DIntersects") - .build(); - public static final Map TEMPORAL_OPERATORS = - new ImmutableMap.Builder() - .put( - TemporalFunction.T_INTERSECTS, - "OVERLAPS") // "({start1},{end1}) OVERLAPS ({start2},{end2})" - .build(); - @Override public String applyToWkt(String column, boolean forcePolygonCCW, boolean linearizeCurves) { - StringBuilder queryBuilder = new StringBuilder("ST_AsText("); + StringBuilder queryBuilder = new StringBuilder(80).append("ST_AsText("); if (linearizeCurves) { queryBuilder.append("ST_CurveToLine("); } @@ -95,12 +96,12 @@ public String applyToWkt(String column, boolean forcePolygonCCW, boolean lineari } queryBuilder.append(column); if (forcePolygonCCW) { - queryBuilder.append(")"); + queryBuilder.append(')'); } if (linearizeCurves) { queryBuilder.append(",32,0,1)"); } - return queryBuilder.append(")").toString(); + return queryBuilder.append(')').toString(); } @Override @@ -110,7 +111,7 @@ public String applyToWkt(String wkt, int srid) { @Override public String applyToWkb(String column, boolean forcePolygonCCW, boolean linearizeCurves) { - StringBuilder binaryBuilder = new StringBuilder("ST_AsBinary("); + StringBuilder binaryBuilder = new StringBuilder(80).append("ST_AsBinary("); if (linearizeCurves) { binaryBuilder.append("ST_CurveToLine("); } @@ -119,12 +120,12 @@ public String applyToWkb(String column, boolean forcePolygonCCW, boolean lineari } binaryBuilder.append(column); if (forcePolygonCCW) { - binaryBuilder.append(")"); + binaryBuilder.append(')'); } if (linearizeCurves) { binaryBuilder.append(",32,0,1)"); } - return binaryBuilder.append(")").toString(); + return binaryBuilder.append(')').toString(); } @Override @@ -239,7 +240,6 @@ public String applyToDatetimeLiteral(String datetime) { public String applyToInstantMin() { return "-infinity"; } - ; @Override public String applyToInstantMax() { @@ -269,6 +269,7 @@ public String applyToDiameter(String geomExpression, boolean is3d) { } @Override + @SuppressWarnings("PMD.CyclomaticComplexity") public String applyToJsonValue( String alias, String column, String path, PropertyTypeInfo typeInfo) { @@ -291,6 +292,8 @@ public String applyToJsonValue( case FEATURE_REF_ARRAY: cast = typeInfo.getValueType().map(this::getCast).orElse(getCast(Type.STRING)); break; + default: + break; } } @@ -321,13 +324,14 @@ private String getCast(Type valueType) { return "::integer"; case BOOLEAN: return "::boolean"; - default: case STRING: + default: return "::varchar"; } } @Override + @SuppressWarnings("PMD.CyclomaticComplexity") public String applyToJsonArrayOp( ArrayFunction op, boolean notInverse, String mainExpression, String jsonValueArray) { if (notInverse ? op == A_CONTAINS : op == A_CONTAINEDBY) { diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlPath.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlPath.java index e9aaef5ac..2c004ccd9 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlPath.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlPath.java @@ -107,6 +107,7 @@ default String asPath() { + getFilterString().map(filterString -> "{filter=" + filterString + "}").orElse(""); } + @Override @Value.Derived default List getParentPath() { return getParentTables().stream().map(SqlPath::asPath).collect(Collectors.toList()); @@ -123,9 +124,8 @@ default List getFullPath() { default boolean parentsIntersect(List parents) { List fullParentPath = parents.stream().flatMap(p -> p.getFullPath().stream()).collect(Collectors.toList()); - boolean intersects = MappedSchemaDeriver.intersects(fullParentPath, getParentPath()); - return intersects; + return MappedSchemaDeriver.intersects(fullParentPath, getParentPath()); } @Override @@ -152,9 +152,12 @@ default T withoutParentIntersection(List parents) { int start = fullParentPath.indexOf(getParentTables().get(0).asPath()); for (int i = 0; i < getParentTables().size(); i++) { - if (fullParentPath.size() > start - && Objects.equals(fullParentPath.get(start++), getParentTables().get(i).asPath())) { - continue; + if (fullParentPath.size() > start) { + int currentStart = start; + start++; + if (Objects.equals(fullParentPath.get(currentStart), getParentTables().get(i).asPath())) { + continue; + } } newParentTables.add(getParentTables().get(i)); } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlPathParser.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlPathParser.java index cb7fec142..67a34e6f6 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlPathParser.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlPathParser.java @@ -34,14 +34,30 @@ import java.util.stream.Stream; import org.apache.hc.core5.http.NameValuePair; import org.apache.hc.core5.net.URLEncodedUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; // TODO: use parser library, e.g. // https://github.com/zhong-j-yu/rekex/blob/main/rekex-example/src/main/java/org/rekex/exmple/parser/ExampleParser_Uri.java (Java17) // or https://github.com/typemeta/funcj/tree/master/parser +@SuppressWarnings({ + "PMD.CouplingBetweenObjects", + "PMD.GodClass", + "PMD.CyclomaticComplexity", + "PMD.TooManyMethods" +}) public class SqlPathParser { + private static final Splitter PATH_SPLITTER = + Splitter.on(Tokens.PATH_SEPARATOR).omitEmptyStrings(); + private static final Joiner PATH_JOINER = Joiner.on(Tokens.PATH_SEPARATOR).skipNulls(); + private static final Splitter MULTI_COLUMN_SPLITTER = + Splitter.on(Tokens.MULTI_COLUMN_SEPARATOR).omitEmptyStrings(); + + private final SqlPathDefaults defaults; + private final Cql cql; + // TODO: remove + private final Optional junctionTableMatcher; + private final Map connectors; + private enum MatcherGroups { PATH, SCHEMA, @@ -195,20 +211,6 @@ private interface Patterns { Pattern JOIN_TYPE_FLAG = Pattern.compile(PatternStrings.JOIN_TYPE_FLAG); } - private static final Logger LOGGER = LoggerFactory.getLogger(SqlPathParser.class); - - private static final Splitter PATH_SPLITTER = - Splitter.on(Tokens.PATH_SEPARATOR).omitEmptyStrings(); - private static final Joiner PATH_JOINER = Joiner.on(Tokens.PATH_SEPARATOR).skipNulls(); - private static final Splitter MULTI_COLUMN_SPLITTER = - Splitter.on(Tokens.MULTI_COLUMN_SEPARATOR).omitEmptyStrings(); - - private final SqlPathDefaults defaults; - private final Cql cql; - // TODO: remove - private final Optional junctionTableMatcher; - private final Map connectors; - public SqlPathParser(SqlPathDefaults defaults, Cql cql, Map connectors) { this.defaults = defaults; this.cql = cql; @@ -237,36 +239,39 @@ public SqlPath parseColumnPath(String path) { Matcher matcher = Patterns.COLUMN_PATH.matcher(path); - if (matcher.find()) { - String column = matcher.group(MatcherGroups.COLUMNS.name()); - - if (Objects.nonNull(column)) { - List columns = MULTI_COLUMN_SPLITTER.splitToList(column); - Builder builder = new ImmutableSqlPath.Builder().name(column).columns(columns); + if (!matcher.find()) { + throw new IllegalArgumentException( + String.format("invalid sourcePath '%s', expected column", path)); + } - String tablePath = matcher.group(MatcherGroups.PATH.name()); + String column = matcher.group(MatcherGroups.COLUMNS.name()); - if (Objects.nonNull(tablePath)) { - builder.parentTables(parseTables(tablePath)); - } + if (Objects.isNull(column)) { + throw new IllegalArgumentException( + String.format("invalid sourcePath '%s', expected column", path)); + } - String flags = Optional.ofNullable(matcher.group(MatcherGroups.FLAGS.name())).orElse(""); + List columns = MULTI_COLUMN_SPLITTER.splitToList(column); + Builder builder = new ImmutableSqlPath.Builder().name(column).columns(columns); - // TODO - builder - .sortKey("") - .sortKeyUnique(true) - .primaryKey("") - .junction(false) - .constantValue(getConstantFlag(flags)) - .generated(getGeneratedFlag(flags)); + String tablePath = matcher.group(MatcherGroups.PATH.name()); - return builder.build(); - } + if (Objects.nonNull(tablePath)) { + builder.parentTables(parseTables(tablePath)); } - throw new IllegalArgumentException( - String.format("invalid sourcePath '%s', expected column", path)); + String flags = Optional.ofNullable(matcher.group(MatcherGroups.FLAGS.name())).orElse(""); + + // TODO + builder + .sortKey("") + .sortKeyUnique(true) + .primaryKey("") + .junction(false) + .constantValue(getConstantFlag(flags)) + .generated(getGeneratedFlag(flags)); + + return builder.build(); } public String tablePathWithDefaults(String path) { @@ -494,11 +499,8 @@ public String getSortKey(String flags) { public boolean getSortKeyUnique(String flags) { Matcher matcher = Patterns.SORT_KEY_UNIQUE_FLAG.matcher(flags); - if (matcher.find()) { - return Boolean.parseBoolean(matcher.group(MatcherGroups.SORTKEYUNIQUE.name())); - } - - return true; + return !matcher.find() + || Boolean.parseBoolean(matcher.group(MatcherGroups.SORTKEYUNIQUE.name())); } public JoinType getJoinType(String flags) { diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryColumn.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryColumn.java index efce860a3..ef85fbaae 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryColumn.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryColumn.java @@ -10,7 +10,6 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import de.ii.xtraplatform.features.domain.SchemaBase; -import de.ii.xtraplatform.features.sql.domain.SqlQueryTable.DefaultsFilter; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -80,5 +79,12 @@ class DefaultsFilter { public boolean equals(Object value) { return Objects.equals(value, 0); } + + // required to pair with the custom equals() above; PMD sees this as merely calling super + @Override + @SuppressWarnings("PMD.UselessOverridingMethod") + public int hashCode() { + return super.hashCode(); + } } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryJoin.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryJoin.java index 68fa892d4..8d7ef772f 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryJoin.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryJoin.java @@ -45,5 +45,12 @@ class DefaultsFilter { public boolean equals(Object value) { return Objects.equals(value, false) || Objects.equals(value, SqlPath.JoinType.INNER); } + + // required to pair with the custom equals() above; PMD sees this as merely calling super + @Override + @SuppressWarnings("PMD.UselessOverridingMethod") + public int hashCode() { + return super.hashCode(); + } } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryMapping.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryMapping.java index bef49b975..e95bc6a07 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryMapping.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryMapping.java @@ -29,6 +29,9 @@ @JsonDeserialize(builder = ImmutableSqlQueryMapping.Builder.class) public interface SqlQueryMapping { + String IN_CONNECTED_ARRAY = "IN_CONNECTED_ARRAY"; + String PATH_IN_CONNECTOR = "PATH_IN_CONNECTOR"; + List getTables(); @Value.Lazy @@ -171,9 +174,6 @@ default Optional getSchemaForValue(String propertyName) { return Optional.ofNullable(getValueSchemas().get(propertyName)); } - String IN_CONNECTED_ARRAY = "IN_CONNECTED_ARRAY"; - String PATH_IN_CONNECTOR = "PATH_IN_CONNECTOR"; - default boolean isInConnectedArray(FeatureSchema schema) { return Boolean.parseBoolean(schema.getAdditionalInfo().get(IN_CONNECTED_ARRAY)); } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQuerySchema.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQuerySchema.java index 56b7603da..101f092d8 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQuerySchema.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQuerySchema.java @@ -83,17 +83,17 @@ default List getColumnPath(SqlQueryColumn column) { @Value.Lazy default List getSortKeys() { List keys = new ArrayList<>(); - String prefix = ""; + StringBuilder prefix = new StringBuilder(); for (SqlQueryJoin join : getRelations()) { - prefix += join.getPathSegment(); + prefix.append(join.getPathSegment()); if (!join.isJunction()) { - keys.add(String.format("%s.%s", prefix, join.getSortKey())); + keys.add(String.format("%s.%s", prefix.toString(), join.getSortKey())); } } - prefix += this.getPathSegment(); + prefix.append(this.getPathSegment()); - keys.add(String.format("%s.%s", prefix, this.getSortKey())); + keys.add(String.format("%s.%s", prefix.toString(), this.getSortKey())); return keys; } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryTable.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryTable.java index 5d1d8fdf0..a62cccfab 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryTable.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/SqlQueryTable.java @@ -46,5 +46,12 @@ class DefaultsFilter { public boolean equals(Object value) { return Objects.equals(value, true) || Objects.equals(value, "id"); } + + // required to pair with the custom equals() above; PMD sees this as merely calling super + @Override + @SuppressWarnings("PMD.UselessOverridingMethod") + public int hashCode() { + return super.hashCode(); + } } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/ValueTypeMapping.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/ValueTypeMapping.java index bbaa12c16..69e353c90 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/ValueTypeMapping.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/domain/ValueTypeMapping.java @@ -17,7 +17,7 @@ import java.util.Objects; import java.util.stream.Stream; -public class ValueTypeMapping { +public final class ValueTypeMapping { private static final List SQL_BOOLEANS = ImmutableList.of(JDBCType.BOOLEAN); @@ -62,7 +62,7 @@ public Integer getVendorTypeNumber() { } }); - private static final Map> mappings = + private static final Map> MAPPINGS = new ImmutableMap.Builder>() .put(Type.BOOLEAN, SQL_BOOLEANS) .put(Type.INTEGER, SQL_INTEGERS) @@ -72,8 +72,10 @@ public Integer getVendorTypeNumber() { .put(Type.GEOMETRY, SQL_GEOMETRIES) .build(); + private ValueTypeMapping() {} + public static List getSourceTypes(Type type) { - return mappings.getOrDefault(type, ImmutableList.of()); + return MAPPINGS.getOrDefault(type, ImmutableList.of()); } public static boolean matches(SQLType sqlType, String databaseSpecificTypeName, Type type) { diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSession.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSession.java index 3512de43b..b449ee586 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSession.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/JdbcSqlSession.java @@ -26,10 +26,14 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@SuppressWarnings("PMD.CyclomaticComplexity") class JdbcSqlSession implements SqlSession { private static final Logger LOGGER = LoggerFactory.getLogger(JdbcSqlSession.class); + private static final String ERROR_SESSION_CLOSED = "SQL session is closed"; + private static final String ERROR_STATEMENT_CONTEXT = " — statement: "; + // Safety cap on accumulated batch size — pathological feature shouldn't grow unbounded. private static final int MAX_BATCH_SIZE = 1000; @@ -56,12 +60,17 @@ class JdbcSqlSession implements SqlSession { } @Override + @SuppressWarnings({ + "PMD.UseTryWithResources", + "PMD.CognitiveComplexity", + "PMD.CyclomaticComplexity" + }) public String run( List> statements, List> idConsumers, Optional featureId) { if (finalised) { - throw new IllegalStateException("SQL session is closed"); + throw new IllegalStateException(ERROR_SESSION_CLOSED); } String firstGeneratedId = null; Statement batchStmt = null; @@ -84,7 +93,7 @@ public String run( batchStmt.addBatch(sql); } catch (SQLException e) { throw new IllegalStateException( - "Mutation statement failed: " + e.getMessage() + " — statement: " + sql, e); + "Mutation statement failed: " + e.getMessage() + ERROR_STATEMENT_CONTEXT + sql, e); } batchedSql.add(sql); batchedConsumers.add(consumer); @@ -121,7 +130,7 @@ public String run( } } catch (SQLException e) { throw new IllegalStateException( - "Mutation statement failed: " + e.getMessage() + " — statement: " + sql, e); + "Mutation statement failed: " + e.getMessage() + ERROR_STATEMENT_CONTEXT + sql, e); } } @@ -143,7 +152,7 @@ public String run( @Override public List runReturning(String sql) { if (finalised) { - throw new IllegalStateException("SQL session is closed"); + throw new IllegalStateException(ERROR_SESSION_CLOSED); } if (LOGGER.isDebugEnabled(MARKER.SQL)) { LOGGER.debug(MARKER.SQL, "Executing statement: {}", sql); @@ -163,14 +172,14 @@ public List runReturning(String sql) { return ids; } catch (SQLException e) { throw new IllegalStateException( - "Mutation statement failed: " + e.getMessage() + " — statement: " + sql, e); + "Mutation statement failed: " + e.getMessage() + ERROR_STATEMENT_CONTEXT + sql, e); } } @Override public List execute(List statements) { if (finalised) { - throw new IllegalStateException("SQL session is closed"); + throw new IllegalStateException(ERROR_SESSION_CLOSED); } List warnings = new ArrayList<>(); for (String sql : statements) { @@ -186,7 +195,9 @@ public List execute(List statements) { // Expected, configuration-driven failure (e.g. a check function RAISE EXCEPTION) — carry // the warnings collected so far so they survive the failure path. throw new FeatureMutationHookException( - "Hook statement failed: " + e.getMessage() + " — statement: " + sql, e, warnings); + "Hook statement failed: " + e.getMessage() + ERROR_STATEMENT_CONTEXT + sql, + e, + warnings); } } return warnings; @@ -215,7 +226,9 @@ private void harvestWarnings(Statement statement) { } statement.clearWarnings(); } catch (SQLException e) { - LOGGER.debug("Reading SQL warnings failed: {}", e.getMessage()); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Reading SQL warnings failed: {}", e.getMessage()); + } } } @@ -262,7 +275,7 @@ private void flushBatch( @Override public void savepoint() { if (finalised) { - throw new IllegalStateException("SQL session is closed"); + throw new IllegalStateException(ERROR_SESSION_CLOSED); } if (activeSavepoint != null) { throw new IllegalStateException("A savepoint is already active on this SQL session"); @@ -275,9 +288,10 @@ public void savepoint() { } @Override + @SuppressWarnings("PMD.NullAssignment") public void releaseSavepoint() { if (finalised) { - throw new IllegalStateException("SQL session is closed"); + throw new IllegalStateException(ERROR_SESSION_CLOSED); } if (activeSavepoint == null) { throw new IllegalStateException("No savepoint is active on this SQL session"); @@ -292,9 +306,10 @@ public void releaseSavepoint() { } @Override + @SuppressWarnings("PMD.NullAssignment") public void rollbackToSavepoint() { if (finalised) { - throw new IllegalStateException("SQL session is closed"); + throw new IllegalStateException(ERROR_SESSION_CLOSED); } if (activeSavepoint == null) { throw new IllegalStateException("No savepoint is active on this SQL session"); @@ -312,7 +327,7 @@ public void rollbackToSavepoint() { @Override public void commit() { if (finalised) { - throw new IllegalStateException("SQL session is closed"); + throw new IllegalStateException(ERROR_SESSION_CLOSED); } try { connection.commit(); @@ -332,7 +347,9 @@ public void rollback() { try { connection.rollback(); } catch (SQLException e) { - LOGGER.warn("Rollback failed: {}", e.getMessage()); + if (LOGGER.isWarnEnabled()) { + LOGGER.warn("Rollback failed: {}", e.getMessage()); + } } finally { finalised = true; releaseConnection(); @@ -341,10 +358,10 @@ public void rollback() { @Override public void close() { - if (!finalised) { - rollback(); - } else { + if (finalised) { releaseConnection(); + } else { + rollback(); } } @@ -355,7 +372,9 @@ private void releaseConnection() { connection.close(); } } catch (SQLException e) { - LOGGER.debug("Connection close failed: {}", e.getMessage()); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Connection close failed: {}", e.getMessage()); + } } } } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SchemaGeneratorSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SchemaGeneratorSql.java index 6cfc8bb64..7a46ae002 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SchemaGeneratorSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SchemaGeneratorSql.java @@ -37,6 +37,7 @@ import schemacrawler.schema.Schema; import schemacrawler.schema.Table; +@SuppressWarnings({"PMD.CouplingBetweenObjects", "PMD.GodClass"}) public class SchemaGeneratorSql implements SchemaGenerator { private static final Logger LOGGER = LoggerFactory.getLogger(SchemaGeneratorSql.class); @@ -54,6 +55,7 @@ public SchemaGeneratorSql(SqlClientBasic sqlClientBasic) { } @Override + @SuppressWarnings("PMD.AvoidCatchingGenericException") public Map> analyze() { try { Catalog catalog = @@ -77,6 +79,7 @@ public Map> analyze() { } @Override + @SuppressWarnings("PMD.AvoidCatchingGenericException") public List generate( Map> types, Consumer>> tracker) { try { @@ -112,12 +115,14 @@ public List generate( return getFeatureType(table, geoInfo); } catch (Throwable e) { - LOGGER.warn( - "Could not generate schema for {}.{}: {} ({})", - schema.getKey(), - tableName, - e.getClass().getSimpleName(), - Objects.requireNonNullElse(e.getMessage(), "no message")); + if (LOGGER.isWarnEnabled()) { + LOGGER.warn( + "Could not generate schema for {}.{}: {} ({})", + schema.getKey(), + tableName, + e.getClass().getSimpleName(), + Objects.requireNonNullElse(e.getMessage(), "no message")); + } if (LOGGER.isDebugEnabled()) { LOGGER.debug("Stacktrace:", e); } @@ -158,6 +163,16 @@ private static void track( } } + // Kept for direct unit-test coverage (SchemaGeneratorSqlSpec); the production path in generate() + // uses the single-table getFeatureType(), which PMD's source-only analysis can't see. + @SuppressWarnings({ + "PMD.UnusedPrivateMethod", + "PMD.AvoidInstantiatingObjectsInLoops", + "PMD.AvoidDeeplyNestedIfStmts", + "PMD.CognitiveComplexity", + "PMD.CyclomaticComplexity", + "PMD.NPathComplexity" + }) private List getFeatureTypes( Catalog catalog, List includeTables, Map geometryInfos) { ImmutableList.Builder featureTypes = new ImmutableList.Builder<>(); @@ -177,7 +192,7 @@ private List getFeatureTypes( ImmutableFeatureSchema.Builder featureType = new ImmutableFeatureSchema.Builder() .name(table.getName()) - .sourcePath("/" + table.getName().toLowerCase()); + .sourcePath("/" + table.getName().toLowerCase(Locale.ROOT)); boolean idFound = false; @@ -211,7 +226,7 @@ private List getFeatureTypes( ImmutableMap.of( "crs", String.valueOf(srid), "force", geometryInfo.getForce())); } - } catch (Throwable e) { + } catch (NumberFormatException e) { // ignore } @@ -224,7 +239,8 @@ private List getFeatureTypes( ImmutableFeatureSchema featureSchema = featureType.build(); - if (featureSchema.getProperties().stream().noneMatch(FeatureSchema::isId)) { + if (featureSchema.getProperties().stream().noneMatch(FeatureSchema::isId) + && LOGGER.isWarnEnabled()) { LOGGER.warn( "No primary key or unique column found for table '{}', you have to adjust the type configuration manually.", table.getName()); @@ -236,6 +252,13 @@ private List getFeatureTypes( return featureTypes.build(); } + @SuppressWarnings({ + "PMD.AvoidInstantiatingObjectsInLoops", + "PMD.AvoidDeeplyNestedIfStmts", + "PMD.CognitiveComplexity", + "PMD.CyclomaticComplexity", + "PMD.NPathComplexity" + }) private FeatureSchema getFeatureType(Table table, Map geometryInfos) { if (LOGGER.isDebugEnabled()) { LOGGER.debug("Generating type '{}'", table.getName()); @@ -246,7 +269,7 @@ private FeatureSchema getFeatureType(Table table, Map geometryI ImmutableFeatureSchema.Builder featureType = new ImmutableFeatureSchema.Builder() .name(table.getName()) - .sourcePath("/" + table.getName().toLowerCase()); + .sourcePath("/" + table.getName().toLowerCase(Locale.ROOT)); boolean idFound = false; @@ -283,7 +306,7 @@ private FeatureSchema getFeatureType(Table table, Map geometryI featureProperty.additionalInfo( ImmutableMap.of("crs", String.valueOf(srid), "force", geometryInfo.getForce())); } - } catch (Throwable e) { + } catch (NumberFormatException e) { // ignore } @@ -296,7 +319,8 @@ private FeatureSchema getFeatureType(Table table, Map geometryI FeatureSchema featureSchema = featureType.build(); - if (featureSchema.getProperties().stream().noneMatch(FeatureSchema::isId)) { + if (featureSchema.getProperties().stream().noneMatch(FeatureSchema::isId) + && LOGGER.isWarnEnabled()) { LOGGER.warn( "No primary key or unique column found for table '{}', you have to adjust the type configuration manually.", table.getName()); @@ -330,7 +354,7 @@ private SchemaBase.Type getFeaturePropertyType(ColumnDataType columnDataType) { != WktWkbGeometryType.NONE) { return SchemaBase.Type.GEOMETRY; } - } catch (Exception ignore) { + } catch (IllegalArgumentException ignore) { // ignore, not a geometry } diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SchemaInfo.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SchemaInfo.java index 55baee4ae..72a917c10 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SchemaInfo.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SchemaInfo.java @@ -33,11 +33,11 @@ class SchemaInfo { this.tables = tables; } - public boolean tableExists(String name) { + boolean tableExists(String name) { return tables.stream().anyMatch(t -> t.getName().equals(name)); } - public boolean columnExists(String name, String table) { + boolean columnExists(String name, String table) { return tables.stream() .filter(t -> t.getName().equals(table)) .flatMap(t -> t.getColumns().stream()) @@ -46,11 +46,11 @@ public boolean columnExists(String name, String table) { // TODO: unique may be either column constraint, table constraint or index plus primary key // TODO: NOT NULL constraints - public boolean isColumnUnique(String columnName, String tableName) { + boolean isColumnUnique(String columnName, String tableName) { return isColumnUnique(columnName, tableName, true); } - public boolean isColumnUnique(String columnName, String tableName, boolean warn) { + boolean isColumnUnique(String columnName, String tableName, boolean warn) { Optional optionalColumn = getColumn(tableName, columnName, true, warn); if (optionalColumn.isPresent()) { @@ -78,13 +78,13 @@ public boolean isColumnUnique(String columnName, String tableName, boolean warn) return false; } - public boolean isColumnReadOnly(String columnName, String tableName) { + boolean isColumnReadOnly(String columnName, String tableName) { return getColumn(tableName, columnName, true, false) .map(column -> column.isAutoIncremented() || column.isGenerated()) .orElse(false); } - public boolean isColumnSpatial(String table, String name) { + boolean isColumnSpatial(String table, String name) { return getColumn(table, name) .filter( c -> @@ -95,7 +95,7 @@ public boolean isColumnSpatial(String table, String name) { .isPresent(); } - public boolean isColumnTemporal(String table, String name) { + boolean isColumnTemporal(String table, String name) { return getColumn(table, name) .filter( c -> @@ -106,11 +106,11 @@ public boolean isColumnTemporal(String table, String name) { .isPresent(); } - public Optional getColumn(String tableName, String columnName) { + Optional getColumn(String tableName, String columnName) { return getColumn(tableName, columnName, false, false); } - public Optional getColumn( + Optional getColumn( String tableName, String columnName, boolean resolveViews, boolean warn) { Optional optionalColumn = tables.stream() @@ -119,21 +119,25 @@ public Optional getColumn( .filter(c -> c.getName().equals(columnName)) .findFirst(); - if (resolveViews && optionalColumn.isPresent()) { - Table table = optionalColumn.get().getParent(); + if (!resolveViews || optionalColumn.isEmpty()) { + return optionalColumn; + } + + Table table = optionalColumn.get().getParent(); - if (table instanceof View) { - Optional> originalColumn = - ViewInfo.getOriginalTableAndColumn(table.getDefinition(), columnName); + if (!(table instanceof View)) { + return optionalColumn; + } - if (originalColumn.isPresent()) { - return getColumn(originalColumn.get().first(), originalColumn.get().second()); - } + Optional> originalColumn = + ViewInfo.getOriginalTableAndColumn(table.getDefinition(), columnName); + + if (originalColumn.isPresent()) { + return getColumn(originalColumn.get().first(), originalColumn.get().second()); + } - if (warn) { - LOGGER.warn(VIEW_COLUMN_NOT_ANALYZABLE, columnName, tableName); - } - } + if (warn) { + LOGGER.warn(VIEW_COLUMN_NOT_ANALYZABLE, columnName, tableName); } return optionalColumn; diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SourceSchemaValidatorSql.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SourceSchemaValidatorSql.java index cc0c2e1e6..a6afe689d 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SourceSchemaValidatorSql.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SourceSchemaValidatorSql.java @@ -21,14 +21,11 @@ import java.util.List; import java.util.function.Supplier; import java.util.stream.Collectors; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import schemacrawler.schema.Catalog; import schemacrawler.schemacrawler.exceptions.SchemaCrawlerException; public class SourceSchemaValidatorSql implements SourceSchemaValidator { - private static final Logger LOGGER = LoggerFactory.getLogger(SourceSchemaValidatorSql.class); public static final String TABLE_DOES_NOT_EXIST = "%s: table '%s' does not exist"; public static final String COLUMN_DOES_NOT_EXIST = "%s: column '%s' in table '%s' does not exist"; public static final String COLUMN_NOT_UNIQUE = @@ -37,7 +34,7 @@ public class SourceSchemaValidatorSql implements SourceSchemaValidator schemas; - private Supplier sqlClient; + private final Supplier sqlClient; public SourceSchemaValidatorSql(List schemas, Supplier sqlClient) { this.schemas = schemas; @@ -60,11 +57,22 @@ public ValidationResult validate(String typeName, List sourceSchemas, } } + @SuppressWarnings({ + "PMD.CognitiveComplexity", + "PMD.CyclomaticComplexity", + "PMD.NPathComplexity", + "PMD.AvoidDeeplyNestedIfStmts" + }) private ValidationResult validate( String typeName, SchemaSql tableSchema, MODE mode, SchemaInfo schemaInfo) { ImmutableValidationResult.Builder result = ImmutableValidationResult.builder().mode(mode); - if (!tableSchema.getRelation().isEmpty()) { + if (tableSchema.getRelation().isEmpty()) { + if (!schemaInfo.tableExists(tableSchema.getName())) { + String context = String.format("Invalid sourcePath in type '%s'", typeName); + result.addErrors(String.format(TABLE_DOES_NOT_EXIST, context, tableSchema.getName())); + } + } else { List relations = tableSchema.getRelation(); for (int i = 0; i < relations.size(); i++) { @@ -87,17 +95,18 @@ private ValidationResult validate( relation.getSourceContainer())); } - if (!schemaInfo.tableExists(relation.getTargetContainer())) { + if (schemaInfo.tableExists(relation.getTargetContainer())) { + if (!schemaInfo.columnExists(relation.getTargetField(), relation.getTargetContainer())) { + result.addErrors( + String.format( + COLUMN_DOES_NOT_EXIST, + context, + relation.getTargetField(), + relation.getTargetContainer())); + } + } else { result.addErrors( String.format(TABLE_DOES_NOT_EXIST, context, relation.getTargetContainer())); - } else if (!schemaInfo.columnExists( - relation.getTargetField(), relation.getTargetContainer())) { - result.addErrors( - String.format( - COLUMN_DOES_NOT_EXIST, - context, - relation.getTargetField(), - relation.getTargetContainer())); } if (relation.getJunction().isPresent() @@ -105,34 +114,31 @@ private ValidationResult validate( result.addErrors( String.format(TABLE_DOES_NOT_EXIST, context, relation.getJunction().get())); } else { - if (relation.getJunctionSource().isPresent() && relation.getJunction().isPresent()) { - if (!schemaInfo.columnExists( - relation.getJunctionSource().get(), relation.getJunction().get())) { - result.addErrors( - String.format( - COLUMN_DOES_NOT_EXIST, - context, - relation.getJunctionSource().get(), - relation.getJunction().get())); - } + if (relation.getJunctionSource().isPresent() + && relation.getJunction().isPresent() + && !schemaInfo.columnExists( + relation.getJunctionSource().get(), relation.getJunction().get())) { + result.addErrors( + String.format( + COLUMN_DOES_NOT_EXIST, + context, + relation.getJunctionSource().get(), + relation.getJunction().get())); } - if (relation.getJunctionTarget().isPresent() && relation.getJunction().isPresent()) { - if (!schemaInfo.columnExists( - relation.getJunctionTarget().get(), relation.getJunction().get())) { - result.addErrors( - String.format( - COLUMN_DOES_NOT_EXIST, - context, - relation.getJunctionTarget().get(), - relation.getJunction().get())); - } + if (relation.getJunctionTarget().isPresent() + && relation.getJunction().isPresent() + && !schemaInfo.columnExists( + relation.getJunctionTarget().get(), relation.getJunction().get())) { + result.addErrors( + String.format( + COLUMN_DOES_NOT_EXIST, + context, + relation.getJunctionTarget().get(), + relation.getJunction().get())); } } } - } else if (!schemaInfo.tableExists(tableSchema.getName())) { - String context = String.format("Invalid sourcePath in type '%s'", typeName); - result.addErrors(String.format(TABLE_DOES_NOT_EXIST, context, tableSchema.getName())); } ValidationResult intermediateResult = result.build(); @@ -141,21 +147,23 @@ private ValidationResult validate( } String context = String.format("Invalid sort key for type '%s'", typeName); - if (!schemaInfo.columnExists(tableSchema.getSortKey().get(), tableSchema.getName())) { + if (schemaInfo.columnExists(tableSchema.getSortKey().get(), tableSchema.getName())) { + if (!schemaInfo.isColumnUnique(tableSchema.getSortKey().get(), tableSchema.getName())) { + result.addStrictErrors( + String.format( + COLUMN_NOT_UNIQUE, + context, + tableSchema.getSortKey().get(), + tableSchema.getName(), + "sort key")); + } + } else { result.addErrors( String.format( COLUMN_DOES_NOT_EXIST, context, tableSchema.getSortKey().get(), tableSchema.getName())); - } else if (!schemaInfo.isColumnUnique(tableSchema.getSortKey().get(), tableSchema.getName())) { - result.addStrictErrors( - String.format( - COLUMN_NOT_UNIQUE, - context, - tableSchema.getSortKey().get(), - tableSchema.getName(), - "sort key")); } tableSchema @@ -168,14 +176,7 @@ private ValidationResult validate( "Invalid sourcePath for property '%s' in type '%s'", attribute.getSourcePath().orElse("???"), typeName); - if (!schemaInfo.columnExists(attribute.getName(), tableSchema.getName())) { - result.addErrors( - String.format( - COLUMN_DOES_NOT_EXIST, - context2, - attribute.getName(), - tableSchema.getName())); - } else { + if (schemaInfo.columnExists(attribute.getName(), tableSchema.getName())) { if (attribute.isId() && !schemaInfo.isColumnUnique(attribute.getName(), tableSchema.getName())) { String context3 = @@ -213,6 +214,13 @@ private ValidationResult validate( tableSchema.getName(), "datetime")); } + } else { + result.addErrors( + String.format( + COLUMN_DOES_NOT_EXIST, + context2, + attribute.getName(), + tableSchema.getName())); } } }); diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlClientRx.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlClientRx.java index 1afe2c7f9..93ced197f 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlClientRx.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlClientRx.java @@ -47,9 +47,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@SuppressWarnings("PMD.CouplingBetweenObjects") public class SqlClientRx implements SqlClient { private static final Logger LOGGER = LoggerFactory.getLogger(SqlClientRx.class); + private static final String SQL_LOG_EXECUTING = "Executing statement: {}"; private final Database session; private final SqlDbmsAdapter dbmsAdapter; @@ -70,7 +72,7 @@ public SqlClientRx( @Override public CompletableFuture> run(String query, SqlQueryOptions options) { if (LOGGER.isDebugEnabled(MARKER.SQL)) { - LOGGER.debug(MARKER.SQL, "Executing statement: {}", query); + LOGGER.debug(MARKER.SQL, SQL_LOG_EXECUTING, query); } CompletableFuture> result = new CompletableFuture<>(); @@ -93,9 +95,10 @@ public CompletableFuture> run(String query, SqlQueryOptions o } @Override + @SuppressWarnings("PMD.CognitiveComplexity") public Reactive.Source getSourceStream(String query, SqlQueryOptions options) { if (LOGGER.isDebugEnabled(MARKER.SQL)) { - LOGGER.debug(MARKER.SQL, "Executing statement: {}", query); + LOGGER.debug(MARKER.SQL, SQL_LOG_EXECUTING, query); } List logBuffer = new ArrayList<>(5); @@ -207,7 +210,7 @@ public Reactive.Source getMutationSource( // LOGGER.debug("VALUES {}", values); LOGGER.debug(""); } catch (SQLException e) { - e.printStackTrace(); + LOGGER.error("Failed to process generated key for a mutation statement", e); } i[0]++; @@ -216,7 +219,7 @@ public Reactive.Source getMutationSource( String first = statements.get(0).get(); if (LOGGER.isDebugEnabled(MARKER.SQL)) { - LOGGER.debug(MARKER.SQL, "Executing statement: {}", first); + LOGGER.debug(MARKER.SQL, SQL_LOG_EXECUTING, first); } Flowable> txFlowable = @@ -234,7 +237,7 @@ public Reactive.Source getMutationSource( tx -> { String next = statements.get(finalJ).get(); if (LOGGER.isDebugEnabled(MARKER.SQL)) { - LOGGER.debug(MARKER.SQL, "Executing statement: {}", next); + LOGGER.debug(MARKER.SQL, SQL_LOG_EXECUTING, next); } return tx.update(next) @@ -254,6 +257,7 @@ public Reactive.Source getMutationSource( } @Override + @SuppressWarnings("PMD.UnnecessaryCast") public Transformer getMutationFlow( Function>>>> mutations, Object executionContext, diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlConnectorRx.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlConnectorRx.java index 3a81c4864..de603e9c2 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlConnectorRx.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlConnectorRx.java @@ -21,7 +21,6 @@ import de.ii.xtraplatform.base.domain.LogContext; import de.ii.xtraplatform.base.domain.LogContext.MARKER; import de.ii.xtraplatform.base.domain.resiliency.AbstractVolatilePolling; -import de.ii.xtraplatform.base.domain.resiliency.Volatile2.Polling; import de.ii.xtraplatform.base.domain.resiliency.VolatileRegistry; import de.ii.xtraplatform.base.domain.resiliency.VolatileRegistry.ChangeHandler; import de.ii.xtraplatform.base.domain.resiliency.VolatileUnavailableException; @@ -38,7 +37,6 @@ import de.ii.xtraplatform.features.sql.domain.SqlRow; import de.ii.xtraplatform.streams.domain.Reactive.Source; import io.reactivex.rxjava3.plugins.RxJavaPlugins; -import java.nio.file.Path; import java.sql.Connection; import java.sql.SQLException; import java.time.Duration; @@ -59,10 +57,10 @@ /** * @author zahnen */ -public class SqlConnectorRx extends AbstractVolatilePolling implements SqlConnector, Polling { +@SuppressWarnings({"PMD.CouplingBetweenObjects", "PMD.GodClass"}) +public class SqlConnectorRx extends AbstractVolatilePolling implements SqlConnector { public static final String CONNECTOR_TYPE = "SLICK"; - private static final SqlQueryOptions NO_OPTIONS = SqlQueryOptions.withColumnTypes(String.class); private static final Logger LOGGER = LoggerFactory.getLogger(SqlConnectorRx.class); private final SqlDbmsAdapters dbmsAdapters; @@ -74,8 +72,6 @@ public class SqlConnectorRx extends AbstractVolatilePolling implements SqlConnec private final int minConnections; private final int queueSize; private final Semaphore connectionBudget; - private final Path dataDir; - private final String applicationName; private final String providerId; private final AtomicInteger refCounter; private final boolean asyncStartup; @@ -113,12 +109,9 @@ public SqlConnectorRx( // TODO this.queueSize = 1024; // Math.max(1024, maxConnections * capacity * 2); - this.dataDir = appContext.getDataDir(); // LOGGER.debug("QUEUE {} {} {} {} {}", connectionInfo.getDatabase(), maxQueries, // maxConnections, capacity, maxConnections * capacity * 2); - this.applicationName = - String.format("%s %s - %s", appContext.getName(), appContext.getVersion(), providerId); this.providerId = providerId; this.refCounter = new AtomicInteger(0); this.asyncStartup = appContext.getConfiguration().getModules().isStartupAsync(); @@ -162,6 +155,7 @@ public String getDialect() { } @Override + @SuppressWarnings("PMD.AvoidCatchingGenericException") public void start() { try { HikariConfig hikariConfig = createHikariConfig(); @@ -180,6 +174,7 @@ public void start() { } @Override + @SuppressWarnings({"PMD.AvoidCatchingGenericException", "PMD.NullAssignment"}) public void stop() { getVolatileRegistry().unregister(this); @@ -360,7 +355,7 @@ private static long getInitFailTimeout(ConnectionInfoSql connectionInfo) { private static long parseMs(String duration) { try { return Long.parseLong(duration) * 1000; - } catch (Throwable e) { + } catch (NumberFormatException e) { // ignore } return Duration.parse("PT" + duration).toMillis(); @@ -399,21 +394,24 @@ public static ChangeHandler withMdc(ChangeHandler consumer) { return consumer; } + // Kept method-level (rather than a dedicated lock object) to stay consistent with the monitor + // AbstractVolatilePolling itself synchronizes on for this instance. @Override + @SuppressWarnings("PMD.AvoidSynchronizedAtMethodLevel") protected synchronized void onVolatileStart() { super.onVolatileStart(); if (asyncStartup) { - if (getState() == State.UNAVAILABLE) { + if (getState() == State.UNAVAILABLE && LOGGER.isWarnEnabled()) { LOGGER.warn("Could not establish connection to database: {}", getDatasetIdentifier()); } onStateChange( withMdc( (from, to) -> { - if (to == State.AVAILABLE) { + if (to == State.AVAILABLE && LOGGER.isInfoEnabled()) { LOGGER.info("Re-established connection to database: {}", getDatasetIdentifier()); - } else if (to == State.UNAVAILABLE) { + } else if (to == State.UNAVAILABLE && LOGGER.isWarnEnabled()) { LOGGER.warn("Lost connection to database: {}", getDatasetIdentifier()); } }), @@ -430,6 +428,12 @@ public Source getSourceStream(SqlQueryBatch queryBatch, SqlQueryOptions } @Override + @SuppressWarnings({ + "PMD.CognitiveComplexity", + "PMD.CyclomaticComplexity", + "PMD.AvoidCatchingGenericException", + "PMD.NullAssignment" + }) public de.ii.xtraplatform.base.domain.util.Tuple check() { if (Objects.isNull(sqlClient)) { // TODO: retry diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlConnectorRxFactory.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlConnectorRxFactory.java index e0296bc01..508bfafb2 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlConnectorRxFactory.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlConnectorRxFactory.java @@ -97,6 +97,7 @@ public boolean deleteInstance(String id) { } @AssistedFactory + @FunctionalInterface public interface FactoryAssisted { SqlConnectorRx create( MetricRegistry metricRegistry, diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlDbmsAdapterGpkg.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlDbmsAdapterGpkg.java index ba6b82a4b..96913bdda 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlDbmsAdapterGpkg.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlDbmsAdapterGpkg.java @@ -43,6 +43,7 @@ @Singleton @AutoBind +@SuppressWarnings("PMD.CouplingBetweenObjects") public class SqlDbmsAdapterGpkg implements SqlDbmsAdapter { public static final String ID = "GPKG"; @@ -76,24 +77,24 @@ public SqlDialect getDialect() { public DataSource createDataSource(String providerId, ConnectionInfoSql connectionInfo) { Path source = Path.of(connectionInfo.getDatabase()); - if (!source.isAbsolute()) { - Optional localPath = Optional.empty(); - try { - localPath = featuresStore.asLocalPath(source, false); - } catch (IOException e) { - // continue - } - if (localPath.isPresent()) { - source = localPath.get(); - } else { - throw new IllegalStateException("GPKG database not found: " + source); - } - } else { + if (source.isAbsolute()) { throw new IllegalStateException( "GPKG database reference must be a path relative to resources/features. Found: " + source); } + Optional localPath = Optional.empty(); + try { + localPath = featuresStore.asLocalPath(source, false); + } catch (IOException e) { + // continue + } + if (localPath.isPresent()) { + source = localPath.get(); + } else { + throw new IllegalStateException("GPKG database not found: " + source); + } + if (!spatiaLiteInitialized && Objects.nonNull(spatiaLiteLoader)) { spatiaLiteLoader.load(); @@ -108,7 +109,7 @@ public SQLiteConnection getConnection(String username, String password) SQLiteConnection connection = super.getConnection(username, password); if (Objects.nonNull(spatiaLiteLoader)) { - try (var statement = connection.createStatement()) { + try (Statement statement = connection.createStatement()) { // connection was created a few milliseconds before, so set query timeout is omitted // (we assume it will succeed) statement.execute( @@ -181,31 +182,32 @@ public Map getGeoInfo(Connection connection, DbInfo dbInfo) thr "SELECT f_table_name AS \"%s\", f_geometry_column AS \"%s\", coord_dimension AS \"%s\", srid AS \"%s\", geometry_type AS \"%s\" FROM geometry_columns;", GeoInfo.TABLE, GeoInfo.COLUMN, GeoInfo.DIMENSION, GeoInfo.SRID, GeoInfo.TYPE); - Statement stmt = connection.createStatement(); - ResultSet rs = stmt.executeQuery(query); - Map result = new LinkedHashMap<>(); - - while (rs.next()) { - String table = rs.getString(GeoInfo.TABLE); - String tableKey = table.toLowerCase(Locale.ROOT); - String schemaTableKey = String.format("%s.%s", "main", table).toLowerCase(Locale.ROOT); - GeoInfo geoInfo = - ImmutableGeoInfo.of( - null, - table, - rs.getString(GeoInfo.COLUMN), - rs.getString(GeoInfo.DIMENSION), - rs.getString(GeoInfo.SRID), - forceAxisOrder((DbInfoGpkg) dbInfo).name(), - rs.getString(GeoInfo.TYPE)); - - // keep a normalized table-only key as canonical lookup key - result.put(tableKey, geoInfo); - // additionally provide normalized schema.table compatibility key - result.put(schemaTableKey, geoInfo); - } + try (Statement stmt = connection.createStatement(); + ResultSet rs = stmt.executeQuery(query)) { + Map result = new LinkedHashMap<>(); + + while (rs.next()) { + String table = rs.getString(GeoInfo.TABLE); + String tableKey = table.toLowerCase(Locale.ROOT); + String schemaTableKey = String.format("%s.%s", "main", table).toLowerCase(Locale.ROOT); + GeoInfo geoInfo = + ImmutableGeoInfo.of( + null, + table, + rs.getString(GeoInfo.COLUMN), + rs.getString(GeoInfo.DIMENSION), + rs.getString(GeoInfo.SRID), + forceAxisOrder((DbInfoGpkg) dbInfo).name(), + rs.getString(GeoInfo.TYPE)); + + // keep a normalized table-only key as canonical lookup key + result.put(tableKey, geoInfo); + // additionally provide normalized schema.table compatibility key + result.put(schemaTableKey, geoInfo); + } - return result; + return result; + } } @Override @@ -218,12 +220,15 @@ public DbInfo getDbInfo(Connection connection) throws SQLException { String query = "SELECT sqlite_version(),spatialite_version(),CASE CheckSpatialMetaData() WHEN 4 THEN 'GPKG' WHEN 3 THEN 'SPATIALITE' ELSE 'UNSUPPORTED' END;"; - Statement stmt = connection.createStatement(); - ResultSet rs = stmt.executeQuery(query); - rs.next(); + try (Statement stmt = connection.createStatement(); + ResultSet rs = stmt.executeQuery(query)) { + if (!rs.next()) { + throw new SQLException("Could not determine GeoPackage/SpatiaLite version info."); + } - return ImmutableDbInfoGpkg.of( - rs.getString(1), rs.getString(2), SpatialMetadata.valueOf(rs.getString(3))); + return ImmutableDbInfoGpkg.of( + rs.getString(1), rs.getString(2), SpatialMetadata.valueOf(rs.getString(3))); + } } @Override diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlDbmsAdapterPgis.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlDbmsAdapterPgis.java index e644ccdc0..a1a64ea0d 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlDbmsAdapterPgis.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlDbmsAdapterPgis.java @@ -25,6 +25,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.text.Collator; +import java.util.IllformedLocaleException; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; @@ -113,6 +114,8 @@ public DataSource createDataSource(String providerId, ConnectionInfoSql connecti case "sslpassword": ds.setSslPassword(String.valueOf(value)); break; + default: + break; } }); @@ -127,7 +130,7 @@ public Optional getInitSql(ConnectionInfoSql connectionInfo) { connectionInfo.getSchemas().stream() .map( schema -> { - if (!Objects.equals(schema, schema.toLowerCase())) { + if (!Objects.equals(schema, schema.toLowerCase(Locale.ROOT))) { return String.format("\"%s\"", schema); } return schema; @@ -179,39 +182,43 @@ public Map getGeoInfo(Connection connection, DbInfo dbInfo) thr GeoInfo.SRID, GeoInfo.TYPE); - Statement stmt = connection.createStatement(); - ResultSet rs = stmt.executeQuery(query); - Map result = new LinkedHashMap<>(); - - while (rs.next()) { - String schema = rs.getString(GeoInfo.SCHEMA); - String table = rs.getString(GeoInfo.TABLE); - String key = String.format("%s.%s", schema, table).toLowerCase(Locale.ROOT); - - result.put( - key, - ImmutableGeoInfo.of( - schema, - table, - rs.getString(GeoInfo.COLUMN), - rs.getString(GeoInfo.DIMENSION), - rs.getString(GeoInfo.SRID), - Force.NONE.name(), - rs.getString(GeoInfo.TYPE))); + try (Statement stmt = connection.createStatement(); + ResultSet rs = stmt.executeQuery(query)) { + Map result = new LinkedHashMap<>(); + + while (rs.next()) { + String schema = rs.getString(GeoInfo.SCHEMA); + String table = rs.getString(GeoInfo.TABLE); + String key = String.format("%s.%s", schema, table).toLowerCase(Locale.ROOT); + + result.put( + key, + ImmutableGeoInfo.of( + schema, + table, + rs.getString(GeoInfo.COLUMN), + rs.getString(GeoInfo.DIMENSION), + rs.getString(GeoInfo.SRID), + Force.NONE.name(), + rs.getString(GeoInfo.TYPE))); + } + + return result; } - - return result; } @Override public DbInfo getDbInfo(Connection connection) throws SQLException { String query = "SELECT version(), PostGIS_Lib_Version();"; - Statement stmt = connection.createStatement(); - ResultSet rs = stmt.executeQuery(query); - rs.next(); + try (Statement stmt = connection.createStatement(); + ResultSet rs = stmt.executeQuery(query)) { + if (!rs.next()) { + throw new SQLException("Could not determine PostgreSQL/PostGIS version info."); + } - return ImmutableDbInfoPgis.of(rs.getString(1), rs.getString(2)); + return ImmutableDbInfoPgis.of(rs.getString(1), rs.getString(2)); + } } @Override @@ -225,14 +232,16 @@ public Collator getRowSortingCollator(Optional defaultCollation) { Locale locale; try { locale = new Builder().setLanguageTag(languageTag).build(); - } catch (Exception e) { + } catch (IllformedLocaleException e) { locale = Locale.US; - LOGGER.warn( - "Invalid default collation '{}', falling back to '{}' for sorting: {}", - languageTag, - Locale.US.toLanguageTag(), - e.getMessage()); + if (LOGGER.isWarnEnabled()) { + LOGGER.warn( + "Invalid default collation '{}', falling back to '{}' for sorting: {}", + languageTag, + Locale.US.toLanguageTag(), + e.getMessage()); + } } return Collator.getInstance(locale); diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlRowVals.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlRowVals.java index a2ab58816..96f5d8c20 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlRowVals.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlRowVals.java @@ -34,6 +34,12 @@ import org.slf4j.LoggerFactory; // TODO: extensive unit tests for compareTo +// compareTo is used purely for sort-merge ordering in the query pipeline; instances are never +// used in equals()-based collections or hashed, so no equals()/hashCode() are provided. +@SuppressWarnings({ + "PMD.AvoidCatchingGenericException", + "PMD.OverrideBothEqualsAndHashCodeOnComparable" +}) class SqlRowVals implements SqlRow { private static final Logger LOGGER = LoggerFactory.getLogger(SqlRowVals.class); @@ -160,6 +166,12 @@ public int getSchemaIndex(int i) { } // TODO: use result.nextObject when column type info is supported + @SuppressWarnings({ + "PMD.CognitiveComplexity", + "PMD.CyclomaticComplexity", + "PMD.AvoidDeeplyNestedIfStmts", + "PMD.ExceptionAsFlowControl" + }) SqlRow read(ResultSet result, SqlQueryOptions queryOptions) { this.priority = queryOptions.getContainerPriority(); this.queryIndex = queryOptions.getQueryIndex(); @@ -175,8 +187,10 @@ SqlRow read(ResultSet result, SqlQueryOptions queryOptions) { columnTypes = queryOptions.getColumnTypes(); for (int i = 0; i < sortKeyNames.size(); i++) { + int currentCursor = cursor; + cursor++; try { - Object id = result.getObject(cursor++); + Object id = result.getObject(currentCursor); if (Objects.isNull(id)) { sortKeys.add(null); if (i >= queryOptions.getCustomSortKeys().size()) { @@ -203,9 +217,11 @@ SqlRow read(ResultSet result, SqlQueryOptions queryOptions) { columnTypes = queryOptions.getColumnTypes(); } - for (int i = 0; i < columnTypes.size(); i++) { + for (Class columnType : columnTypes) { + int currentCursor = cursor; + cursor++; try { - values.add(getValue(result, cursor++, columnTypes.get(i))); + values.add(getValue(result, currentCursor, columnType)); } catch (Throwable e) { break; } @@ -214,26 +230,58 @@ SqlRow read(ResultSet result, SqlQueryOptions queryOptions) { return this; } + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) private Object getValue(ResultSet result, int cursor, Class type) throws SQLException { - if (type == BigDecimal.class) return result.getBigDecimal(cursor); - if (type == Blob.class) return result.getBlob(cursor); - if (type == Byte.class) return result.getByte(cursor); - if (type == byte[].class) return result.getBytes(cursor); - if (type == Clob.class) return result.getClob(cursor); - if (type == Date.class) return result.getDate(cursor); - if (type == Double.class) return result.getDouble(cursor); - if (type == Float.class) return result.getFloat(cursor); - if (type == Integer.class) return result.getInt(cursor); - if (type == Long.class) return result.getLong(cursor); - if (type == Object.class) return result.getObject(cursor); - if (type == Short.class) return result.getShort(cursor); - if (type == String.class) return result.getString(cursor); - if (type == Time.class) return result.getTime(cursor); - if (type == Timestamp.class) return result.getTimestamp(cursor); + if (type == BigDecimal.class) { + return result.getBigDecimal(cursor); + } + if (type == Blob.class) { + return result.getBlob(cursor); + } + if (type == Byte.class) { + return result.getByte(cursor); + } + if (type == byte[].class) { + return result.getBytes(cursor); + } + if (type == Clob.class) { + return result.getClob(cursor); + } + if (type == Date.class) { + return result.getDate(cursor); + } + if (type == Double.class) { + return result.getDouble(cursor); + } + if (type == Float.class) { + return result.getFloat(cursor); + } + if (type == Integer.class) { + return result.getInt(cursor); + } + if (type == Long.class) { + return result.getLong(cursor); + } + if (type == Object.class) { + return result.getObject(cursor); + } + if (type == Short.class) { + return result.getShort(cursor); + } + if (type == String.class) { + return result.getString(cursor); + } + if (type == Time.class) { + return result.getTime(cursor); + } + if (type == Timestamp.class) { + return result.getTimestamp(cursor); + } return result.getString(cursor); } + @SuppressWarnings("PMD.NullAssignment") void clear() { this.values.clear(); this.ids.clear(); @@ -282,6 +330,7 @@ private static int getNumberOfCommonElements(List list1, List li return size; } + @SuppressWarnings({"PMD.CognitiveComplexity", "PMD.CyclomaticComplexity"}) private static int compareSortKeys( List> ids1, List> ids2, @@ -289,7 +338,7 @@ private static int compareSortKeys( List idColumnDirections, Collator collator) { for (int i = 0; i < numberOfIds; i++) { - int result = 0; + int result; Comparable id1 = ids1.get(i); Comparable id2 = ids2.get(i); int direction = idColumnDirections.get(i) == Direction.DESCENDING ? -1 : 1; diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlSchemaCrawler.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlSchemaCrawler.java index 1e92633dd..c53e7e0c4 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlSchemaCrawler.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/SqlSchemaCrawler.java @@ -28,7 +28,6 @@ import schemacrawler.schemacrawler.SchemaCrawlerOptions; import schemacrawler.schemacrawler.SchemaCrawlerOptionsBuilder; import schemacrawler.schemacrawler.SchemaInfoLevelBuilder; -import schemacrawler.schemacrawler.exceptions.SchemaCrawlerException; import schemacrawler.tools.utility.SchemaCrawlerUtility; import us.fatehi.utility.datasource.DatabaseConnectionSource; @@ -40,26 +39,23 @@ public SqlSchemaCrawler(Connection connection) { this.connection = new SingleDatabaseConnectionSource(connection); } - public Catalog getCatalog(List excludeSchemas, List excludeTables) - throws SchemaCrawlerException { + public Catalog getCatalog(List excludeSchemas, List excludeTables) { return crawlSchemasAndTables(excludeSchemas, excludeTables); } - public Catalog getCatalog(String schema, String table) throws SchemaCrawlerException { + public Catalog getCatalog(String schema, String table) { return getCatalogAndMatching( schema.isEmpty() ? List.of() : List.of(schema), List.of(table), List.of()) .first(); } public Catalog getCatalog( - List schemas, List includeTables, List excludeTables) - throws SchemaCrawlerException { + List schemas, List includeTables, List excludeTables) { return getCatalogAndMatching(schemas, includeTables, excludeTables).first(); } public Tuple> getCatalogAndMatching( - List schemas, List includeTables, List excludeTables) - throws SchemaCrawlerException { + List schemas, List includeTables, List excludeTables) { Catalog catalog = crawlWithDetails(schemas, includeTables, excludeTables); List matchingTables = catalog.getTables().stream().map(Table::getName).collect(Collectors.toList()); @@ -84,8 +80,7 @@ public Tuple> getCatalogAndMatching( } private Catalog crawlWithDetails( - List schemas, List includeTables, List excludeTables) - throws SchemaCrawlerException { + List schemas, List includeTables, List excludeTables) { String includeSchemas = schemas.stream().distinct().collect(Collectors.joining("|", "(", ")")); Collector tableCollector = @@ -118,8 +113,7 @@ private Catalog crawlWithDetails( return SchemaCrawlerUtility.getCatalog(connection, options); } - private Catalog crawlSchemasAndTables(List excludeSchemas, List excludeTables) - throws SchemaCrawlerException { + private Catalog crawlSchemasAndTables(List excludeSchemas, List excludeTables) { LimitOptionsBuilder limitOptionsBuilder = LimitOptionsBuilder.builder() .tableTypes("BASE TABLE", "TABLE", "VIEW", "MATERIALIZED VIEW"); @@ -147,6 +141,7 @@ private Catalog crawlSchemasAndTables(List excludeSchemas, List } @Override + @SuppressWarnings("PMD.AvoidCatchingGenericException") public void close() throws IOException { try { connection.close(); diff --git a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/ViewInfo.java b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/ViewInfo.java index c31ee954b..fdbedba0a 100644 --- a/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/ViewInfo.java +++ b/xtraplatform-features-sql/src/main/java/de/ii/xtraplatform/features/sql/infra/db/ViewInfo.java @@ -32,7 +32,9 @@ import net.sf.jsqlparser.statement.select.SelectItemVisitorAdapter; import net.sf.jsqlparser.util.TablesNamesFinder; -public class ViewInfo { +public final class ViewInfo { + + private ViewInfo() {} public static List getOriginalTables(String viewDefinition) { if (Objects.isNull(viewDefinition)) { @@ -42,7 +44,7 @@ public static List getOriginalTables(String viewDefinition) { PlainSelect select = parse(viewDefinition); TablesNamesFinder tablesNamesFinder = - new TablesNamesFinder() { + new TablesNamesFinder<>() { @Override protected String extractTableName(Table table) { return table.getName(); @@ -61,7 +63,6 @@ public T accept(StatementVisitor statementVisitor, S s) { } catch (JSQLParserException | ParseException e) { // ignore - boolean br = true; } return ImmutableList.of(); @@ -80,7 +81,6 @@ public static Optional> getOriginalTableAndColumn( } catch (JSQLParserException | ParseException e) { // ignore - boolean br = true; } return Optional.empty(); @@ -91,28 +91,29 @@ private static Optional> getOriginalTableAndColumn( ImmutableTuple.Builder builder = ImmutableTuple.builder().second(columnName); - for (SelectItem selectItem : select.getSelectItems()) { - selectItem.accept( - new SelectItemVisitorAdapter() { - @Override - public void visit(SelectItem item) { - if (item.getExpression() instanceof Column) { - Column column = (Column) item.getExpression(); - - if (Objects.nonNull(item.getAlias()) - && Objects.equals(item.getAlias().getName(), columnName)) { - builder.first(column.getTable().getName()).second(column.getColumnName()); - } else if (Objects.equals(column.getColumnName(), columnName)) { - builder.first(column.getTable().getName()); - } + SelectItemVisitorAdapter visitor = + new SelectItemVisitorAdapter<>() { + @Override + public void visit(SelectItem item) { + if (item.getExpression() instanceof Column) { + Column column = (Column) item.getExpression(); + + if (Objects.nonNull(item.getAlias()) + && Objects.equals(item.getAlias().getName(), columnName)) { + builder.first(column.getTable().getName()).second(column.getColumnName()); + } else if (Objects.equals(column.getColumnName(), columnName)) { + builder.first(column.getTable().getName()); } } - }, - null); + } + }; + + for (SelectItem selectItem : select.getSelectItems()) { + selectItem.accept(visitor, null); } try { return Optional.of(builder.build()); - } catch (Throwable e) { + } catch (IllegalStateException e) { // column not found } @@ -147,7 +148,7 @@ public void visit(ParenthesedFromItem subjoin) { } private static PlainSelect parse(String select) throws JSQLParserException, ParseException { - net.sf.jsqlparser.statement.Statement parsed = + Statement parsed = CCJSqlParserUtil.parse(select, ccjSqlParser -> ccjSqlParser.setErrorRecovery(true)); Select statement; diff --git a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/FeatureMutationsSqlSpec.groovy b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/FeatureMutationsSqlSpec.groovy index fd081f9c9..a9ed85898 100644 --- a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/FeatureMutationsSqlSpec.groovy +++ b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/FeatureMutationsSqlSpec.groovy @@ -9,7 +9,6 @@ package de.ii.xtraplatform.features.sql.app import com.google.common.collect.ImmutableList import com.google.common.collect.ImmutableMap -import de.ii.xtraplatform.crs.domain.OgcCrs import de.ii.xtraplatform.features.sql.domain.ImmutableSqlPathDefaults import org.slf4j.Logger import org.slf4j.LoggerFactory @@ -33,7 +32,7 @@ class FeatureMutationsSqlSpec extends Specification { given: - FeatureMutationsSql inserts = Spy(new FeatureMutationsSql(null, new SqlInsertGenerator2(OgcCrs.CRS84, null, new ImmutableSqlPathDefaults.Builder().build()),new ImmutableSqlPathDefaults.Builder().build())) + FeatureMutationsSql inserts = Spy(new FeatureMutationsSql(null, new SqlInsertGenerator2(new ImmutableSqlPathDefaults.Builder().build()))) Map, List> rows = ImmutableMap., List> builder() .put(MAIN_M_2_N_SCHEMA.getFullPath(), ImmutableList.of(3)) @@ -85,7 +84,7 @@ class FeatureMutationsSqlSpec extends Specification { given: FeatureStoreInsertGenerator generator = Mock(); - FeatureMutationsSql inserts = new FeatureMutationsSql(null, generator,null) + FeatureMutationsSql inserts = new FeatureMutationsSql(null, generator) List rows = ImmutableList.of(0, 0, 1) when: @@ -104,7 +103,7 @@ class FeatureMutationsSqlSpec extends Specification { given: FeatureStoreInsertGenerator generator = Mock(); - FeatureMutationsSql inserts = new FeatureMutationsSql(null, generator, null) + FeatureMutationsSql inserts = new FeatureMutationsSql(null, generator) List rows = ImmutableList.of(0, 0, 0, 1) when: @@ -127,7 +126,7 @@ class FeatureMutationsSqlSpec extends Specification { given: FeatureStoreInsertGenerator generator = Mock(); - FeatureMutationsSql inserts = new FeatureMutationsSql(null, generator, null) + FeatureMutationsSql inserts = new FeatureMutationsSql(null, generator) List rows = ImmutableList.of(0, 0, 0, 1) when: diff --git a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/MutationSchemaDeriverSpec.groovy b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/MutationSchemaDeriverSpec.groovy index cd768baae..0c08089f5 100644 --- a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/MutationSchemaDeriverSpec.groovy +++ b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/MutationSchemaDeriverSpec.groovy @@ -26,7 +26,7 @@ class MutationSchemaDeriverSpec extends Specification { def defaults = new ImmutableSqlPathDefaults.Builder().build() def cql = new CqlImpl() - pathParser = new PathParserSql(syntax, cql) + pathParser = new PathParserSql(syntax) pathParser2 = new SqlPathParser(defaults, cql, Set.of()) schemaBuilderSql = new MutationSchemaDeriver(pathParser, pathParser2) } diff --git a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/QuerySchemaDeriverSpec.groovy b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/QuerySchemaDeriverSpec.groovy index fb0cd522d..21db36411 100644 --- a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/QuerySchemaDeriverSpec.groovy +++ b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/QuerySchemaDeriverSpec.groovy @@ -31,7 +31,7 @@ class QuerySchemaDeriverSpec extends Specification { def defaults = new ImmutableSqlPathDefaults.Builder().build() def cql = new CqlImpl() def pathParser = new SqlPathParser(defaults, cql, Map.of("JSON", new DecoderFactoryJson())) - def pathParser2 = new PathParserSql(ImmutableSqlPathSyntax.builder().options(defaults).build(), cql) + def pathParser2 = new PathParserSql(ImmutableSqlPathSyntax.builder().options(defaults).build()) schemaDeriver = new QuerySchemaDeriver(pathParser) schemaDeriver2 = new MutationSchemaDeriver(pathParser2, pathParser) mappingOperationResolver = new MappingOperationResolver() diff --git a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlInsertGeneratorSpec2.groovy b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlInsertGeneratorSpec2.groovy index a70b8ea3f..5e1de3ed0 100644 --- a/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlInsertGeneratorSpec2.groovy +++ b/xtraplatform-features-sql/src/test/groovy/de/ii/xtraplatform/features/sql/app/SqlInsertGeneratorSpec2.groovy @@ -36,7 +36,7 @@ class SqlInsertGeneratorSpec2 extends Specification { given: - SqlInsertGenerator2 inserts = new SqlInsertGenerator2(null, null, sqlPathDefaults); + SqlInsertGenerator2 inserts = new SqlInsertGenerator2(sqlPathDefaults); when: @@ -68,7 +68,7 @@ class SqlInsertGeneratorSpec2 extends Specification { given: - SqlInsertGenerator2 inserts = new SqlInsertGenerator2(null, null, sqlPathDefaults); + SqlInsertGenerator2 inserts = new SqlInsertGenerator2(sqlPathDefaults); SchemaSql schema = MERGE_MERGE_ONE_2_ONE_SCHEMA when: @@ -86,7 +86,7 @@ class SqlInsertGeneratorSpec2 extends Specification { given: - SqlInsertGenerator2 inserts = new SqlInsertGenerator2(null, null, sqlPathDefaults); + SqlInsertGenerator2 inserts = new SqlInsertGenerator2(sqlPathDefaults); SchemaSql schema = MERGE_MERGE_M_2_N_SCHEMA when: