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 <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2024-07-18 18:13:03 +03:00
committed by GitHub
parent bb288eab6b
commit 56ab399493
7 changed files with 33 additions and 249 deletions

View File

@@ -123,38 +123,6 @@ public interface TargetTypeManagement {
@PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET)
Optional<TargetType> get(long id);
/**
* @param targetId
* Target ID
* @return Target Type
*/
@PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET)
Optional<TargetType> findByTargetId(long targetId);
/**
* @param targetIds
* List of Target ID
* @return Target Type
*/
@PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET)
List<TargetType> findByTargetIds(Collection<Long> targetIds);
/**
* @param controllerId
* Target controller ID
* @return Target Type
*/
@PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET)
Optional<TargetType> findByTargetControllerId(String controllerId);
/**
* @param controllerIds
* List of Target controller ID
* @return Target Type
*/
@PreAuthorize(SpPermission.SpringEvalExpressions.HAS_AUTH_READ_TARGET)
List<TargetType> findByTargetControllerIds(Collection<String> controllerIds);
/**
* @param ids
* List of Target type ID

View File

@@ -33,11 +33,6 @@ public interface TargetType extends Type {
*/
Set<DistributionSetType> getCompatibleDistributionSetTypes();
/**
* @return immutable set of optional {@link Target}s
*/
Set<Target> getTargets();
/**
* Checks if the given {@link DistributionSetType} is in
* {@link #getCompatibleDistributionSetTypes()}.

View File

@@ -166,30 +166,6 @@ public class JpaTargetTypeManagement implements TargetTypeManagement {
return targetTypeRepository.findById(id).map(TargetType.class::cast);
}
@Override
public Optional<TargetType> findByTargetId(final long targetId) {
return targetTypeRepository.findOne(TargetTypeSpecification.hasTarget(targetId)).map(TargetType.class::cast);
}
@Override
public List<TargetType> findByTargetIds(final Collection<Long> targetIds) {
return targetTypeRepository
.findAll(TargetTypeSpecification.hasTarget(targetIds)).stream().map(TargetType.class::cast).toList();
}
@Override
public Optional<TargetType> findByTargetControllerId(final String controllerId) {
return targetTypeRepository
.findOne(TargetTypeSpecification.hasTargetControllerId(controllerId)).map(TargetType.class::cast);
}
@Override
public List<TargetType> findByTargetControllerIds(final Collection<String> controllerIds) {
return targetTypeRepository
.findAll(TargetTypeSpecification.hasTargetControllerIdIn(controllerIds))
.stream().map(TargetType.class::cast).toList();
}
@Override
public List<TargetType> get(final Collection<Long> ids) {
return Collections.unmodifiableList(targetTypeRepository.findAllById(ids));

View File

@@ -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<DistributionSetType> distributionSetTypes;
@OneToMany(targetEntity = JpaTarget.class, mappedBy = "targetType", fetch = FetchType.LAZY)
private Set<Target> targets;
/**
* Constructor
*/
public JpaTargetType() {
// default public constructor for JPA
}
/**
* Constructor, legacy support where <code>key</code> is set to passed <code>name</code>.
*
* @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<DistributionSetType> 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<Target> 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()));
}
}
}

View File

@@ -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);
};
}
}
}

View File

@@ -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<JpaTargetType> 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<JpaTargetType> hasTarget(final long id) {
return (targetRoot, query, cb) -> {
final SetJoin<JpaTargetType, JpaTarget> 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<JpaTargetType> hasTarget(final Collection<Long> ids) {
return (targetRoot, query, cb) -> {
final SetJoin<JpaTargetType, JpaTarget> 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<JpaTargetType> hasTargetControllerId(final String controllerId) {
return (targetRoot, query, cb) -> {
final SetJoin<JpaTargetType, JpaTarget> 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<JpaTargetType> hasTargetControllerIdIn(final Collection<String> controllerIds) {
return (targetRoot, query, cb) -> {
final SetJoin<JpaTargetType, JpaTarget> 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<JpaTargetType> hasIdIn(final Collection<Long> 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<JpaTargetType> 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<JpaTargetType> 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<JpaTargetType> 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<JpaTargetType> likeName(final String name) {
return (targetRoot, query, cb) -> cb.like(cb.lower(targetRoot.get(JpaTargetType_.name)), name.toLowerCase());
}
}
}

View File

@@ -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 -> {