-
Notifications
You must be signed in to change notification settings - Fork 191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Repository API query signatures Entity free #403
Repository API query signatures Entity free #403
Conversation
Signed-off-by: kaizimmerm <[email protected]>
Signed-off-by: kaizimmerm <[email protected]>
Conflicts: hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetManagement.java hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/management/footer/ManangementConfirmationWindowLayout.java
Signed-off-by: kaizimmerm <[email protected]>
Signed-off-by: kaizimmerm <[email protected]>
Signed-off-by: kaizimmerm <[email protected]>
Signed-off-by: kaizimmerm <[email protected]>
Signed-off-by: kaizimmerm <[email protected]>
Signed-off-by: kaizimmerm <[email protected]>
Signed-off-by: kaizimmerm <[email protected]>
Conflicts: hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TargetManagement.java
final List<Target> allTargets = Collections.unmodifiableList(targetRepository | ||
.findAll(TargetSpecifications.byControllerIdWithStatusAndTagsInJoin(Arrays.asList(controllerID)))); | ||
final List<Target> unAssignTag = unAssignTag(allTargets, targetTag); | ||
final List<Target> unAssignTag = unAssignTag(allTargets, | ||
Optional.ofNullable(targetTagRepository.findOne(targetTagId)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't call the database within a nested list of method parameters
public void deleteTargets(final Collection<Long> targetIDs) { | ||
targetRepository.deleteByIdIn(targetIDs); | ||
|
||
targetIDs.forEach(targetId -> eventPublisher.publishEvent(new TargetDeletedEvent(tenantAware.getCurrentTenant(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why it is necessary to fire the deletion event?
Normally the JpaTarget#fireDeleteEvent
should fire the event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, not in is this case as this is a delete query not an entity manager delete.
* @return a page of the found {@link Target}s | ||
*/ | ||
@PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_TARGET) | ||
Page<Target> findAllTargetsByTargetFilterQueryAndNonDS(@NotNull Pageable pageRequest, Long distributionSetId, | ||
@NotNull TargetFilterQuery targetFilterQuery); | ||
@NotNull String rsqlParam); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a empty string allowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. We use normally NotNull for these but I guess we might switch to not empty one. However, I am not fully sure concerning the consequences.
* @return a page of the found {@link Target}s | ||
*/ | ||
@PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_TARGET) | ||
Page<Target> findAllTargetsByTargetFilterQueryAndNotInRolloutGroups(@NotNull Pageable pageRequest, | ||
List<RolloutGroup> groups, @NotNull String targetFilterQuery); | ||
@NotEmpty Collection<Long> groups, @NotNull String rsqlParam); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a empty string allowed?
* to delete | ||
*/ | ||
@PreAuthorize(SpringEvalExpressions.HAS_AUTH_DELETE_REPOSITORY) | ||
void deleteDistributionSetType(@NotNull DistributionSetType type); | ||
void deleteDistributionSetType(@NotNull Long typeId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this method and some other throws a EntityNotFoudException
. I think it is helpfull to adapt the javadoc
@Query(value = "SELECT DISTINCT t FROM JpaTarget t JOIN t.tags tt WHERE tt = :tag") | ||
List<JpaTarget> findByTag(@Param("tag") final JpaTargetTag tag); | ||
@Query(value = "SELECT DISTINCT t FROM JpaTarget t JOIN t.tags tt WHERE tt.id = :tag") | ||
List<JpaTarget> findByTag(@Param("tag") final Long tag); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tag is now a tagId. I think it's better if you rename it
@@ -196,5 +194,5 @@ void setAssignedDistributionSet(@Param("set") JpaDistributionSet set, @Param("la | |||
* the page request parameter | |||
* @return a page of all targets related to a rollout group | |||
*/ | |||
Page<Target> findByActionsRolloutGroup(JpaRolloutGroup rolloutGroup, Pageable page); | |||
Page<Target> findByActionsRolloutGroupId(Long rolloutGroup, Pageable page); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
Signed-off-by: kaizimmerm <[email protected]>
Signed-off-by: kaizimmerm <[email protected]>
Signed-off-by: kaizimmerm <[email protected]>
Conflicts: hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiDeploymentBaseTest.java
Signed-off-by: kaizimmerm <[email protected]>
* Migrated target management queries to IDs inetsead of full entities Signed-off-by: kaizimmerm <[email protected]> * Added missing comment. Signed-off-by: kaizimmerm <[email protected]> * refactored target,DS,cont,deploy,rg mangement. Signed-off-by: kaizimmerm <[email protected]> * Adde versioning documentation. Signed-off-by: kaizimmerm <[email protected]> * Rollout, Dist and Software mgmt refactored Signed-off-by: kaizimmerm <[email protected]> * Readded line that was remove by incident. Signed-off-by: kaizimmerm <[email protected]> * Fixed broken tests. Signed-off-by: kaizimmerm <[email protected]> * Query management refactored Signed-off-by: kaizimmerm <[email protected]> * Fix bug of auto assign DS delete Signed-off-by: kaizimmerm <[email protected]> * Switch to collection Signed-off-by: kaizimmerm <[email protected]> * Fixed compile error Signed-off-by: kaizimmerm <[email protected]> * Small glitches Signed-off-by: kaizimmerm <[email protected]> * Fixed test after merge Signed-off-by: kaizimmerm <[email protected]>
Another task of #197 is to use entity IDs in queries and not the complete (JPA) object.
This makes the repository API much more JPA independent and reduces the risk even in JPA that you use and change an object in an transaction that has been changed in the meantime.