File support improvements#297
Conversation
There was a problem hiding this comment.
Pull request overview
Improves file handling across the UI and backend repository access so that file inputs can work with both ACM-managed uploads (/var/acm/file/...) and existing AEM DAM assets (/content/dam/...), addressing issues #287 and #288.
Changes:
- Frontend: introduces a managed-file root check and updates the uploader UI to treat managed vs unmanaged file paths differently.
- Backend: extends repository file-reading to resolve DAM “original rendition” binaries and adds binary-data detection utilities.
- Backend: adds a constants holder for DAM-related paths and tightens deletion logic to the managed file root.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ui.frontend/src/types/main.ts | Adds FileRoot and isFileManaged() helper for distinguishing ACM-managed files. |
| ui.frontend/src/components/FileUploader.tsx | Uses isFileManaged() to control whether delete actions are available. |
| core/src/main/java/dev/vml/es/acm/core/util/JcrUtils.java | Adds helper to locate binary data properties on nodes (incl. DAM-like structures). |
| core/src/main/java/dev/vml/es/acm/core/util/AemConstants.java | Introduces DAM path constants used by repo resolution logic. |
| core/src/main/java/dev/vml/es/acm/core/repo/RepoResource.java | Updates file resolution/stream reading to support DAM assets by finding original rendition binary content. |
| core/src/main/java/dev/vml/es/acm/core/code/FileManager.java | Centralizes managed root constant and adds a managed-root deletion guard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| } | ||
| if (!StringUtils.startsWith(resource.getPath(), root.getPath() + "/")) { | ||
| throw new AcmException(String.format("File is outside managed root and cannot be deleted '%s'!", path)); | ||
| } |
There was a problem hiding this comment.
delete() adds an “outside managed root” guard, but findResource() only ever returns resources under root (it only resolves absolute paths when they already start with root.getPath() + "/"). As a result, attempts to delete /content/dam/... will still hit the "does not exist" branch and this new check will never trigger. If you want a clear error for unmanaged absolute paths, either (1) resolve absolute paths in findResource() when path starts with / and then enforce the root-prefix check here, or (2) add a fast pre-check on the input path against ROOT before calling findResource().
core/src/main/java/dev/vml/es/acm/core/code/input/AbstractFileInput.java
Show resolved
Hide resolved
| return Optional.of(new RepoResource(repo, damContent.getPath())); | ||
| } | ||
| } | ||
| if (JcrUtils.hasBinaryData(contentResource.adaptTo(Node.class))) { |
There was a problem hiding this comment.
this simply behaves like Sling adapter for InputStream ; but unfortunately the original method is internal to Sling to copied-adapted to ACM to have 1-to-1 compatibility
fixes #286, #287, #288
bug fixes HC and log printing