Remove FieldValueConverter (#2699)

* (because) used only for ActionFields.STATUS
* could be part of new Node mapping mechanism
* simplify
* Deprecate ActionFields.STATUS, add / replace it with ActionFields.ACTIVE. In future STATUS will become the real action status (and DETAILSTATUS will be removed)
* Deprecate MgmtAction.getStatus add / replace it with MgmtAction.isActive. In future status will become the real action status (and detailStatus will be removed)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2025-09-26 16:57:10 +03:00
committed by GitHub
parent 1abfa0a2f4
commit a6867219b1
11 changed files with 88 additions and 141 deletions

View File

@@ -17,10 +17,12 @@ import lombok.Getter;
* Sort and search fields for actions.
*/
@Getter
public enum ActionFields implements QueryField, FieldValueConverter<ActionFields> {
public enum ActionFields implements QueryField {
ID("id"),
STATUS("active"), // true if status is "pending", false if "finished"
@Deprecated(since = "0.10.0", forRemoval = true) // use ACTIVE
STATUS("active"), // true if status is "pending", false if "finished", after removal, will deprecate DETAILSTATUS too and replace with STATUS
ACTIVE("active"), // true if st
DETAILSTATUS("status"), // real status
LASTSTATUSCODE("lastActionStatusCode"),
CREATEDAT("createdAt"),
@@ -39,9 +41,6 @@ public enum ActionFields implements QueryField, FieldValueConverter<ActionFields
ROLLOUTGROUP("rolloutGroup", RolloutGroupFields.ID.getJpaEntityFieldName(), RolloutGroupFields.NAME.getJpaEntityFieldName()),
EXTERNALREF("externalRef");
private static final String ACTIVE = "pending";
private static final String INACTIVE = "finished";
private final String jpaEntityFieldName;
private final List<String> subEntityAttributes;
@@ -50,19 +49,15 @@ public enum ActionFields implements QueryField, FieldValueConverter<ActionFields
this.subEntityAttributes = List.of(subEntityAttributes);
}
@Override
public Object convertValue(final ActionFields enumValue, final String value) {
return STATUS == enumValue ? convertStatusValue(value) : value;
}
private static Object convertStatusValue(final String value) {
@Deprecated(since = "0.10.0", forRemoval = true) // remove together with STATUS (with active meaning)
public static Object convertStatusValue(final String value) {
final String trimmedValue = value.trim();
if (trimmedValue.equalsIgnoreCase(ACTIVE)) {
if (trimmedValue.equalsIgnoreCase("pending")) {
return true;
} else if (trimmedValue.equalsIgnoreCase(INACTIVE)) {
} else if (trimmedValue.equalsIgnoreCase("finished")) {
return false;
} else {
throw new IllegalArgumentException("field 'status' must be one of the following values {" + ACTIVE + ", " + INACTIVE + "}");
throw new IllegalArgumentException("field 'status' must be one of the following values {pending, finished}");
}
}
}

View File

@@ -1,32 +0,0 @@
/**
* Copyright (c) 2015 Bosch Software Innovations GmbH and others
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.eclipse.hawkbit.repository;
/**
* A value convert which converts given string based values into an object which
* can be used for building generic queries. Mapping external API values e.g.
* REST API to inside representation on database. E.g. mapping 'pending' or
* 'finished' values in rest queries to Action#isActive boolean value.
*
* @param <T> the enum parameter
*/
public interface FieldValueConverter<T extends Enum<T>> {
/**
* Converts the given {@code value} into the representation to build a generic query.
*
* @param enumValue the enum value to build the value for
* @param value the value in string representation
* @return the converted object if conversion is applicable, or if the given enum value does not need to be converted the
* unmodified {@code value} is returned.
* @throws IllegalArgumentException if the value is not supported
*/
Object convertValue(final T enumValue, final String value);
}

View File

@@ -76,20 +76,28 @@ public class MgmtAction extends MgmtBaseEntity {
public static final String ACTION_CANCEL = "cancel";
/**
* API definition for action completed.
*
* @deprecated since 0.10.0 will be removed together with status field
*/
@Deprecated(since = "0.10.0", forRemoval = true)
public static final String ACTION_FINISHED = "finished";
/**
* API definition for action still active.
*
* @deprecated since 0.10.0 will be removed together with status field
*/
@Deprecated(since = "0.10.0", forRemoval = true)
public static final String ACTION_PENDING = "pending";
@Schema(description = "ID of the action", example = "7")
private Long id;
@Schema(description = "Type of action", example = "update")
private String type;
@Schema(description = "Status of action", example = "finished")
@Deprecated(since = "0.10.0")
@Schema(description = "Status of action, use active", example = "finished", deprecated = true)
private String status;
@Schema(description = "Status of action")
private boolean active;
@Schema(description = "Detailed status of action", example = "finished")
private String detailStatus;
@Schema(example = "1691065903238")

View File

@@ -239,12 +239,9 @@ public final class MgmtTargetMapper {
action.getWeight().ifPresent(result::setWeight);
result.setForceType(MgmtRestModelMapper.convertActionType(action.getActionType()));
if (action.isActive()) {
result.setStatus(MgmtAction.ACTION_PENDING);
} else {
result.setStatus(MgmtAction.ACTION_FINISHED);
}
result.setStatus(action.isActive() ? MgmtAction.ACTION_PENDING : MgmtAction.ACTION_FINISHED);
result.setActive(action.isActive());
result.setDetailStatus(action.getStatus().toString().toLowerCase());
action.getLastActionStatusCode().ifPresent(result::setLastStatusCode);

View File

@@ -9,6 +9,8 @@
*/
package org.eclipse.hawkbit.repository.jpa.ql;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import jakarta.persistence.EntityManager;
@@ -21,6 +23,7 @@ import lombok.NoArgsConstructor;
import lombok.Setter;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.text.StrLookup;
import org.eclipse.hawkbit.repository.ActionFields;
import org.eclipse.hawkbit.repository.QueryField;
import org.eclipse.hawkbit.repository.exception.QueryException;
import org.eclipse.hawkbit.repository.exception.RSQLParameterSyntaxException;
@@ -186,32 +189,45 @@ public class QLSupport {
@Override
public <T extends Enum<T> & QueryField> Node parse(final String query, final Class<T> queryFieldType) throws QueryException {
return RsqlParser.parse(query, queryFieldType);
}
}
public static class MappingQueryParser extends DefaultQueryParser {
@Override
public <T extends Enum<T> & QueryField> Node parse(final String query, final Class<T> queryFieldType) throws QueryException {
return RsqlParser.parse(query, queryFieldType).map(this::map);
return RsqlParser.parse(query, queryFieldType).map(comparison -> map(comparison, queryFieldType));
}
protected Comparison map(final Comparison comparison) {
final String key = mapKey(comparison.getKey(), comparison).toString();
final Object value = mapValue(comparison.getValue(), comparison);
protected <T extends Enum<T> & QueryField> Comparison map(final Comparison comparison, final Class<T> queryFieldType) {
final String key = mapKey(comparison.getKey(), comparison, queryFieldType).toString();
final Object value = mapValue(comparison.getValue(), comparison, queryFieldType);
return key.equals(comparison.getKey()) && Objects.equals(value, comparison.getValue())
? comparison : Comparison.builder().key(key).op(comparison.getOp()).value(value).build();
}
// just extension points for subclasses
protected Object mapKey(final String key, final Comparison comparison) {
protected <T extends Enum<T> & QueryField>Object mapKey(final String key, final Comparison comparison, final Class<T> queryFieldType) {
return key;
}
// just extension points for subclasses
protected Object mapValue(final Object value, final Comparison comparison) {
protected <T extends Enum<T> & QueryField> Object mapValue(final Object value, final Comparison comparison, final Class<T> queryFieldType) {
if (queryFieldType == (Class<?>)ActionFields.class && "active".equalsIgnoreCase(comparison.getKey())) {
if (value instanceof List) {
return ((List<?>)value).stream().map(DefaultQueryParser::mapActionStatus).toList();
} else {
return mapActionStatus(value);
}
}
return value;
}
private static Object mapActionStatus(final Object value){
final String strValue = String.valueOf(value);
if ("true".equalsIgnoreCase(strValue) || "false".equalsIgnoreCase(strValue)) {
return value;
} else {
// handle custom action fields status
try {
return ActionFields.convertStatusValue(strValue);
} catch (final IllegalArgumentException e) {
throw new RSQLParameterUnsupportedFieldException(e.getMessage());
}
}
}
}
}

View File

@@ -28,8 +28,6 @@ import java.util.Set;
import java.util.function.Function;
import java.util.function.UnaryOperator;
import jakarta.annotation.Nullable;
import cz.jirutka.rsql.parser.RSQLParser;
import cz.jirutka.rsql.parser.RSQLParserException;
import cz.jirutka.rsql.parser.ast.AndNode;
@@ -41,7 +39,6 @@ import cz.jirutka.rsql.parser.ast.RSQLVisitor;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.eclipse.hawkbit.repository.FieldValueConverter;
import org.eclipse.hawkbit.repository.QueryField;
import org.eclipse.hawkbit.repository.exception.RSQLParameterSyntaxException;
import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException;
@@ -79,14 +76,14 @@ public class RsqlParser {
}
public static Node parse(final String rsql) {
return parse(rsql, (Function<String, Key>) null);
return parse(rsql, (UnaryOperator<String>) null);
}
public static <T extends Enum<T> & QueryField> Node parse(final String rsql, final Class<T> queryFieldType) {
return parse(rsql, queryFieldType == null ? null : key -> resolveKey(key, queryFieldType));
}
private static Node parse(final String rsql, final Function<String, Key> keyResolver) {
private static Node parse(final String rsql, final UnaryOperator<String> keyResolver) {
try {
return RSQL_PARSER
.parse(rsql)
@@ -97,7 +94,7 @@ public class RsqlParser {
}
@SuppressWarnings("java:S3776") // java:S3776 - group in single method for easier read of whole logic
private static <T extends Enum<T> & QueryField> Key resolveKey(final String key, final Class<T> rsqlQueryFieldType) {
private static <T extends Enum<T> & QueryField> String resolveKey(final String key, final Class<T> rsqlQueryFieldType) {
final int firstSeparatorIndex = key.indexOf(SUB_ATTRIBUTE_SEPARATOR);
final String enumName = (firstSeparatorIndex == -1 ? key : key.substring(0, firstSeparatorIndex)).toUpperCase();
log.debug("Get field identifier by name {} of enum type {}", enumName, rsqlQueryFieldType);
@@ -151,17 +148,15 @@ public class RsqlParser {
}
}
return new Key(attribute, RsqlVisitor.valueConverter(enumValue));
return attribute;
}
private record Key(String path, UnaryOperator<Object> converter) {}
private static class RsqlVisitor implements RSQLVisitor<Node, String> {
private final Function<String, Key> keyResolver;
private final Function<String, String> keyResolver;
private RsqlVisitor(final Function<String, Key> keyResolver) {
this.keyResolver = keyResolver == null ? str -> new Key(str, UnaryOperator.identity()) : keyResolver;
private RsqlVisitor(final UnaryOperator<String> keyResolver) {
this.keyResolver = keyResolver == null ? UnaryOperator.identity() : keyResolver;
}
@Override
@@ -184,40 +179,39 @@ public class RsqlParser {
@Override
public Node visit(final ComparisonNode node, final String param) {
final String nodeSelector = node.getSelector();
final Key key = keyResolver.apply(nodeSelector);
final String path = key.path();
final Object value = toValue(node, key);
final String key = keyResolver.apply(nodeSelector);
final Object value = toValue(node);
final ComparisonOperator op = node.getOperator();
if (op == IS || op == RSQLOperators.EQUAL) {
if ("".equals(value)) {
// keep special backward compatible behaviour tags == '' means "null / has not or ''
return new Comparison(path, EQ, null, nodeSelector).or(new Comparison(path, EQ, "", nodeSelector));
return new Comparison(key, EQ, null, nodeSelector).or(new Comparison(key, EQ, "", nodeSelector));
}
return new Comparison(path, isLike(value) ? LIKE : EQ, value, nodeSelector);
return new Comparison(key, isLike(value) ? LIKE : EQ, value, nodeSelector);
} else if (op == NOT || op == RSQLOperators.NOT_EQUAL) {
if ("".equals(value)) {
// keep special backward compatible behaviour. != '' means "not null / has and not ''
return new Comparison(path, LIKE, "*", nodeSelector).and(new Comparison(path, NE, "", nodeSelector));
return new Comparison(key, LIKE, "*", nodeSelector).and(new Comparison(key, NE, "", nodeSelector));
}
return new Comparison(path, isLike(value) ? NOT_LIKE : NE, value, nodeSelector);
return new Comparison(key, isLike(value) ? NOT_LIKE : NE, value, nodeSelector);
} else if (op == RSQLOperators.GREATER_THAN) {
return new Comparison(path, GT, value, nodeSelector);
return new Comparison(key, GT, value, nodeSelector);
} else if (op == RSQLOperators.GREATER_THAN_OR_EQUAL) {
return new Comparison(path, GTE, value, nodeSelector);
return new Comparison(key, GTE, value, nodeSelector);
} else if (op == RSQLOperators.LESS_THAN) {
return new Comparison(path, LT, value, nodeSelector);
return new Comparison(key, LT, value, nodeSelector);
} else if (op == RSQLOperators.LESS_THAN_OR_EQUAL) {
return new Comparison(path, LTE, value, nodeSelector);
return new Comparison(key, LTE, value, nodeSelector);
} else if (op == RSQLOperators.IN) {
return new Comparison(path, IN, value, nodeSelector);
return new Comparison(key, IN, value, nodeSelector);
} else if (op == RSQLOperators.NOT_IN) {
return new Comparison(path, NOT_IN, value, nodeSelector);
return new Comparison(key, NOT_IN, value, nodeSelector);
} else {
throw new IllegalArgumentException("Unsupported operator: " + node.getOperator());
}
}
private Object toValue(final ComparisonNode node, final Key key) {
private Object toValue(final ComparisonNode node) {
final List<String> arguments = node.getArguments();
final ComparisonOperator operator = node.getOperator();
if (arguments.isEmpty()) {
@@ -228,35 +222,16 @@ public class RsqlParser {
return null;
} else {
if (operator == RSQLOperators.IN || operator == RSQLOperators.NOT_IN) {
return arguments.stream().map(key.converter()).toList();
return arguments.stream().toList();
} else if (arguments.size() > 1) {
throw new IllegalArgumentException(
"Operator " + operator + " requires exactly one argument, but got: " + arguments.size());
} else {
return key.converter().apply(arguments.get(0));
return arguments.get(0);
}
}
}
@SuppressWarnings({ "rawtypes", "unchecked" })
private static <T extends Enum<T>> UnaryOperator<Object> valueConverter(@Nullable final T enumValue) {
if (enumValue instanceof FieldValueConverter fieldValueConverter) {
return value -> {
if (value instanceof String strValue) {
try {
return fieldValueConverter.convertValue(enumValue, strValue);
} catch (final Exception e) {
throw new RSQLParameterUnsupportedFieldException(e.getMessage(), null);
}
} else {
return value;
}
};
} else {
return UnaryOperator.identity();
}
}
private static final String ESCAPE_CHAR_WITH_ASTERISK = "\\" + LIKE_WILDCARD;
private static boolean isLike(final Object value) {

View File

@@ -42,7 +42,7 @@ import cz.jirutka.rsql.parser.ast.OrNode;
import cz.jirutka.rsql.parser.ast.RSQLVisitor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.math.NumberUtils;
import org.eclipse.hawkbit.repository.FieldValueConverter;
import org.eclipse.hawkbit.repository.ActionFields;
import org.eclipse.hawkbit.repository.QueryField;
import org.eclipse.hawkbit.repository.exception.RSQLParameterSyntaxException;
import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException;
@@ -301,8 +301,8 @@ public class JpaQueryRsqlVisitor<A extends Enum<A> & QueryField, T> extends Abst
if (javaType != null && javaType.isEnum()) {
return transformEnumValue(node, value, javaType);
}
if (fieldName instanceof FieldValueConverter) {
return convertFieldConverterValue(fieldName, value);
if (fieldName == ActionFields.STATUS) {
return ActionFields.convertStatusValue(value);
}
if (Boolean.TYPE.equals(javaType)) {
@@ -323,15 +323,6 @@ public class JpaQueryRsqlVisitor<A extends Enum<A> & QueryField, T> extends Abst
}
}
@SuppressWarnings({ "rawtypes", "unchecked" })
private Object convertFieldConverterValue(final A fieldName, final String value) {
try {
return ((FieldValueConverter) fieldName).convertValue(fieldName, value);
} catch (final Exception e) {
throw new RSQLParameterSyntaxException(e.getMessage(), null);
}
}
private List<Predicate> mapToPredicate(final ComparisonNode node, final Path<Object> fieldPath,
final List<String> values, final List<Object> transformedValues, final QueryPath queryPath) {

View File

@@ -40,7 +40,7 @@ import cz.jirutka.rsql.parser.ast.RSQLOperators;
import cz.jirutka.rsql.parser.ast.RSQLVisitor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.math.NumberUtils;
import org.eclipse.hawkbit.repository.FieldValueConverter;
import org.eclipse.hawkbit.repository.ActionFields;
import org.eclipse.hawkbit.repository.QueryField;
import org.eclipse.hawkbit.repository.exception.RSQLParameterSyntaxException;
import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException;
@@ -367,12 +367,8 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & QueryField, T>
return toEnumValue(node, javaType, value);
}
if (enumValue instanceof FieldValueConverter fieldValueConverter) {
try {
return fieldValueConverter.convertValue(enumValue, value);
} catch (final Exception e) {
throw new RSQLParameterUnsupportedFieldException(e.getMessage(), null);
}
if (enumValue == ActionFields.STATUS) {
return ActionFields.convertStatusValue(value);
}
if (boolean.class.equals(javaType) || Boolean.class.equals(javaType)) {

View File

@@ -25,6 +25,7 @@ import org.eclipse.hawkbit.artifact.encryption.ArtifactEncryptionSecretsStorage;
import org.eclipse.hawkbit.artifact.encryption.ArtifactEncryptionService;
import org.eclipse.hawkbit.repository.DeploymentManagement;
import org.eclipse.hawkbit.repository.PropertiesQuotaManagement;
import org.eclipse.hawkbit.repository.QueryField;
import org.eclipse.hawkbit.repository.QuotaManagement;
import org.eclipse.hawkbit.repository.RepositoryConfiguration;
import org.eclipse.hawkbit.repository.RepositoryProperties;
@@ -67,10 +68,8 @@ import org.eclipse.hawkbit.repository.jpa.model.JpaTargetType;
import org.eclipse.hawkbit.repository.jpa.model.helper.AfterTransactionCommitExecutorHolder;
import org.eclipse.hawkbit.repository.jpa.model.helper.EntityInterceptorHolder;
import org.eclipse.hawkbit.repository.jpa.model.helper.TenantAwareHolder;
import org.eclipse.hawkbit.repository.jpa.ql.Node;
import org.eclipse.hawkbit.repository.jpa.ql.Node.Comparison;
import org.eclipse.hawkbit.repository.jpa.ql.QLSupport.DefaultQueryParser;
import org.eclipse.hawkbit.repository.jpa.ql.QLSupport.MappingQueryParser;
import org.eclipse.hawkbit.repository.jpa.ql.QLSupport.QueryParser;
import org.eclipse.hawkbit.repository.jpa.repository.ActionRepository;
import org.eclipse.hawkbit.repository.jpa.repository.DistributionSetRepository;
@@ -529,11 +528,12 @@ public class JpaRepositoryConfiguration {
@Bean
@ConditionalOnMissingBean
QueryParser queryParser(final Optional<VirtualPropertyResolver> virtualPropertyResolver) {
return virtualPropertyResolver.<QueryParser>map(resolver -> new MappingQueryParser() {
return virtualPropertyResolver.<QueryParser>map(resolver -> new DefaultQueryParser() {
@Override
protected Object mapValue(final Object value, final Comparison comparison) {
return value instanceof String strValue ? resolver.replace(strValue) : value;
protected <T extends Enum<T> & QueryField> Object mapValue(
final Object value, final Comparison comparison, final Class<T> queryFieldType) {
return super.mapValue(value instanceof String strValue ? resolver.replace(strValue) : value, comparison, queryFieldType);
}
}).orElseGet(DefaultQueryParser::new);
}

View File

@@ -14,6 +14,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import org.eclipse.hawkbit.repository.ActionFields;
import org.eclipse.hawkbit.repository.TargetManagement.Create;
import org.eclipse.hawkbit.repository.exception.QueryException;
import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException;
import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest;
import org.eclipse.hawkbit.repository.jpa.model.JpaAction;
@@ -77,8 +78,8 @@ class RsqlActionFieldsTest extends AbstractJpaIntegrationTest {
assertRSQLQuery(ActionFields.STATUS.name() + "=in=(pending)", 5);
assertRSQLQuery(ActionFields.STATUS.name() + "=out=(pending)", 6);
final String rsql = ActionFields.STATUS.name() + "==true";
assertThatExceptionOfType(RSQLParameterUnsupportedFieldException.class)
final String rsql = ActionFields.STATUS.name() + "==true2";
assertThatExceptionOfType(QueryException.class)
.as("RSQLParameterUnsupportedFieldException because status cannot be compared with 'true'")
.isThrownBy(() -> assertRSQLQuery(rsql, 5));
}

View File

@@ -114,7 +114,7 @@ public class TargetActionsHistory extends Grid<TargetActionsHistory.ActionStatus
}
private boolean isActive() {
return action.getStatus().equals(MgmtAction.ACTION_PENDING);
return action.isActive();
}
private boolean isCancelingOrCanceled() {