From ab36537c324662dda9904e4a6620d7b9a84e5ad7 Mon Sep 17 00:00:00 2001 From: Pavel Horal Date: Tue, 6 Feb 2024 12:43:49 +0100 Subject: [PATCH] Add support for COUNT queries and NUMBER and BOOLEAN property mapping --- .../openidm/repo/jdbc/DatabaseType.java | 3 +- .../openidm/repo/jdbc/TableHandler.java | 34 +--- .../openidm/repo/jdbc/impl/Activator.java | 1 - .../repo/jdbc/impl/JDBCRepoService.java | 155 +++++++----------- .../jdbc/impl/handler/MappedColumnConfig.java | 50 +++++- .../jdbc/impl/handler/MappedTableHandler.java | 2 +- .../jdbc/impl/mapper/MappedResultMapper.java | 2 + .../query/MappedSQLQueryFilterVisitor.java | 35 ++-- .../impl/vendor/DB2MappedTableHandler.java | 18 +- .../impl/vendor/H2MappedTableHandler.java | 18 ++ .../impl/vendor/MySQLMappedTableHandler.java | 18 ++ .../impl/vendor/OracleMappedTableHandler.java | 18 ++ .../vendor/PostgreSQLMappedTableHandler.java | 19 +++ .../AbstractMappedTableHandlerTest.java | 8 +- .../handler/AbstractTableHandlerTest.java | 20 +-- .../impl/statement/NamedParameterSqlTest.java | 2 - .../statement/NamedParameterSupportTest.java | 2 - .../vendor/OracleMappedTableHandlerIT.java | 17 ++ .../src/test/resources/hsqldb.sql | 5 +- .../src/test/resources/vendor/db2.sql | 5 +- .../src/test/resources/vendor/h2.sql | 5 +- .../src/test/resources/vendor/mssql.sql | 5 +- .../src/test/resources/vendor/mysql.sql | 5 +- .../src/test/resources/vendor/oracle.sql | 5 +- .../src/test/resources/vendor/postgresql.sql | 5 +- 25 files changed, 270 insertions(+), 187 deletions(-) diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/DatabaseType.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/DatabaseType.java index 73f63de31..3f5ffd4fd 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/DatabaseType.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/DatabaseType.java @@ -2,6 +2,7 @@ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER. * * Copyright © 2011-2013 ForgeRock AS. All rights reserved. + * Portions Copyright Wren Security 2024 * * The contents of this file are subject to the terms * of the Common Development and Distribution License @@ -27,5 +28,5 @@ * Supported database types. */ public enum DatabaseType { - SQLSERVER, MYSQL, POSTGRESQL, ORACLE, DB2, H2, ANSI_SQL99, ODBC; + SQLSERVER, MYSQL, POSTGRESQL, ORACLE, DB2, H2, ANSI_SQL99; } diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/TableHandler.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/TableHandler.java index 78b5ae78b..22b67cd0e 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/TableHandler.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/TableHandler.java @@ -29,7 +29,6 @@ import java.sql.SQLException; import java.util.List; import java.util.Map; -import org.forgerock.json.JsonPointer; import org.forgerock.json.resource.BadRequestException; import org.forgerock.json.resource.InternalServerErrorException; import org.forgerock.json.resource.NotFoundException; @@ -37,7 +36,6 @@ import org.forgerock.json.resource.ResourceException; import org.forgerock.json.resource.ResourceResponse; import org.forgerock.openidm.repo.QueryConstants; -import org.forgerock.util.query.QueryFilter; /** * Handler responsible for performing SQL operations on the underlying data source. @@ -187,10 +185,8 @@ List> query(String type, Map params, Connect * @throws InternalServerErrorException if the operation failed because of a (possibly transient) failure * @throws SQLException if a DB failure is reported */ - default Integer queryCount(String type, Map params, Connection connection) - throws SQLException, ResourceException { - throw new UnsupportedOperationException(); // TODO remove default after dropping legacy handlers - } + Integer queryCount(String type, Map params, Connection connection) + throws SQLException, ResourceException; /** * Perform the command on the specified target and return the number of affected objects. @@ -211,32 +207,6 @@ default Integer queryCount(String type, Map params, Connection c Integer command(String type, Map params, Connection connection) throws SQLException, ResourceException; - /** - * Build a raw query from the supplied filter. - * - * @param filter the query filter - * @param replacementTokens a map to store any replacement tokens - * @param params a map containing query parameters - * @param count whether to render a query for total number of matched rows - * @return the raw query string - */ - @Deprecated - default String renderQueryFilter(QueryFilter filter, Map replacementTokens, - Map params) { - throw new UnsupportedOperationException(); - } - - /** - * Check if a given queryId exists in our set of known queries - * - * @param queryId Identifier for the query - * @return true if queryId is available - */ - @Deprecated - default boolean queryIdExists(final String queryId) { - throw new UnsupportedOperationException(); - } - /** * Check if a given exception signifies a well known error type. * diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/Activator.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/Activator.java index 31b8aca59..4d2e305cc 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/Activator.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/Activator.java @@ -116,7 +116,6 @@ private String getDbDirname(JsonValue repoConfig) { case H2: return databaseType.toString().toLowerCase(); case ANSI_SQL99: - case ODBC: default: return null; } diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/JDBCRepoService.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/JDBCRepoService.java index 2491ddcbd..fb1050e2a 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/JDBCRepoService.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/JDBCRepoService.java @@ -20,7 +20,6 @@ import static org.forgerock.json.JsonValue.json; import static org.forgerock.json.JsonValue.object; import static org.forgerock.json.JsonValueFunctions.enumConstant; -import static org.forgerock.json.resource.QueryResponse.NO_COUNT; import static org.forgerock.json.resource.ResourceException.newResourceException; import static org.forgerock.json.resource.ResourceResponse.FIELD_CONTENT_ID; import static org.forgerock.json.resource.ResourceResponse.FIELD_CONTENT_REVISION; @@ -33,7 +32,6 @@ import static org.forgerock.openidm.repo.QueryConstants.QUERY_FILTER; import static org.forgerock.openidm.repo.QueryConstants.QUERY_ID; import static org.forgerock.openidm.repo.QueryConstants.SORT_KEYS; -import static org.wrensecurity.guava.common.base.Strings.isNullOrEmpty; import java.io.IOException; import java.sql.Connection; @@ -583,105 +581,37 @@ public Promise handlePatch(Context context, @Override public Promise handleQuery(Context context, QueryRequest request, QueryResourceHandler handler) { try { - - // If paged results are requested then decode the cookie in order to determine - // the index of the first result to be returned. - final int requestPageSize = request.getPageSize(); - - // Cookie containing offset of last request - final String pagedResultsCookie = request.getPagedResultsCookie(); - - final boolean pagedResultsRequested = requestPageSize > 0; - - // index of first record (used for SKIP/OFFSET) - final int firstResultIndex; - - if (pagedResultsRequested) { - if (!isNullOrEmpty(pagedResultsCookie)) { - try { - firstResultIndex = Integer.parseInt(pagedResultsCookie); - } catch (final NumberFormatException e) { - throw new BadRequestException("Invalid paged results cookie"); - } - } else { - firstResultIndex = Math.max(0, request.getPagedResultsOffset()); - } - } else { - firstResultIndex = 0; - } - - // Once cookie is processed Queries.query() can rely on the offset. - request.setPagedResultsOffset(firstResultIndex); + String type = trimStartingSlash(request.getResourcePath()); + Map params = createQueryParams(request); List results = query(request); for (ResourceResponse result : results) { handler.handleResource(result); } - /* - * Execute additional -count query if we are paging - */ - final String nextCookie; - - // The number of results (if known) - final int resultCount; - - if (pagedResultsRequested) { - TableHandler tableHandler = getTableHandler(trimStartingSlash(request.getResourcePath())); - - // count if requested - switch (request.getTotalPagedResultsPolicy()) { - case ESTIMATE: - case EXACT: - // Get total if -count query is available - final String countQueryId = request.getQueryId() + "-count"; - if (tableHandler.queryIdExists(countQueryId)) { - QueryRequest countRequest = Requests.copyOfQueryRequest(request); - countRequest.setQueryId(countQueryId); - - // Strip pagination parameters - countRequest.setPageSize(0); - countRequest.setPagedResultsOffset(0); - countRequest.setPagedResultsCookie(null); - - List countResult = query(countRequest); - - if (countResult != null && !countResult.isEmpty()) { - resultCount = countResult.get(0).getContent().get("total").asInteger(); - } else { - logger.debug("Count query {} failed", countQueryId); - resultCount = NO_COUNT; - } - } else { - logger.debug("Count query with id {} not found", countQueryId); - resultCount = NO_COUNT; - } - break; - case NONE: - default: - resultCount = NO_COUNT; - break; - } + if (request.getPageSize() == 0) { + return newQueryResponse(null).asPromise(); + } - if (results.size() < requestPageSize) { - nextCookie = null; - } else { - final int remainingResults = resultCount - (firstResultIndex + results.size()); - if (remainingResults == 0) { - nextCookie = null; - } else { - nextCookie = String.valueOf(firstResultIndex + requestPageSize); - } - } - } else { - nextCookie = null; - resultCount = NO_COUNT; + var tableHandler = getTableHandler(type); + + Integer totalCount = null; + if (request.getTotalPagedResultsPolicy() != CountPolicy.NONE) { + try (var connection = getConnection()) { + totalCount = tableHandler.queryCount(type, params, connection); + } } - if (resultCount == NO_COUNT) { - return newQueryResponse(nextCookie).asPromise(); + int nextOffset = ((Integer) params.get(PAGED_RESULTS_OFFSET)) + results.size(); + + if (totalCount != null) { + return newQueryResponse( + totalCount > nextOffset ? String.valueOf(nextOffset) : null, + CountPolicy.EXACT, + totalCount).asPromise(); } else { - return newQueryResponse(nextCookie, CountPolicy.EXACT, resultCount).asPromise(); + return newQueryResponse( + results.size() >= request.getPageSize() ? String.valueOf(nextOffset) : null).asPromise(); } } catch (final ResourceException e) { return e.asPromise(); @@ -690,20 +620,49 @@ public Promise handleQuery(Context context, Qu } } - @Override - public List query(QueryRequest request) throws ResourceException { - String fullId = request.getResourcePath(); - String type = trimStartingSlash(fullId); - logger.trace("Full id: {} Extracted type: {}", fullId, type); + /** + * Create query parameters for calling {@link TableHandler} implementation. + * + * @param request the original query request + * @return table handler's query parameters + */ + private Map createQueryParams(QueryRequest request) throws BadRequestException { Map params = new HashMap<>(); - params.putAll(request.getAdditionalParameters()); + + // query parameters params.put(QUERY_ID, request.getQueryId()); params.put(QUERY_EXPRESSION, request.getQueryExpression()); params.put(QUERY_FILTER, request.getQueryFilter()); + + // paging parameters params.put(PAGE_SIZE, request.getPageSize()); - params.put(PAGED_RESULTS_OFFSET, request.getPagedResultsOffset()); + final String pagedResultsCookie = request.getPagedResultsCookie(); + if (pagedResultsCookie != null && !pagedResultsCookie.isEmpty()) { + try { + params.put(PAGED_RESULTS_OFFSET, Math.max(0, Integer.parseInt(pagedResultsCookie))); + } catch (NumberFormatException e) { + throw new BadRequestException("Invalid paged results cookie"); + } + } else { + params.put(PAGED_RESULTS_OFFSET, request.getPagedResultsOffset()); + } + + // sorting parameters params.put(SORT_KEYS, request.getSortKeys()); + // additional named parameters + params.putAll(request.getAdditionalParameters()); + + return params; + } + + @Override + public List query(QueryRequest request) throws ResourceException { + String fullId = request.getResourcePath(); + String type = trimStartingSlash(fullId); + logger.trace("Full id: {} Extracted type: {}", fullId, type); + var params = createQueryParams(request); + Connection connection = null; try { TableHandler tableHandler = getTableHandler(type); diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/handler/MappedColumnConfig.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/handler/MappedColumnConfig.java index ec1585111..7ab48c9d7 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/handler/MappedColumnConfig.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/handler/MappedColumnConfig.java @@ -40,15 +40,19 @@ public enum ValueType { public static final String COLUMN_NAME = "column"; public static final String VALUE_TYPE = "valueType"; + public static final String JAVA_TYPE = "javaType"; public final JsonPointer propertyName; public final ValueType valueType; + public final Class javaType; public final String columnName; - public MappedColumnConfig(JsonPointer propertyName, String columnName, ValueType valueType) { + public MappedColumnConfig(JsonPointer propertyName, String columnName, + ValueType valueType, Class javaType) { this.propertyName = propertyName; this.columnName = columnName; this.valueType = valueType; + this.javaType = javaType; } /** @@ -76,7 +80,30 @@ public static MappedColumnConfig parse(String name, JsonValue columnConfig) { return new MappedColumnConfig( new JsonPointer(name), columnConfig.required().asString(), - ValueType.STRING); + ValueType.STRING, + String.class); + } + } + + /** + * Parse java value class name (used as a type hint when mapping JDBC types). + * + * @param className java class name + * @return java class that for the stored value + */ + private static Class parseClass(String className) { + // intentionally avoid Class#forName + switch (className) { + case "java.lang.Integer": + return Integer.class; + case "java.lang.Long": + return Long.class; + case "java.lang.Double": + return Double.class; + case "java.lang.String": + return String.class; + default: + throw new InvalidException("Unsupported java class name " + className); } } @@ -87,25 +114,28 @@ public static MappedColumnConfig parse(String name, JsonValue columnConfig) { * * 
      *   "propertyPointer": ["columnName", "valueType"],
+     *   "propertyPointer": ["columnName", "valueType", "javaType"]
      *
* * Example: * * 
-     *   "foo": ["foo", "STRING"]
+     *   "foo": ["foo", "STRING"],
+     *   "bar": ["bar", "NUMBER", "java.lang.Double"]
      *
*/ private static MappedColumnConfig parseList(String name, JsonValue columnConfig) { int size = columnConfig.asList().size(); if (size < 2 || size > 3) { throw new InvalidException("Explicit table mapping has invalid entry for " - + name + ", expecting [column name, value type, stored type] but contains " + + name + ", expecting [column name, value type, java type] but contains " + columnConfig.asList()); } return new MappedColumnConfig( new JsonPointer(name), columnConfig.get(0).required().asString(), - size > 1 ? ValueType.valueOf(columnConfig.get(1).asString()) : ValueType.STRING); + size > 1 ? ValueType.valueOf(columnConfig.get(1).asString()) : ValueType.STRING, + size > 2 ? parseClass(columnConfig.get(2).asString()) : null); } /** @@ -117,13 +147,18 @@ private static MappedColumnConfig parseList(String name, JsonValue columnConfig) * "propertyPointer": { * "type": "VALUE_TYPE", * }, + * "propertyPointer": { + * "type": "VALUE_TYPE", + * "javaType": "JAVA_CLASS" + * }, * * * Example: * * 
      *   "foo": {
-     *     "type": "NUMBER"
+     *     "type": "NUMBER",
+     *     "javaType": "java.lang.Double"
      *   }
      *
*/ @@ -135,7 +170,8 @@ private static MappedColumnConfig parseMap(String name, JsonValue columnConfig) return new MappedColumnConfig( new JsonPointer(name), columnConfig.get(COLUMN_NAME).required().asString(), - valueType != null ? ValueType.valueOf(valueType) : ValueType.STRING); + valueType != null ? ValueType.valueOf(valueType) : ValueType.STRING, + columnConfig.isDefined(JAVA_TYPE) ? parseClass(columnConfig.get(JAVA_TYPE).asString()) : null); } } diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/handler/MappedTableHandler.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/handler/MappedTableHandler.java index a2af2310e..c218493f2 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/handler/MappedTableHandler.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/handler/MappedTableHandler.java @@ -306,7 +306,7 @@ protected void applyStatementParameter(PreparedStatement statement, int index, M break; case BOOLEAN: if (rawValue instanceof Boolean) { - statement.setObject(index, ((Boolean) rawValue).booleanValue() ? 1 : 0); + statement.setObject(index, ((Boolean) rawValue).booleanValue() ? 1 : 0, Types.BIT); } else if (rawValue == null) { statement.setNull(index, Types.BIT); } else { diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/mapper/MappedResultMapper.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/mapper/MappedResultMapper.java index 791085c10..7d73b0d64 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/mapper/MappedResultMapper.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/mapper/MappedResultMapper.java @@ -17,6 +17,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import java.io.IOException; +import java.math.BigDecimal; +import java.math.BigInteger; import java.sql.ResultSet; import java.sql.SQLException; import java.util.Collection; diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/query/MappedSQLQueryFilterVisitor.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/query/MappedSQLQueryFilterVisitor.java index 17f7af013..d5759c82f 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/query/MappedSQLQueryFilterVisitor.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/query/MappedSQLQueryFilterVisitor.java @@ -19,8 +19,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.forgerock.json.JsonPointer; import org.forgerock.openidm.repo.jdbc.impl.handler.MappedColumnConfig; -import org.forgerock.openidm.repo.jdbc.impl.handler.MappedConfigResolver; import org.forgerock.openidm.repo.jdbc.impl.handler.MappedColumnConfig.ValueType; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedConfigResolver; import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterCollector; import org.forgerock.openidm.repo.util.StringSQLQueryFilterVisitor; import org.forgerock.openidm.repo.util.StringSQLRenderer; @@ -45,22 +45,12 @@ public StringSQLRenderer visitValueAssertion(NamedParameterCollector collector, Object valueAssertion) { MappedColumnConfig config = configResolver.resolve(field); - // convert column value to DECIMAL to ensure correct operator behavior if (isNumeric(valueAssertion) && config.valueType == ValueType.NUMBER) { - String paramName = collector.register("v", valueAssertion); - return new StringSQLRenderer( - "CAST(" + config.columnName + " AS DECIMAL)" - + " " + operand + " " - + "CAST(${" + paramName + "} AS DECIMAL)"); + return visitNumericAssertion(collector, config, operand, field, valueAssertion); } - // convert column value to BIT to ensure database vendor support if (valueAssertion instanceof Boolean && config.valueType == ValueType.BOOLEAN) { - String paramName = collector.register("v", ((Boolean) valueAssertion).booleanValue() ? 1 : 0); - return new StringSQLRenderer( - "CAST(" + config.columnName + " AS BIT)" - + " " + operand + " " - + "CAST(${" + paramName + "} AS BIT)"); + return visitBooleanAssertion(collector, config, operand, field, valueAssertion); } String paramValue; @@ -79,6 +69,25 @@ public StringSQLRenderer visitValueAssertion(NamedParameterCollector collector, + "${" + paramName + "}"); } + protected StringSQLRenderer visitNumericAssertion(NamedParameterCollector collector, MappedColumnConfig config, + String operand, JsonPointer field, Object valueAssertion) { + // convert column value to DECIMAL to ensure correct operator behavior + String paramName = collector.register("v", valueAssertion); + return new StringSQLRenderer( + "CAST(" + config.columnName + " AS DECIMAL)" + + " " + operand + " " + + "CAST(${" + paramName + "} AS DECIMAL)"); + } + + protected StringSQLRenderer visitBooleanAssertion(NamedParameterCollector collector, MappedColumnConfig config, + String operand, JsonPointer field, Object valueAssertion) { + // convert column value to SMALLINT to ensure database vendor support + String paramName = collector.register("v", ((Boolean) valueAssertion).booleanValue() ? 1 : 0); + return new StringSQLRenderer( + "CAST(" + config.columnName + " AS BIT)" + + " " + operand + " " + + "${" + paramName + "}"); + } @Override public StringSQLRenderer visitPresentFilter(NamedParameterCollector collector, JsonPointer field) { diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/DB2MappedTableHandler.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/DB2MappedTableHandler.java index 4647b1ed4..a3735abe3 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/DB2MappedTableHandler.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/DB2MappedTableHandler.java @@ -16,10 +16,16 @@ package org.forgerock.openidm.repo.jdbc.impl.vendor; import java.util.Map; +import org.forgerock.json.JsonPointer; import org.forgerock.json.JsonValue; import org.forgerock.openidm.repo.jdbc.SQLExceptionHandler; import org.forgerock.openidm.repo.jdbc.impl.SQLBuilder; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedColumnConfig; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedConfigResolver; import org.forgerock.openidm.repo.jdbc.impl.handler.MappedTableHandler; +import org.forgerock.openidm.repo.jdbc.impl.query.MappedSQLQueryFilterVisitor; +import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterCollector; +import org.forgerock.openidm.repo.util.StringSQLRenderer; /** * DB2 database {@link MappedTableHandler} implementation. @@ -58,6 +64,16 @@ public String toSQL() { }; } - // XXX query filter visitor using TO_NUMBER() function for numeric assertions + @Override + protected MappedSQLQueryFilterVisitor createFilterVisitor(MappedConfigResolver configResolver) { + return new MappedSQLQueryFilterVisitor(configResolver, objectMapper) { + @Override + protected StringSQLRenderer visitBooleanAssertion(NamedParameterCollector collector, + MappedColumnConfig config, String operand, JsonPointer field, Object valueAssertion) { + String paramName = collector.register("v", ((Boolean) valueAssertion).booleanValue() ? 1 : 0); + return new StringSQLRenderer(config.columnName + " " + operand + " " + "${" + paramName + "}"); + } + }; + } } diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/H2MappedTableHandler.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/H2MappedTableHandler.java index c92f1cfe5..7ce1d44c8 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/H2MappedTableHandler.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/H2MappedTableHandler.java @@ -17,9 +17,15 @@ import java.util.Map; import java.util.stream.Collectors; +import org.forgerock.json.JsonPointer; import org.forgerock.json.JsonValue; import org.forgerock.openidm.repo.jdbc.SQLExceptionHandler; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedColumnConfig; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedConfigResolver; import org.forgerock.openidm.repo.jdbc.impl.handler.MappedTableHandler; +import org.forgerock.openidm.repo.jdbc.impl.query.MappedSQLQueryFilterVisitor; +import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterCollector; +import org.forgerock.openidm.repo.util.StringSQLRenderer; /** * H2 database {@link MappedTableHandler} implementation. @@ -58,4 +64,16 @@ protected Map initializeImplicitSql() { return result; } + @Override + protected MappedSQLQueryFilterVisitor createFilterVisitor(MappedConfigResolver configResolver) { + return new MappedSQLQueryFilterVisitor(configResolver, objectMapper) { + @Override + protected StringSQLRenderer visitBooleanAssertion(NamedParameterCollector collector, + MappedColumnConfig config, String operand, JsonPointer field, Object valueAssertion) { + String paramName = collector.register("v", valueAssertion); + return new StringSQLRenderer(config.columnName + " " + operand + " " + "${" + paramName + "}"); + } + }; + } + } diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/MySQLMappedTableHandler.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/MySQLMappedTableHandler.java index c4f4ca195..e32f0614d 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/MySQLMappedTableHandler.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/MySQLMappedTableHandler.java @@ -16,9 +16,15 @@ package org.forgerock.openidm.repo.jdbc.impl.vendor; import java.util.Map; +import org.forgerock.json.JsonPointer; import org.forgerock.json.JsonValue; import org.forgerock.openidm.repo.jdbc.SQLExceptionHandler; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedColumnConfig; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedConfigResolver; import org.forgerock.openidm.repo.jdbc.impl.handler.MappedTableHandler; +import org.forgerock.openidm.repo.jdbc.impl.query.MappedSQLQueryFilterVisitor; +import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterCollector; +import org.forgerock.openidm.repo.util.StringSQLRenderer; /** * MySQL database {@link MappedTableHandler} implementation. @@ -35,4 +41,16 @@ public MySQLMappedTableHandler( super(schemaName, tableName, columnMapping, queryConfig, commandConfig, exceptionHandler); } + @Override + protected MappedSQLQueryFilterVisitor createFilterVisitor(MappedConfigResolver configResolver) { + return new MappedSQLQueryFilterVisitor(configResolver, objectMapper) { + @Override + protected StringSQLRenderer visitBooleanAssertion(NamedParameterCollector collector, + MappedColumnConfig config, String operand, JsonPointer field, Object valueAssertion) { + String paramName = collector.register("v", valueAssertion); + return new StringSQLRenderer(config.columnName + " " + operand + " " + "${" + paramName + "}"); + } + }; + } + } diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/OracleMappedTableHandler.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/OracleMappedTableHandler.java index 0babda300..a0e07a6ba 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/OracleMappedTableHandler.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/OracleMappedTableHandler.java @@ -16,10 +16,16 @@ package org.forgerock.openidm.repo.jdbc.impl.vendor; import java.util.Map; +import org.forgerock.json.JsonPointer; import org.forgerock.json.JsonValue; import org.forgerock.openidm.repo.jdbc.SQLExceptionHandler; import org.forgerock.openidm.repo.jdbc.impl.SQLBuilder; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedColumnConfig; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedConfigResolver; import org.forgerock.openidm.repo.jdbc.impl.handler.MappedTableHandler; +import org.forgerock.openidm.repo.jdbc.impl.query.MappedSQLQueryFilterVisitor; +import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterCollector; +import org.forgerock.openidm.repo.util.StringSQLRenderer; /** * Oracle database {@link MappedTableHandler} implementation. @@ -58,4 +64,16 @@ public String toSQL() { }; } + @Override + protected MappedSQLQueryFilterVisitor createFilterVisitor(MappedConfigResolver configResolver) { + return new MappedSQLQueryFilterVisitor(configResolver, objectMapper) { + @Override + protected StringSQLRenderer visitBooleanAssertion(NamedParameterCollector collector, + MappedColumnConfig config, String operand, JsonPointer field, Object valueAssertion) { + String paramName = collector.register("v", ((Boolean) valueAssertion).booleanValue() ? 1 : 0); + return new StringSQLRenderer(config.columnName + " " + operand + " " + "${" + paramName + "}"); + } + }; + } + } diff --git a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/PostgreSQLMappedTableHandler.java b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/PostgreSQLMappedTableHandler.java index 7aa440829..9a40f1fd8 100644 --- a/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/PostgreSQLMappedTableHandler.java +++ b/openidm-repo-jdbc/src/main/java/org/forgerock/openidm/repo/jdbc/impl/vendor/PostgreSQLMappedTableHandler.java @@ -18,9 +18,15 @@ import java.util.Map; import java.util.stream.Collectors; +import org.forgerock.json.JsonPointer; import org.forgerock.json.JsonValue; import org.forgerock.openidm.repo.jdbc.SQLExceptionHandler; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedColumnConfig; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedConfigResolver; import org.forgerock.openidm.repo.jdbc.impl.handler.MappedTableHandler; +import org.forgerock.openidm.repo.jdbc.impl.query.MappedSQLQueryFilterVisitor; +import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterCollector; +import org.forgerock.openidm.repo.util.StringSQLRenderer; /** * PostgreSQL database {@link MappedTableHandler} implementation. @@ -61,4 +67,17 @@ protected Map initializeImplicitSql() { return result; } + + @Override + protected MappedSQLQueryFilterVisitor createFilterVisitor(MappedConfigResolver configResolver) { + return new MappedSQLQueryFilterVisitor(configResolver, objectMapper) { + @Override + protected StringSQLRenderer visitBooleanAssertion(NamedParameterCollector collector, + MappedColumnConfig config, String operand, JsonPointer field, Object valueAssertion) { + String paramName = collector.register("v", valueAssertion); + return new StringSQLRenderer(config.columnName + " " + operand + " " + "${" + paramName + "}"); + } + }; + } + } diff --git a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/handler/AbstractMappedTableHandlerTest.java b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/handler/AbstractMappedTableHandlerTest.java index 03b437dd0..28d62f4a2 100644 --- a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/handler/AbstractMappedTableHandlerTest.java +++ b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/handler/AbstractMappedTableHandlerTest.java @@ -54,13 +54,15 @@ protected JsonValue getColumnMapping() { field("_rev", Constants.RAW_OBJECT_REV), // string column property field("name", "name"), - // number column property + // number (integer) column property + field("priority", array("priority", ValueType.NUMBER.name())), + // number (decimal) column property field("score", object( field("column", "ranking"), // intentional name discrepancy - field("type", (FUTURE_MODE ? ValueType.NUMBER : ValueType.STRING).name()) + field("type", ValueType.NUMBER.name()) )), // boolean column property - field("visible", array("visible", (FUTURE_MODE ? ValueType.BOOLEAN : ValueType.STRING).name())), + field("visible", array("visible", ValueType.BOOLEAN.name())), // json list column property field("tags", array("tags", ValueType.JSON_LIST.name())), // json map column property diff --git a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/handler/AbstractTableHandlerTest.java b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/handler/AbstractTableHandlerTest.java index c1af68341..c6db84212 100644 --- a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/handler/AbstractTableHandlerTest.java +++ b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/handler/AbstractTableHandlerTest.java @@ -77,11 +77,6 @@ */ public abstract class AbstractTableHandlerTest { - /** - * Whether to use support for number and boolean data types (this will be possible after dropping legacy mode). - */ - protected static final boolean FUTURE_MODE = false; - protected static final String OBJECT_TYPE = "greeting"; protected static final String RESOURCE_ID = "hello"; @@ -205,8 +200,9 @@ public void testCreate() throws Exception { public void testRead() throws Exception { var template = Map.of( "name", "HELLO", - "score", FUTURE_MODE ? 7 : "7", - "visible", FUTURE_MODE ? true : "true", + "priority", 42, + "score", 7.0, + "visible", true, "tags", List.of("foo", "bar"), "meta", Map.of("owner", "john")); createResource(RESOURCE_ID, template); @@ -242,8 +238,8 @@ public void testReadNonExistent() throws Exception { public void testQueryFilterSimple() throws Exception { var template = Map.of( "name", "HELLO", - "score", FUTURE_MODE ? 7.0 : "7", - "visible", FUTURE_MODE ? true : "true", + "score", 7.0, + "visible", true, "tags", List.of("foo", "bar"), "meta", Map.of("owner", "john")); createResource(RESOURCE_ID, template); @@ -268,7 +264,7 @@ public void testQueryFilterComplex() throws Exception { assertEquals(resultIds, Set.of(RESOURCE_ID)); } - @Test(enabled = FUTURE_MODE) + @Test public void testQueryFilterNumber() throws Exception { createResource("lower-value", Map.of("score", 9)); createResource("lower-decimal", Map.of("score", 9.1)); @@ -442,7 +438,7 @@ public void testUpdateLock() throws Exception { ); } - @Test(enabled = FUTURE_MODE) + @Test public void testQueryIdCount() throws Exception { assertNull(tableHandler.queryCount(OBJECT_TYPE, Map.of("_queryId", "non-existing"), connection)); @@ -461,7 +457,7 @@ public void testQueryIdCount() throws Exception { assertEquals(count, result.size()); } - @Test(enabled = FUTURE_MODE) + @Test public void testQueryFilterCount() throws Exception { createResource(RESOURCE_ID, Map.of("name", "HELLO")); createResource("alternative", Map.of("name", "GUTEN TAG")); diff --git a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/statement/NamedParameterSqlTest.java b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/statement/NamedParameterSqlTest.java index f157970b9..3adc5b2bc 100644 --- a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/statement/NamedParameterSqlTest.java +++ b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/statement/NamedParameterSqlTest.java @@ -22,8 +22,6 @@ import java.util.List; import java.util.stream.Collectors; -import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterSql; -import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterToken; import org.testng.annotations.Test; /** diff --git a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/statement/NamedParameterSupportTest.java b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/statement/NamedParameterSupportTest.java index 9e064105d..ef6e5fb0c 100644 --- a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/statement/NamedParameterSupportTest.java +++ b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/statement/NamedParameterSupportTest.java @@ -19,8 +19,6 @@ import java.util.List; import java.util.Map; -import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterSql; -import org.forgerock.openidm.repo.jdbc.impl.statement.NamedParameterSupport; import org.testng.annotations.Test; /** diff --git a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/vendor/OracleMappedTableHandlerIT.java b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/vendor/OracleMappedTableHandlerIT.java index 3f0395fee..6f7361d4d 100644 --- a/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/vendor/OracleMappedTableHandlerIT.java +++ b/openidm-repo-jdbc/src/test/java/org/forgerock/openidm/repo/jdbc/impl/vendor/OracleMappedTableHandlerIT.java @@ -15,9 +15,14 @@ */ package org.forgerock.openidm.repo.jdbc.impl.vendor; +import static org.forgerock.json.JsonValue.field; +import static org.forgerock.json.JsonValue.object; + import java.sql.Connection; +import org.forgerock.json.JsonValue; import org.forgerock.openidm.repo.jdbc.TableHandler; import org.forgerock.openidm.repo.jdbc.impl.handler.AbstractMappedTableHandlerTest; +import org.forgerock.openidm.repo.jdbc.impl.handler.MappedColumnConfig.ValueType; import org.testng.annotations.Test; @Test(singleThreaded = true, suiteName = "oracle") @@ -40,4 +45,16 @@ protected TableHandler createTableHandler() throws Exception { ); } + @Override + protected JsonValue getColumnMapping() { + var mapping = super.getColumnMapping(); + // we need to specify java type hint because for Oracle every numeric data type is mapped to BigDecimal + mapping.put("priority", object( + field("column", "priority"), + field("type", ValueType.NUMBER.name()), + field("javaType", Integer.class.getName() + ))); + return mapping; + } + } diff --git a/openidm-repo-jdbc/src/test/resources/hsqldb.sql b/openidm-repo-jdbc/src/test/resources/hsqldb.sql index 25d04f96d..d3ff24549 100644 --- a/openidm-repo-jdbc/src/test/resources/hsqldb.sql +++ b/openidm-repo-jdbc/src/test/resources/hsqldb.sql @@ -34,8 +34,9 @@ CREATE TABLE wrenidm.managedgreeting ( objectid VARCHAR(255) NOT NULL, rev VARCHAR(38) NOT NULL, name VARCHAR(255), - ranking VARCHAR(32), - visible VARCHAR(5), + priority INTEGER, + ranking NUMERIC(10, 2), + visible BOOLEAN, tags VARCHAR(255), meta VARCHAR(2048), PRIMARY KEY (objectid) diff --git a/openidm-repo-jdbc/src/test/resources/vendor/db2.sql b/openidm-repo-jdbc/src/test/resources/vendor/db2.sql index 4cd7a11ae..b3dad19a3 100644 --- a/openidm-repo-jdbc/src/test/resources/vendor/db2.sql +++ b/openidm-repo-jdbc/src/test/resources/vendor/db2.sql @@ -38,8 +38,9 @@ CREATE TABLE wrenidm.managedgreeting ( objectid VARCHAR(255) NOT NULL, rev VARCHAR(38) NOT NULL, name VARCHAR(255), - ranking VARCHAR(32), - visible VARCHAR(5), + priority INTEGER, + ranking DECIMAL(10, 2), + visible SMALLINT, tags VARCHAR(255), meta VARCHAR(2048), PRIMARY KEY (objectid) diff --git a/openidm-repo-jdbc/src/test/resources/vendor/h2.sql b/openidm-repo-jdbc/src/test/resources/vendor/h2.sql index 121759654..716f33e9e 100644 --- a/openidm-repo-jdbc/src/test/resources/vendor/h2.sql +++ b/openidm-repo-jdbc/src/test/resources/vendor/h2.sql @@ -34,8 +34,9 @@ CREATE TABLE wrenidm.managedgreeting ( objectid VARCHAR(255) NOT NULL, rev VARCHAR(38) NOT NULL, name VARCHAR(255), - ranking VARCHAR(32), - visible VARCHAR(5), + priority INTEGER, + ranking NUMERIC(10, 2), + visible BOOLEAN, tags VARCHAR(255), meta VARCHAR(2048), PRIMARY KEY (objectid) diff --git a/openidm-repo-jdbc/src/test/resources/vendor/mssql.sql b/openidm-repo-jdbc/src/test/resources/vendor/mssql.sql index 4063180e6..8b7473142 100644 --- a/openidm-repo-jdbc/src/test/resources/vendor/mssql.sql +++ b/openidm-repo-jdbc/src/test/resources/vendor/mssql.sql @@ -47,8 +47,9 @@ CREATE TABLE wrenidm.managedgreeting ( objectid NVARCHAR(255) NOT NULL, rev NVARCHAR(38) NOT NULL, name NVARCHAR(255), - ranking NVARCHAR(32), - visible NVARCHAR(5), + priority INTEGER, + ranking DECIMAL(10, 2), + visible BIT, tags NVARCHAR(255), meta NVARCHAR(2048), PRIMARY KEY (objectid) diff --git a/openidm-repo-jdbc/src/test/resources/vendor/mysql.sql b/openidm-repo-jdbc/src/test/resources/vendor/mysql.sql index 8df22b901..cf539cbd7 100644 --- a/openidm-repo-jdbc/src/test/resources/vendor/mysql.sql +++ b/openidm-repo-jdbc/src/test/resources/vendor/mysql.sql @@ -33,8 +33,9 @@ CREATE TABLE wrenidm.managedgreeting ( objectid VARCHAR(255) NOT NULL, rev VARCHAR(38) NOT NULL, name VARCHAR(255), - ranking VARCHAR(32), - visible VARCHAR(5), + priority INTEGER, + ranking DECIMAL(10, 2), + visible BIT, tags VARCHAR(255), meta VARCHAR(2048), PRIMARY KEY (objectid) diff --git a/openidm-repo-jdbc/src/test/resources/vendor/oracle.sql b/openidm-repo-jdbc/src/test/resources/vendor/oracle.sql index 4ee16df04..e35284672 100644 --- a/openidm-repo-jdbc/src/test/resources/vendor/oracle.sql +++ b/openidm-repo-jdbc/src/test/resources/vendor/oracle.sql @@ -32,8 +32,9 @@ CREATE TABLE wrenidm.managedgreeting ( objectid VARCHAR2(255 CHAR) NOT NULL, rev VARCHAR2(38 CHAR) NOT NULL, name VARCHAR2(255), - ranking VARCHAR2(32 CHAR), - visible VARCHAR2(5 CHAR), + priority INTEGER, + ranking FLOAT, + visible SMALLINT, tags VARCHAR2(255), meta VARCHAR2(2048), PRIMARY KEY (objectid) diff --git a/openidm-repo-jdbc/src/test/resources/vendor/postgresql.sql b/openidm-repo-jdbc/src/test/resources/vendor/postgresql.sql index 41f21ff57..46f626fdf 100644 --- a/openidm-repo-jdbc/src/test/resources/vendor/postgresql.sql +++ b/openidm-repo-jdbc/src/test/resources/vendor/postgresql.sql @@ -36,8 +36,9 @@ CREATE TABLE wrenidm.managedgreeting ( objectid VARCHAR(255) NOT NULL, rev VARCHAR(38) NOT NULL, name VARCHAR(255), - ranking VARCHAR(32), - visible VARCHAR(5), + priority INTEGER, + ranking DECIMAL(10, 2), + visible BOOLEAN, tags JSON, meta JSON, PRIMARY KEY (objectid)