35023 task implement Bundle management Rest apis

[hassandotcms]

New REST API endpoints for managing bundle assets, replacing legacy RemotePublishAjaxAction.addToBundle() and JSP-based asset deletion.

• POST /v1/bundles/assets — Add assets to a bundle (resolve by ID → name → auto-create)
• DELETE /v1/bundles/{bundleId}/assets — Remove assets from an unpushed bundle (bulk, per-asset results)

Details

• BundleManagementResource with Lazy DI, WebResource auth, Swagger/OpenAPI annotations
• Partial success model: permission errors reported per-asset, don't fail the request
• DELETE guards against in-progress bundles (409) and validates asset existence in queue
• Forms: AddAssetsToBundleForm (Validated), RemoveAssetsFromBundleForm (Immutables)
• Views: AddAssetsToBundleView, RemoveAssetResultView (Immutables + ResponseEntityView wrappers)

Test plan

• Integration tests cover: add by ID, by name, auto-create, empty/null validation (400)
• Integration tests cover: single/multi removal, last-asset audit cleanup, not-found (404), in-progress (409), asset-not-in-bundle (success=false

Resolves #34325 #35023

Screenshots

Original	Updated
** original screenshot **	** updated screenshot **

---

Note

Medium Risk
Adds new REST endpoints that create/modify bundle queue contents and enforce publishing-status guards; mistakes could impact publishing workflows or bundle integrity. Covered by new integration tests, but behavior changes (idempotent add, per-asset partial failures) should be reviewed for compatibility with existing clients.

Overview
Adds a new BundleManagementResource under /v1/bundles with POST /assets to add assets to an existing or auto-created bundle (resolve by bundleId → unsent bundleName → create), returning AddAssetsToBundleView with partial-success errors and skipping duplicates for idempotency.

Adds DELETE /{bundleId}/assets to bulk-remove assets from a bundle with per-asset RemoveAssetResultView responses, including validation, “not in bundle” handling, and 409 guards when the bundle is in an in-progress publishing status.

Updates openapi.yaml to document the new endpoints/schemas and adds BundleManagementResourceIntegrationTest (wired into MainSuite2b) to cover success paths and 400/404/409 edge cases.

^{Reviewed by Cursor Bugbot for commit 6f44b39. Bugbot is set up for automated code reviews on this repo. Configure here.}

This PR fixes: #35023

[claude]

Claude finished @hassandotcms's task in 2m 35s —— View job

---

Code Review

• Gather context
• Read BundleManagementResource.java
• Read form/view/response classes
• Read integration test
• Post review

---

Summary

Overall solid implementation — the partial-success model, idempotent duplicate skipping, in-progress guard, and per-asset result views are all well-designed. A few real issues worth addressing before merge:

---

Issues

1. No bundle ownership / authorization check (Medium)

addAssetsToBundle and removeAssetsFromBundle only verify the requester is a backend user. Any authenticated backend user can add or remove assets from any bundle, including bundles owned by other users. The bundle.getOwner() field is available; at minimum consider checking user.isAdmin() || bundle.getOwner().equals(user.getUserId()). Compare: the legacy RemotePublishAjaxAction.addToBundle() enforces permission checks. If there is an existing permission API for bundles, use it.

2. Race condition in bundle name lookup + auto-creation (Medium)

BundleManagementResource.java:214-232 — The lookup in step 2b and creation in step 2c are not atomic. Two concurrent POST /v1/bundles/assets requests with the same bundleName and no existing bundle will both miss the lookup and both create separate bundles. If two bundles with identical names is acceptable the current behaviour is fine, but this should be explicitly documented in the Javadoc and Swagger description, because the current text implies lookup-then-create is deterministic.

3. AddAssetsToBundleForm.checkValid() mutates field state (Low)

AddAssetsToBundleForm.java:77-79 — checkValid() reassigns this.assetIds (filters blanks in place). A caller that reads getAssetIds() before calling checkValid() sees different data than after. This is especially surprising because Validated.checkValid() is semantically a guard, not a transform. The filtering belongs in a constructor or getter, not checkValid(). Also: if checkValid() is called twice (shouldn't be but is possible), the second call is a no-op only because the list is already filtered, which is fragile.

4. AddAssetsToBundleView.total semantics are confusing (Low)

AbstractAddAssetsToBundleView.java:74 — The field counts "non-duplicate assets processed" and the Javadoc says "subtract errors().size() to get successful adds". This requires clients to do math and understand the internal semantics. Consider naming the field processed and adding a separate added count (= total - errors.size()), or just returning added and failed directly. The current shape is hard to consume correctly from a REST client.

5. resultMap unchecked cast silently swallows failures (Low)

BundleManagementResource.java:280-282:

final List<String> errorMessages = resultMap.get("errorMessages") instanceof List
        ? (List<String>) resultMap.get("errorMessages")
        : List.of();

If PublisherAPIImpl.saveBundleAssets ever changes the key name or type (it currently uses "errorMessages" consistently), this falls back to an empty list and errors are silently dropped. The "total" fallback at line 285 has the same issue. At minimum, add a Logger.warn when the expected keys are absent.

6. Double stream over assetIds (Minor)

BundleManagementResource.java:253-260 — form.getAssetIds() is streamed twice (once for newAssets, once for duplicateAssets). Use Collectors.partitioningBy instead:

Map<Boolean, List<String>> partitioned = form.getAssetIds().stream()
    .collect(Collectors.partitioningBy(id -> !existingAssets.contains(id)));
List<String> newAssets = partitioned.get(true);
List<String> duplicateAssets = partitioned.get(false);

7. Hardcoded page size of 1000 in getUnsendBundlesByName (Minor)

BundleManagementResource.java:216-217 — If a user has >1000 unsent bundles, the name lookup silently fails to find the correct bundle and auto-creates a duplicate. Either paginate fully (loop until empty), use Integer.MAX_VALUE, or add a comment documenting this known limitation.

8. Swagger 404 description is internally contradictory (Minor)

BundleManagementResource.java:153-157 — The @ApiResponse(responseCode = "404") description says "bundleId provided but not found and no bundleName fallback", but the @Operation description says "A non-matching bundleId does NOT return 404". Both are technically correct but read as contradictory at a glance. Restate the 404 condition precisely: "404 is only returned when bundleId is provided, resolves to nothing, AND bundleName is absent."

9. Test asserts on an internal error-message substring (Minor)

BundleManagementResourceIntegrationTest.java:229-230:

assertTrue("Error should mention permission denial",
        view.errors().get(0).contains("does not have Publish Permission"));

This couples the test to the exact wording inside PublisherAPIImpl. Use assertFalse(view.errors().isEmpty()) and skip the substring check, or at minimum add a comment linking to the source of that string so it's easy to find if it changes.

---

Already Fixed

The Cursor Bugbot NPE report (b.getName().equalsIgnoreCase(...)) was addressed in commit 77f9fc8c — the current code correctly uses form.getBundleName().equalsIgnoreCase(b.getName()) which handles null bundle names safely.

[claude]

Rollback Safety Analysis

• Read rollback-unsafe categories reference
• Get full PR diff
• Analyze diff against all unsafe categories
• Apply rollback safety label

---

Result: ✅ Safe to Rollback

This PR is purely additive — no changes match any rollback-unsafe category.

Analysis by Category

Category	Match?	Notes
C-1 Structural Data Model Change	✅ No	No DB schema changes, no runonce tasks
C-2 Elasticsearch Mapping Change	✅ No	No ESMapping* file changes, no reindex tasks
C-3 Content JSON Model Version Bump	✅ No	No CURRENT_MODEL_VERSION changes
C-4 DROP TABLE / DROP COLUMN	✅ No	No DROP statements
H-1 One-Way Data Migration	✅ No	No data transformation tasks
H-2 RENAME TABLE / RENAME COLUMN	✅ No	No rename statements
H-3 PK Restructuring	✅ No	No primary key changes
H-4 New Content Type Field Type	✅ No	No new field types registered
H-5 Binary Storage Provider Change	✅ No	No storage provider changes
H-6 DROP PROCEDURE / DROP FUNCTION	✅ No	No stored procedure changes
H-7 NOT NULL Column Without Default	✅ No	No column additions
M-1 Non-Broadening Column Type Change	✅ No	No column type changes
M-2 Push Publishing Bundle Format Change	✅ No	No Bundler/Handler XML format changes
M-3 REST / GraphQL API Contract Change	✅ No	Only new endpoints added (additive)
M-4 OSGi Plugin API Breakage	✅ No	No interface signature changes

What This PR Changes

• 7 new Java files: BundleManagementResource, form/view/response classes — all brand new, no modifications to existing code
• openapi.yaml: Adds documentation for POST /v1/bundles/assets and DELETE /v1/bundles/{bundleId}/assets — purely additive
• MainSuite2b.java: Adds BundleManagementResourceIntegrationTest to the test suite
• New integration test file: 535-line test class

Rolling back from N to N-1 simply removes these new endpoints. No data is at risk, no existing functionality breaks, and no clients could have built against these new endpoints yet.

View job run

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is kicking off a free cloud agent to fix this issue. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 6f44b39. Configure here.}