Introduce soft deleted list option for soft deletable entities#3093
Introduce soft deleted list option for soft deletable entities#3093strailov wants to merge 5 commits into
Conversation
Signed-off-by: strailov <Stanislav.Trailov@bosch.io>
Signed-off-by: strailov <Stanislav.Trailov@bosch.io>
Signed-off-by: strailov <Stanislav.Trailov@bosch.io>
Signed-off-by: strailov <Stanislav.Trailov@bosch.io>
Signed-off-by: strailov <Stanislav.Trailov@bosch.io>
| * The request parameter for specifying the soft deletion listing of entities. The value of this parameter | ||
| * can be soft_deleted, not_soft_deleted or all | ||
| */ | ||
| public static final String REQUEST_PARAMETER_LIST_SOFT_DELETED_MODE = "soft_deleted_mode"; |
There was a problem hiding this comment.
The naming of the 3 modes maybe should be reconsidered. What about:
SOFT_DELETED: INCLUDE (all), ONLY (only soft deleted), EXCLUDE (only not deleted)?
| "'soft_deleted' - returns only soft-deleted rollouts; " + | ||
| "'all' - returns both active and soft-deleted rollouts.", | ||
| allowableValues = { "not_soft_deleted", "soft_deleted", "all" }) | ||
| String softDeletedMode); |
There was a problem hiding this comment.
why string? It should be enumeration, e.g. SoftDeltedMode:
INCLUDE, EXCLUDE, ALL/ANY, or (with current naming)
SOFT_DELETED, NOT_SOFT_DELETED, ALL/ANY
| LogUtility.logDeprecated("Usage of MgmtDistributionSetResource.getActions with 'complete': 'complete' distribution set search field is limited and may be removed."); | ||
| } | ||
| final Pageable pageable = PagingUtility.toPageable(pagingOffsetParam, pagingLimitParam, sanitizeDistributionSetSortParam(sortParam)); | ||
| final SoftDeletedFilter softDeletedFilter = SoftDeletedFilter.fromValue(softDeletedModeParam) |
There was a problem hiding this comment.
Maybe SoftDeltedMode? "Filter" implies something like RSQL in scope of hawkbit
There was a problem hiding this comment.
Yeah, I am open for discussion for the names ... I left it softDeletedMode on the Rest API side, but on the implementation side it is still a filter though which goes directly to the db ... So that's why currently I left it that way.
| METADATA("metadata"), | ||
| VALID("valid"); | ||
| VALID("valid"), | ||
| DELETED("deleted"); |
There was a problem hiding this comment.
do we want to add deleted field? if we add deleted field we could query with RSQL deleted=true and soft deleted mode "not deleted"? which will take prefernce? or will let user to be responsible for collisions?
There was a problem hiding this comment.
I guess this is the price if we want sort option by deleted if ALL is applied ... Personally I would not include that and not include sort at all.
| return filter(JpaManagementHelper.findAllWithCountBySpec(jpaRepository, specList, pageable), completed); | ||
| } | ||
| } | ||
| return findByRsqlAndDeleted(rsql, SoftDeletedFilter.NOT_SOFT_DELETED, pageable); |
There was a problem hiding this comment.
just idea - what about if we don't add rest soft deleted mode, but just DELETED filter fields?
And here just check - if RSQL contains (parse the filter) deleted=** - we dont add in specification deleted=false.
if don't - like now - we add deleted = false?
This could keep the changes and the api expansion really small as keeping all new features.
There was a problem hiding this comment.
I think it will be almost the same change - it won't be that small - we still have to filter the entities through the database (which currently is the biggest change), so we will skip maybe 50% of the Controller layer which i don't think is that big now... Also the solution won't be that clean, it feels like a stitch. Idk ...
2 participants
Add
soft_deleted_modequery parameter to listing REST endpointsSummary
Introduces a
soft_deleted_modefilter parameter to all soft-deletable entity listing endpoints, allowing API consumers to query soft-deleted entities that were previously hidden from listing results.Motivation
Soft-deleted entities (Distribution Sets, Software Modules, their Types, and Rollouts) were previously only accessible by direct ID lookup. There was no way to list or search them via the REST API, making it difficult for UI and integrations to display or manage deleted resources.
Changes
New API contract:
SoftDeletedFilterenum (soft_deleted,not_soft_deleted,all) with safefromValue()parsingSoftDeletableRepositoryManagementinterface extendingRepositoryManagementwithfindAll(SoftDeletedFilter, Pageable)andfindByRsql(String, SoftDeletedFilter, Pageable)count(SoftDeletedFilter)added toRepositoryManagementwith fallback for non-soft-deletable entitiesAffected REST endpoints:
GET /rest/v1/distributionsetsGET /rest/v1/distributionsettypesGET /rest/v1/softwaremodulesGET /rest/v1/softwaremoduletypesGET /rest/v1/rollouts(compact mode only; full mode intentionally excluded)Mutation guards:
update,lock,unlock,invalidate,assignTag,unassignTagnow reject operations on soft-deleted entities withDeletedExceptionImplementations:
JpaDistributionSetManagement— refactoredfindByRsqlto support deleted filter while preserving legacycompletefield handlingJpaSoftwareModuleManagement,JpaSoftwareModuleTypeManagement,JpaDistributionSetTypeManagement— addedfindAll/findByRsqlwith soft-delete filtering and update guardsJpaRolloutManagement— addedfindAll/findByRsqlwithSoftDeletedFilterAbstractJpaRepositoryManagementdue to that some entities are not soft-deletable by definition. So the current approach may have some duplications withfindAllorfindByRsqlimplementations but seems with more clear architecture approach and distinguish those entities.AbstractJpaRepositoryManagement→AbstractJpaRepositoryWithMetadataManagement- Would need two variants (with/without metadata) or force unrelated coupling. Not worth the complexity. Decided to override the updates for each entity which is easy to review, no risk for other entities and straightforward .