Merge pull request #45 from bsinno/DistributionSetAssignmentResult-co…

…nsistency-fix

ok, merging. Thanks.

hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/ActionRepository.java

@@ -37,19 +37,9 @@
  */
 @Transactional(readOnly = true)
 public interface ActionRepository extends BaseEntityRepository<Action, Long>, JpaSpecificationExecutor<Action> {
-
-    /*
-     * (non-Javadoc)
-     * 
-     * @see org.springframework.data.repository.CrudRepository#findAll()
-     */
-    @Override
-    @EntityGraph(value = "Action.all", type = EntityGraphType.LOAD)
-    Iterable<Action> findAll();
-
     /**
      * Retrieves an Action with all lazy attributes.
-     * 
+     *
      * @param actionId
      *            the ID of the action
      * @return the found {@link Action}
@@ -67,13 +57,12 @@ public interface ActionRepository extends BaseEntityRepository<Action, Long>, Jp
      *            the {@link DistributionSet} on which will be filtered
      * @return the found {@link Action}s
      */
-    @EntityGraph(value = "Action.all", type = EntityGraphType.LOAD)
     Page<Action> findByDistributionSet(final Pageable pageable, final DistributionSet ds);
 
     /**
      * Retrieves all {@link Action}s which are referring the given
      * {@link Target}.
-     * 
+     *
      * @param pageable
      *            page parameters
      * @param target
@@ -84,8 +73,9 @@ public interface ActionRepository extends BaseEntityRepository<Action, Long>, Jp
 
     /**
      * Retrieves all {@link Action}s which are active and referring the given
-     * {@link Target} in a specified order.
-     * 
+     * {@link Target} in a specified order. Loads also the lazy
+     * {@link Action#getDistributionSet()} field.
+     *
      * @param pageable
      *            page parameters
      * @param target
@@ -125,14 +115,13 @@ List<Action> findActionByTargetAndSoftwareModule(@Param("target") final String t
      * @return the found {@link UpdateAction}s
      */
     @Query("Select a from Action a where a.target = :target and a.distributionSet = :ds order by a.id")
-    @EntityGraph(value = "Action.all", type = EntityGraphType.LOAD)
     Page<Action> findByTargetAndDistributionSet(final Pageable pageable, @Param("target") final Target target,
             @Param("ds") DistributionSet ds);
 
     /**
      * Retrieves all {@link Action}s of a specific target, without pagination
      * ordered by action ID.
-     * 
+     *
      * @param target
      *            to search for
      * @return a list of actions according to the searched target
@@ -143,7 +132,7 @@ Page<Action> findByTargetAndDistributionSet(final Pageable pageable, @Param("tar
     /**
      * Retrieves all {@link Action}s of a specific target and given active flag
      * ordered by action ID.
-     * 
+     *
      * @param pageable
      *            the pagination parameter
      * @param target
@@ -159,8 +148,9 @@ Page<Action> findByActiveAndTarget(Pageable pageable, @Param("target") Target ta
 
     /**
      * Retrieves all {@link Action}s of a specific target and given active flag
-     * ordered by action ID.
-     * 
+     * ordered by action ID. Loads also the lazy
+     * {@link Action#getDistributionSet()} field.
+     *
      * @param target
      *            to search for
      * @param active
@@ -174,7 +164,7 @@ Page<Action> findByActiveAndTarget(Pageable pageable, @Param("target") Target ta
 
     /**
      * Updates all {@link Action} to inactive for all targets with given ID.
-     * 
+     *
      * @param keySet
      *            the list of actions to set inactive
      * @param targetsIds
@@ -190,7 +180,7 @@ Page<Action> findByActiveAndTarget(Pageable pageable, @Param("target") Target ta
      * Switches the status of actions from one specific status into another,
      * only if the actions are in a specific status. This should be a atomar
      * operation.
-     * 
+     *
      * @param statusToSet
      *            the new status the actions should get
      * @param targetIds
@@ -210,7 +200,7 @@ void switchStatus(@Param("statusToSet") Action.Status statusToSet, @Param("targe
      * Switches the status of actions from one specific status into another,
      * only if the actions are in a specific status. This should be a atomar
      * operation.
-     * 
+     *
      * @param statusToSet
      *            the new status the actions should get
      * @param rollout
@@ -227,7 +217,7 @@ void switchStatus(@Param("statusToSet") Action.Status statusToSet, @Param("rollo
             @Param("active") boolean active, @Param("currentStatus") Action.Status currentStatus);
 
     /**
-     * 
+     *
      * Retrieves all {@link Action}s which are active and referring to the given
      * target Ids and distribution set required migration step.
      *
@@ -243,36 +233,24 @@ List<Action> findByActiveAndTargetIdInAndActionStatusNotEqualToAndDistributionSe
 
     /**
      * Counts all {@link Action}s referring to the given target.
-     * 
+     *
      * @param target
      *            the target to count the {@link Action}s
      * @return the count of actions referring to the given target
      */
     Long countByTarget(Target target);
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see org.springframework.data.repository.CrudRepository#save(java.lang.
-     * Iterable)
-     */
     @Override
     @CacheEvict(value = "feedbackReceivedOverTime", allEntries = true)
     <S extends Action> List<S> save(Iterable<S> entities);
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see
-     * org.springframework.data.repository.CrudRepository#save(java.lang.Object)
-     */
     @Override
     @CacheEvict(value = "feedbackReceivedOverTime", allEntries = true)
     <S extends Action> S save(S entity);
 
     /**
      * Counts all {@link Action}s referring to the given DistributionSet.
-     * 
+     *
      * @param distributionSet
      *            DistributionSet to count the {@link Action}s from
      * @return the count of actions referring to the given distributionSet
@@ -281,7 +259,7 @@ List<Action> findByActiveAndTargetIdInAndActionStatusNotEqualToAndDistributionSe
 
     /**
      * Counts all {@link Action}s referring to the given rollout.
-     * 
+     *
      * @param rollout
      *            the rollout to count the {@link Action}s from
      * @return the count of actions referring to the given rollout
@@ -293,7 +271,7 @@ List<Action> findByActiveAndTargetIdInAndActionStatusNotEqualToAndDistributionSe
      * are currently not in the given status. An in-clause statement does not
      * work with the spring-data, so this is specific usecase regarding to the
      * rollout-management to find out actions which are not in specific states.
-     * 
+     *
      * @param rollout
      *            the rollout the actions are belong to
      * @param rolloutGroup
@@ -312,7 +290,7 @@ Long countByRolloutAndRolloutGroupAndStatusNotAndStatusNotAndStatusNot(Rollout r
 
     /**
      * Counts all actions referring to a given rollout and rolloutgroup.
-     * 
+     *
      * @param rollout
      *            the rollout the actions belong to
      * @param rolloutGroup
@@ -323,7 +301,7 @@ Long countByRolloutAndRolloutGroupAndStatusNotAndStatusNotAndStatusNot(Rollout r
 
     /**
      * Counts all actions referring to a given rollout, rolloutgroup and status.
-     * 
+     *
      * @param rolloutId
      *            the ID of rollout the actions belong to
      * @param rolloutGroupId
@@ -338,7 +316,7 @@ Long countByRolloutAndRolloutGroupAndStatusNotAndStatusNotAndStatusNot(Rollout r
     /**
      * Retrieving all actions referring to a given rollout with a specific
      * action as parent reference and a specific status.
-     * 
+     *
      * Finding all actions of a specific rolloutgroup parent relation.
      *
      * @param rollout
@@ -355,7 +333,7 @@ List<Action> findByRolloutAndRolloutGroupParentAndStatus(Rollout rollout, Rollou
 
     /**
      * Retrieves all actions for a specific rollout and in a specific status.
-     * 
+     *
      * @param rollout
      *            the rollout the actions beglong to
      * @param actionStatus
@@ -367,7 +345,7 @@ List<Action> findByRolloutAndRolloutGroupParentAndStatus(Rollout rollout, Rollou
     /**
      * Get list of objects which has details of status and count of targets in
      * each status in specified rollout.
-     * 
+     *
      * @param rolloutId
      *            id of {@link Rollout}
      * @return list of objects with status and target count
@@ -378,7 +356,7 @@ List<Action> findByRolloutAndRolloutGroupParentAndStatus(Rollout rollout, Rollou
     /**
      * Get list of objects which has details of status and count of targets in
      * each status in specified rollout.
-     * 
+     *
      * @param rolloutId
      *            id of {@link Rollout}
      * @return list of objects with status and target count
@@ -389,7 +367,7 @@ List<Action> findByRolloutAndRolloutGroupParentAndStatus(Rollout rollout, Rollou
     /**
      * Get list of objects which has details of status and count of targets in
      * each status in specified rollout group.
-     * 
+     *
      * @param rolloutGroupId
      *            id of {@link RolloutGroup}
      * @return list of objects with status and target count
@@ -400,7 +378,7 @@ List<Action> findByRolloutAndRolloutGroupParentAndStatus(Rollout rollout, Rollou
     /**
      * Get list of objects which has details of status and count of targets in
      * each status in specified rollout group.
-     * 
+     *
      * @param rolloutGroupId
      *            list of id of {@link RolloutGroup}
      * @return list of objects with status and target count

hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/DeploymentManagement.java

@@ -378,11 +378,13 @@ private DistributionSetAssignmentResult assignDistributionSetToTargets(@NotNull
             actionStatusRepository.save(actionStatus);
         });
 
-        // select updated targets in order to return them
+        // flush to get action IDs
+        entityManager.flush();
+        // collect updated target and actions IDs in order to return them
         final DistributionSetAssignmentResult result = new DistributionSetAssignmentResult(
                 targets.stream().map(target -> target.getControllerId()).collect(Collectors.toList()), targets.size(),
-                controllerIDs.size() - targets.size(), Lists.newArrayList(targetIdsToActions.values()),
-                targetManagement);
+                controllerIDs.size() - targets.size(),
+                targetIdsToActions.values().stream().map(Action::getId).collect(Collectors.toList()), targetManagement);
 
         LOG.debug("assignDistribution({}) finished {}", set, result);
 
@@ -391,13 +393,16 @@ private DistributionSetAssignmentResult assignDistributionSetToTargets(@NotNull
         // detaching as it is not necessary to persist the set itself
         entityManager.detach(set);
 
-        // send distribution set assignment event
+        sendDistributionSetAssignmentEvent(targets, targetIdsCancellList, targetIdsToActions, softwareModules);
 
+        return result;
+    }
+
+    private void sendDistributionSetAssignmentEvent(final List<Target> targets, final Set<Long> targetIdsCancellList,
+            final Map<String, Action> targetIdsToActions, final List<SoftwareModule> softwareModules) {
         targets.stream().filter(t -> !!!targetIdsCancellList.contains(t.getId()))
                 .forEach(t -> assignDistributionSetEvent(t, targetIdsToActions.get(t.getControllerId()).getId(),
                         softwareModules));
-
-        return result;
     }
 
     /**
@@ -709,8 +714,8 @@ public Action findAction(@NotNull final Long actionId) {
     }
 
     /**
-     * Get the {@link Action} entity for given actionId with all lazy
-     * attributes.
+     * Get the {@link Action} entity for given actionId with all lazy attributes
+     * (i.e. distributionSet, target, target.assignedDs).
      *
      * @param actionId
      *            to be id of the action

hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/DistributionSetAssignmentResult.java

@@ -10,7 +10,6 @@
 
 import java.util.List;
 
-import org.eclipse.hawkbit.repository.model.Action;
 import org.eclipse.hawkbit.repository.model.AssignmentResult;
 import org.eclipse.hawkbit.repository.model.Target;
 
@@ -19,14 +18,11 @@
  * information of an assignment and how much of the assignment has been done and
  * how much of the assignments had already been existed.
  *
- *
- *
- *
  */
 public class DistributionSetAssignmentResult extends AssignmentResult {
 
     private final List<String> assignedTargets;
-    private final List<Action> actions;
+    private final List<Long> actions;
 
     private final TargetManagement targetManagement;
 
@@ -48,7 +44,7 @@ public class DistributionSetAssignmentResult extends AssignmentResult {
      *
      */
     public DistributionSetAssignmentResult(final List<String> assignedTargets, final int assigned,
-            final int alreadyAssigned, final List<Action> actions, final TargetManagement targetManagement) {
+            final int alreadyAssigned, final List<Long> actions, final TargetManagement targetManagement) {
         super(assigned, alreadyAssigned);
         this.assignedTargets = assignedTargets;
         this.actions = actions;
@@ -63,9 +59,9 @@ public List<Target> getAssignedTargets() {
     }
 
     /**
-     * @return the actionId
+     * @return the actionIds
      */
-    public List<Action> getActions() {
+    public List<Long> getActions() {
         return actions;
     }
 

hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/model/Action.java

@@ -24,6 +24,7 @@
 import javax.persistence.NamedAttributeNode;
 import javax.persistence.NamedEntityGraph;
 import javax.persistence.NamedEntityGraphs;
+import javax.persistence.NamedSubgraph;
 import javax.persistence.OneToMany;
 import javax.persistence.Table;
 import javax.persistence.Transient;
@@ -34,28 +35,22 @@
 
 /**
  * <p>
- * Applicable transition changes of the software {@link SoftwareModule} state of
- * a {@link Target}, e.g. install, uninstall, update, start, stop, and
- * preparations for the transition change, i.e. download.
+ * Applicable transition changes of the {@link SoftwareModule}s state of a
+ * {@link Target}, e.g. install, uninstall, update and preparations for the
+ * transition change, i.e. download.
  * </p>
  *
  * <p>
- * Actions are managed by the SP server (SPS) and applied to the edge controller
- * by the SP controller (SPC). Actions may also be value added commands that are
- * nor directly related to SP, e.g. factory reset.
+ * Actions are managed by the SP server and applied to the targets by the
+ * client.
  * <p>
- *
- *
- *
- *
- *
  */
 @Table(name = "sp_action", indexes = { @Index(name = "sp_idx_action_01", columnList = "tenant,distribution_set"),
         @Index(name = "sp_idx_action_02", columnList = "tenant,target,active"),
         @Index(name = "sp_idx_action_prim", columnList = "tenant,id") })
 @NamedEntityGraphs({ @NamedEntityGraph(name = "Action.ds", attributeNodes = { @NamedAttributeNode("distributionSet") }),
         @NamedEntityGraph(name = "Action.all", attributeNodes = { @NamedAttributeNode("distributionSet"),
-                @NamedAttributeNode("target") }) })
+                @NamedAttributeNode(value = "target", subgraph = "target.ds") }, subgraphs = @NamedSubgraph(name = "target.ds", attributeNodes = @NamedAttributeNode("assignedDistributionSet") ) ) })
 @Entity
 public class Action extends BaseEntity implements Comparable<Action> {
     private static final long serialVersionUID = 1L;