From 56ab3994931275497d54def42e154012eb4fbed3 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Thu, 18 Jul 2024 18:13:03 +0300 Subject: [PATCH] Remove target type to target reference (#1771) it is not used and could lead to extensive memory usage if JPA provider load targets while loading types Signed-off-by: Marinov Avgustin --- .../repository/TargetTypeManagement.java | 32 ------ .../hawkbit/repository/model/TargetType.java | 5 - .../management/JpaTargetTypeManagement.java | 24 ---- .../repository/jpa/model/JpaTargetType.java | 76 +++---------- .../DistributionSetSpecification.java | 13 +-- .../TargetTypeSpecification.java | 107 ++---------------- .../TargetTypeAccessControllerTest.java | 25 ---- 7 files changed, 33 insertions(+), 249 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetTypeManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetTypeManagement.java index e94672366..a92620196 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetTypeManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetTypeManagement.java @@ -123,38 +123,6 @@ public interface TargetTypeManagement { @PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET) Optional get(long id); - /** - * @param targetId - * Target ID - * @return Target Type - */ - @PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET) - Optional findByTargetId(long targetId); - - /** - * @param targetIds - * List of Target ID - * @return Target Type - */ - @PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET) - List findByTargetIds(Collection targetIds); - - /** - * @param controllerId - * Target controller ID - * @return Target Type - */ - @PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET) - Optional findByTargetControllerId(String controllerId); - - /** - * @param controllerIds - * List of Target controller ID - * @return Target Type - */ - @PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET) - List findByTargetControllerIds(Collection controllerIds); - /** * @param ids * List of Target type ID diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TargetType.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TargetType.java index 0cde5aceb..efb8cf6af 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TargetType.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/TargetType.java @@ -33,11 +33,6 @@ public interface TargetType extends Type { */ Set getCompatibleDistributionSetTypes(); - /** - * @return immutable set of optional {@link Target}s - */ - Set getTargets(); - /** * Checks if the given {@link DistributionSetType} is in * {@link #getCompatibleDistributionSetTypes()}. diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetTypeManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetTypeManagement.java index 49b6caf43..043f54528 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetTypeManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetTypeManagement.java @@ -166,30 +166,6 @@ public class JpaTargetTypeManagement implements TargetTypeManagement { return targetTypeRepository.findById(id).map(TargetType.class::cast); } - @Override - public Optional findByTargetId(final long targetId) { - return targetTypeRepository.findOne(TargetTypeSpecification.hasTarget(targetId)).map(TargetType.class::cast); - } - - @Override - public List findByTargetIds(final Collection targetIds) { - return targetTypeRepository - .findAll(TargetTypeSpecification.hasTarget(targetIds)).stream().map(TargetType.class::cast).toList(); - } - - @Override - public Optional findByTargetControllerId(final String controllerId) { - return targetTypeRepository - .findOne(TargetTypeSpecification.hasTargetControllerId(controllerId)).map(TargetType.class::cast); - } - - @Override - public List findByTargetControllerIds(final Collection controllerIds) { - return targetTypeRepository - .findAll(TargetTypeSpecification.hasTargetControllerIdIn(controllerIds)) - .stream().map(TargetType.class::cast).toList(); - } - @Override public List get(final Collection ids) { return Collections.unmodifiableList(targetTypeRepository.findAllById(ids)); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaTargetType.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaTargetType.java index 219a77c2b..bc56a2557 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaTargetType.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaTargetType.java @@ -9,6 +9,8 @@ */ package org.eclipse.hawkbit.repository.jpa.model; +import lombok.NoArgsConstructor; +import lombok.ToString; import org.eclipse.hawkbit.repository.event.remote.TargetTypeDeletedEvent; import org.eclipse.hawkbit.repository.event.remote.entity.TargetTypeCreatedEvent; import org.eclipse.hawkbit.repository.event.remote.entity.TargetTypeUpdatedEvent; @@ -38,8 +40,9 @@ import java.util.Set; /** * A target type defines which distribution set types can or have to be * {@link Target}. - * */ +@NoArgsConstructor // default public constructor for JPA +@ToString(callSuper = true) @Entity @Table(name = "sp_target_type", indexes = { @Index(name = "sp_idx_target_type_prim", columnList = "tenant,id") }, uniqueConstraints = { @@ -56,79 +59,46 @@ public class JpaTargetType extends AbstractJpaTypeEntity implements TargetType, @JoinColumn(name = "distribution_set_type", nullable = false, updatable = false, foreignKey = @ForeignKey(value = ConstraintMode.CONSTRAINT, name = "fk_target_type_relation_ds_type"))}) private Set distributionSetTypes; - @OneToMany(targetEntity = JpaTarget.class, mappedBy = "targetType", fetch = FetchType.LAZY) - private Set targets; - - /** - * Constructor - */ - public JpaTargetType() { - // default public constructor for JPA - } - - /** - * Constructor, legacy support where key is set to passed name. - * - * @deprecated will be removed - * - * @param name - * of the type - * @param description - * of the type - * @param colour - * of the type - */ - @Deprecated(forRemoval = true) - public JpaTargetType(final String name, final String description, final String colour) { - this(name, name, description, colour); - } - - /** - * Constructor. - * - * @param key - * of the type - * @param name - * of the type - * @param description - * of the type - * @param colour - * of the type. It will be null by default - */ public JpaTargetType(final String key, final String name, final String description, final String colour) { super(name, description, key, colour); } /** - * @param dsSetType - * Distribution set type + * Adds a compatible distribution set type to this target type. + * + * @param dsSetType Distribution set type to add * @return Target type */ - public void addCompatibleDistributionSetType(final DistributionSetType dsSetType) { + public JpaTargetType addCompatibleDistributionSetType(final DistributionSetType dsSetType) { if (distributionSetTypes == null) { distributionSetTypes = new HashSet<>(); } distributionSetTypes.add(dsSetType); + + return this; } /** - * @param dsTypeId - * Distribution set type ID + * Remove a compatible distribution set type from this target type. + * @param dsTypeId Distribution set type ID * @return Target type */ public JpaTargetType removeDistributionSetType(final Long dsTypeId) { if (distributionSetTypes == null) { return this; } - distributionSetTypes.stream().filter(element -> element.getId().equals(dsTypeId)).findAny() + + distributionSetTypes.stream() + .filter(element -> element.getId().equals(dsTypeId)) + .findAny() .ifPresent(distributionSetTypes::remove); + return this; } @Override public Set getCompatibleDistributionSetTypes() { - if (distributionSetTypes == null) { return Collections.emptySet(); } @@ -136,16 +106,6 @@ public class JpaTargetType extends AbstractJpaTypeEntity implements TargetType, return Collections.unmodifiableSet(distributionSetTypes); } - @Override - public Set getTargets() { - return targets; - } - - @Override - public String toString() { - return "TargetType [key=" + getKey() + ", isDeleted()=" + isDeleted() + ", getId()=" + getId() + "]"; - } - @Override public void fireCreateEvent(final DescriptorEvent descriptorEvent) { EventPublisherHolder.getInstance().getEventPublisher().publishEvent( @@ -163,4 +123,4 @@ public class JpaTargetType extends AbstractJpaTypeEntity implements TargetType, EventPublisherHolder.getInstance().getEventPublisher().publishEvent(new TargetTypeDeletedEvent( getTenant(), getId(), getClass(), EventPublisherHolder.getInstance().getApplicationId())); } -} +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/DistributionSetSpecification.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/DistributionSetSpecification.java index cc0241703..128e0da9a 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/DistributionSetSpecification.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/DistributionSetSpecification.java @@ -16,15 +16,14 @@ import java.util.List; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.Expression; import jakarta.persistence.criteria.JoinType; -import jakarta.persistence.criteria.ListJoin; import jakarta.persistence.criteria.Order; import jakarta.persistence.criteria.Path; import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; import jakarta.persistence.criteria.SetJoin; -import org.eclipse.hawkbit.repository.jpa.model.JpaAction; -import org.eclipse.hawkbit.repository.jpa.model.JpaAction_; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetTag; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetTag_; @@ -41,12 +40,9 @@ import org.springframework.util.CollectionUtils; /** * Specifications class for {@link DistributionSet}s. The class provides Spring * Data JPQL Specifications. - * */ +@NoArgsConstructor(access = AccessLevel.PRIVATE) public final class DistributionSetSpecification { - private DistributionSetSpecification() { - // utility class - } /** * {@link Specification} for retrieving {@link DistributionSet}s with @@ -293,5 +289,4 @@ public final class DistributionSetSpecification { return cb.equal(targetRoot.get(JpaTarget_.controllerId), linkedControllerId); }; } - -} +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetTypeSpecification.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetTypeSpecification.java index 719c2395f..d50538350 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetTypeSpecification.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetTypeSpecification.java @@ -9,12 +9,12 @@ */ package org.eclipse.hawkbit.repository.jpa.specifications; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetType; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetType_; -import org.eclipse.hawkbit.repository.jpa.model.JpaTarget; import org.eclipse.hawkbit.repository.jpa.model.JpaTargetType; import org.eclipse.hawkbit.repository.jpa.model.JpaTargetType_; -import org.eclipse.hawkbit.repository.jpa.model.JpaTarget_; import org.eclipse.hawkbit.repository.model.DistributionSetType; import org.eclipse.hawkbit.repository.model.TargetType; import org.springframework.data.jpa.domain.Specification; @@ -23,94 +23,15 @@ import jakarta.persistence.criteria.SetJoin; import java.util.Collection; /** - * Specifications class for {@link TargetType}s. The class provides Spring Data JPQL - * Specifications. - * + * Specifications class for {@link TargetType}s. The class provides Spring Data JPQL Specifications. */ +@NoArgsConstructor(access = AccessLevel.PRIVATE) public final class TargetTypeSpecification { - private TargetTypeSpecification() { - // utility class - } - /** * {@link Specification} for retrieving {@link TargetType}s by controllerId * - * @param id - * to search for - * - * @return the {@link TargetType} {@link Specification} - */ - public static Specification hasId(final Long id) { - return (targetRoot, query, cb) -> cb.equal(targetRoot.get(JpaTargetType_.id), id); - } - - /** - * {@link Specification} for retrieving {@link TargetType}s by controllerId - * - * @param id - * to search for - * - * @return the {@link TargetType} {@link Specification} - */ - public static Specification hasTarget(final long id) { - return (targetRoot, query, cb) -> { - final SetJoin join = targetRoot.join(JpaTargetType_.targets); - return cb.equal(join.get(JpaTarget_.id), id); - }; - } - - /** - * {@link Specification} for retrieving {@link TargetType}s by controllerId - * - * @param ids - * to search for - * - * @return the {@link TargetType} {@link Specification} - */ - public static Specification hasTarget(final Collection ids) { - return (targetRoot, query, cb) -> { - final SetJoin join = targetRoot.join(JpaTargetType_.targets); - return join.get(JpaTarget_.id).in(ids); - }; - } - - /** - * {@link Specification} for retrieving {@link TargetType}s by controllerId - * - * @param controllerId - * to search for - * - * @return the {@link TargetType} {@link Specification} - */ - public static Specification hasTargetControllerId(final String controllerId) { - return (targetRoot, query, cb) -> { - final SetJoin join = targetRoot.join(JpaTargetType_.targets); - return cb.equal(join.get(JpaTarget_.controllerId), controllerId); - }; - } - - /** - * {@link Specification} for retrieving {@link TargetType}s by controllerId - * - * @param controllerIds - * to search for - * - * @return the {@link TargetType} {@link Specification} - */ - public static Specification hasTargetControllerIdIn(final Collection controllerIds) { - return (targetRoot, query, cb) -> { - final SetJoin join = targetRoot.join(JpaTargetType_.targets); - return join.get(JpaTarget_.controllerId).in(controllerIds); - }; - } - - /** - * {@link Specification} for retrieving {@link TargetType}s by controllerId - * - * @param ids - * to search for - * + * @param ids to search for * @return the {@link TargetType} {@link Specification} */ public static Specification hasIdIn(final Collection ids) { @@ -118,12 +39,9 @@ public final class TargetTypeSpecification { } /** - * {@link Specification} for retrieving {@link TargetType}s based on a - * {@link DistributionSetType} name. - * - * @param dsTypeId - * to search for + * {@link Specification} for retrieving {@link TargetType}s based on a {@link DistributionSetType} name. * + * @param dsTypeId to search for * @return the {@link TargetType} {@link Specification} */ public static Specification hasDsSetType(final Long dsTypeId) { @@ -138,8 +56,7 @@ public final class TargetTypeSpecification { * given {@link TargetType#getKey()} including fetching the * elements list. * - * @param key - * to search + * @param key to search * @return the {@link TargetType} {@link Specification} */ public static Specification hasKey(final String key) { @@ -151,8 +68,7 @@ public final class TargetTypeSpecification { * given {@link TargetType#getName()} including fetching the * elements list. * - * @param name - * to search + * @param name to search * @return the {@link TargetType} {@link Specification} */ public static Specification hasName(final String name) { @@ -162,11 +78,10 @@ public final class TargetTypeSpecification { /** * {@link Specification} for retrieving {@link TargetType}s by "like name". * - * @param name - * to be filtered on + * @param name to be filtered on * @return the {@link TargetType} {@link Specification} */ public static Specification likeName(final String name) { return (targetRoot, query, cb) -> cb.like(cb.lower(targetRoot.get(JpaTargetType_.name)), name.toLowerCase()); } -} +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TargetTypeAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TargetTypeAccessControllerTest.java index 66647c50a..e42e6b77e 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TargetTypeAccessControllerTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/TargetTypeAccessControllerTest.java @@ -67,31 +67,6 @@ class TargetTypeAccessControllerTest extends AbstractAccessControllerTest { assertThat(targetTypeManagement.findByRsql(Pageable.unpaged(), "id==*").get().map(Identifiable::getId).toList()) .containsOnly(permittedTargetType.getId()); - // verify targetTypeManagement#findByTargetControllerId - assertThat(targetTypeManagement.findByTargetControllerId(targetWithPermittedTargetType.getControllerId())) - .hasValueSatisfying(foundType -> assertThat(foundType.getId()).isEqualTo(permittedTargetType.getId())); - assertThat(targetTypeManagement.findByTargetControllerId(targetWithHiddenTargetType.getControllerId())) - .isEmpty(); - - // verify targetTypeManagement#findByTargetControllerIds - assertThat( - targetTypeManagement - .findByTargetControllerIds(Arrays.asList(targetWithPermittedTargetType.getControllerId(), - targetWithHiddenTargetType.getControllerId())) - .stream().map(Identifiable::getId).toList()) - .hasSize(1).containsOnly(permittedTargetType.getId()); - - // verify targetTypeManagement#findByTargetId - assertThat(targetTypeManagement.findByTargetId(targetWithPermittedTargetType.getId())) - .hasValueSatisfying(foundType -> assertThat(foundType.getId()).isEqualTo(permittedTargetType.getId())); - assertThat(targetTypeManagement.findByTargetId(targetWithHiddenTargetType.getId())).isEmpty(); - - // verify targetTypeManagement#findByTargetIds - assertThat(targetTypeManagement - .findByTargetIds( - Arrays.asList(targetWithPermittedTargetType.getId(), targetWithHiddenTargetType.getId())) - .stream().map(Identifiable::getId).toList()).hasSize(1).containsOnly(permittedTargetType.getId()); - // verify targetTypeManagement#findByName assertThat(targetTypeManagement.findByName(Pageable.unpaged(), permittedTargetType.getName()).getContent()) .hasSize(1).satisfies(results -> {