Skip to content

feat(content-drive): add FILEASSET base-type → content-type resolution strategy (#36279)#36317

Open
zJaaal wants to merge 7 commits into
mainfrom
issue-36279-fileasset-base-type-strategy
Open

feat(content-drive): add FILEASSET base-type → content-type resolution strategy (#36279)#36317
zJaaal wants to merge 7 commits into
mainfrom
issue-36279-fileasset-base-type-strategy

Conversation

@zJaaal

@zJaaal zJaaal commented Jun 25, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Resolves #36279.

Content Drive's upload dialog (PR #36243 / #35575) lets users choose Asset vs File — semantically these are base types. But the backend could only resolve a content type from a base type for DOTASSET, so firing the NEW workflow action with baseType: FILEASSET (no contentType) failed with "The content type or base type, is not set or is invalid." This adds the missing FILEASSET resolution strategy and switches the Content Drive frontend to send the base type for both options.

Backend

  • BaseTypeMimeTypeMatcher (new) — the dotAsset mime-matching algorithm extracted into a reusable matcher parameterized by base type + binary field var. DotAssetAPIImpl.tryMatch now delegates to it, so DOTASSET behavior is unchanged.
  • DotAssetBaseTypeToContentTypeStrategyImpl — moved out of BaseTypeToContentTypeStrategyResolver into its own top-level class (per review feedback).
  • FileAssetBaseTypeToContentTypeStrategyImpl (new) — mirrors dotAsset: matches the uploaded file's mime type against the accept (ALLOWED_FILE_TYPES) field variable of the FILEASSET content types, falling back to the system default FileAsset.
  • BaseTypeToContentTypeStrategyResolver — registers both strategies and fixes a latent bug in subscribe(...), which ignored its baseContentType argument and always registered under DOTASSET (it also threw Multiple entries with same key on any second registration).
  • MapToContentletPopulator.getContentTypeInode is intentionally left untouched (resolution happens earlier in ContentHelper.tryToSetContentType).

Frontend

  • Reverted uploadDotAsset to its original 2-arg form (existing callers — browser selector, edit-content — unchanged).
  • Added newContentletByBaseType (DotWorkflowActionsFireService) and uploadFileByBaseType (DotUploadFileService), which send baseType instead of contentType.
  • Content Drive's upload selector now emits DOTASSET / FILEASSET and routes uploads through the new method.
  • Root-level uploads now carry the current site identifier so the asset lands on the site being browsed instead of the backend default host.
Screen.Recording.2026-06-25.at.2.01.55.PM-1.mov

Checklist

  • Tests — backend resolver unit test (subscribe regression + FILEASSET registration, written red-first), FileAsset strategy integration test, and frontend specs for the new methods, base-type selection, and current-site root upload.
  • Frontend: data-access (694) and portlets-content-drive (667) suites + lint pass.
  • Backend: unit test green; modules compile; integration test compiles (execution requires the enterprise-licensed integration environment / CI).

Testing

  1. Build core and run the integration test: ./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false -Dit.test=FileAssetBaseTypeToContentTypeStrategyImplTest
  2. In Content Drive, upload a file and choose File → confirm a FileAsset is created (PDF → document type, image → image type, etc.); choosing Asset still creates a dotAsset.

🤖 Generated with Claude Code

#36279)

Content Drive's upload dialog lets users pick Asset vs File, which are
semantically base types. Previously only DOTASSET could be resolved from a
base type on the backend, so firing the NEW workflow action with
baseType: FILEASSET failed with "content type or base type is not set or is
invalid". This adds the missing FILEASSET resolution and switches the
frontend to send the base type.

Backend:
- Extract the dotAsset mime-matching algorithm into a reusable
  BaseTypeMimeTypeMatcher (parameterized by base type + binary field var);
  DotAssetAPIImpl.tryMatch now delegates to it (DOTASSET behavior unchanged).
- Move DotAssetBaseTypeToContentTypeStrategyImpl out of the resolver into its
  own class and add FileAssetBaseTypeToContentTypeStrategyImpl, which
  mime-matches FILEASSET content types and falls back to the default FileAsset.
- Register both strategies; fix BaseTypeToContentTypeStrategyResolver.subscribe
  which ignored its baseContentType arg and always registered under DOTASSET.

Frontend:
- Revert uploadDotAsset to its original 2-arg form; add uploadFileByBaseType
  (DotUploadFileService) and newContentletByBaseType (DotWorkflowActionsFireService)
  which send baseType instead of contentType.
- Content Drive now emits DOTASSET/FILEASSET and routes uploads through the new
  method; root-level uploads carry the current site identifier so they land on
  the browsed site, not the default host.

Tests: resolver unit test (subscribe regression + FILEASSET registration),
FileAsset strategy integration test, and frontend specs for the new methods,
base-type selection, and current-site root upload.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels Jun 25, 2026
@claude

claude Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

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


Rollback Safety Analysis

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

Result: ✅ Safe To Rollback

All 18 changed files were checked against every rollback-unsafe category. None matched.

Category Verdict
C-1 Structural Data Model Change ✅ No DB schema changes of any kind
C-2 Elasticsearch Mapping Change ✅ No ES mapping or reindex code touched
C-3 Content JSON Model Version Bump ✅ No CURRENT_MODEL_VERSION or ImmutableContentlet changes
C-4 DROP TABLE / DROP COLUMN ✅ No DDL drops
H-1 One-Way Data Migration ✅ No runonce tasks or data transformations
H-2 RENAME TABLE / RENAME COLUMN ✅ No renames
H-3 PK / Unique Constraint Restructuring ✅ No PK changes
H-4 New Content Type Field Type ✅ No new field types registered
H-5 Binary Storage Provider Change ✅ No StoragePersistenceProvider or metadata version changes
H-6 DROP PROCEDURE / FUNCTION ✅ No stored procedure changes
H-7 NOT NULL Column Without Default ✅ No DDL at all
H-8 VTL Viewtool Contract Change ✅ No viewtool classes or returned DTOs modified
M-1 Non-Broadening Column Type Change ✅ No column type changes
M-2 Push Publishing Bundle Format ✅ No *Bundler.java or bundle handler changes
M-3 REST / GraphQL API Contract Change ✅ New frontend methods added (newContentletByBaseType, uploadFileByBaseType) are additive; existing uploadDotAsset signature simplified but Content Drive was the only consumer of the old 3-arg form and is updated in the same PR
M-4 OSGi Plugin API Breakage ✅ No OSGi-exported interface signatures changed

Summary: This PR is purely additive — new Java strategy classes (FileAssetBaseTypeToContentTypeStrategyImpl, DotAssetBaseTypeToContentTypeStrategyImpl, BaseTypeMimeTypeMatcher), a bug-fix refactor of BaseTypeToContentTypeStrategyResolver, and new frontend service methods. No database, Elasticsearch, content model, storage, VTL, or OSGi contracts were altered. Rolling back to the previous release leaves no orphaned data and no broken queries.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94APILocator.getFileAssetAPI().getMimeType(file) may return null or empty string for unknown mime types, and this is passed directly to BaseTypeMimeTypeMatcher.match() without validation. This causes the matcher to treat empty/invalid mime type as a wildcard match, potentially returning an incorrect content type or failing silently. Must validate mimeType before use.
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97-100 — When mimeTypeMatcher.match() returns empty, the code falls back to find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) without checking if the returned ContentType is non-null. If the default FileAsset content type is missing or deleted, this throws a NullPointerException when .get() is called on the optional. Must check isPresent() before .get().
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:124mimeTypeFieldVariables is initialized as new String[]{DotAssetAPI.ALL_MIME_TYPE} but DotAssetAPI.ALL_MIME_TYPE is not imported or defined in this class. This will cause a NoClassDefFoundError at runtime. Must use BaseTypeMimeTypeMatcher.ALL_MIME_TYPE or define constant locally.
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154mimeType is passed to MimeTypeUtils.match() without null check. If mimeType is null (e.g., from file with no extension), this throws NullPointerException. Must guard against null input.
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102Logger.debug(...) logs exception message but does not log exception stack trace. This obscures debugging context. Use Logger.error(this, e, () -> "...") to capture full stack.
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:124DotAssetAPI.ALL_MIME_TYPE used but not defined in this class. Assumption: DotAssetAPI.ALL_MIME_TYPE is */* and exists. What to verify: Confirm DotAssetAPI.ALL_MIME_TYPE is public, static, and equals "*/*" in DotAssetAPI.java.

Existing

  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:124 — Prior finding still present: DotAssetAPI.ALL_MIME_TYPE referenced but not imported or defined in this class.

Resolved

  • core-web/libs/data-access/src/lib/dot-upload-file/dot-upload-file.service.ts:110uploadDotAsset no longer accepts contentType parameter; now uses 'dotAsset' hardcoded — fixed by removing deprecated parameter.
  • core-web/libs/data-access/src/lib/dot-workflow-actions-fire/dot-workflow-actions-fire.service.ts:140newContentletByBaseType now correctly omits contentType from body — fixed by using baseType key instead.
  • core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts:122contentType field replaced with baseType — fixed by consistent type rename.
  • dotCMS/src/main/java/com/dotcms/contenttype/business/DotAssetAPIImpl.java:37 — MimeType matching logic extracted to BaseTypeMimeTypeMatcher — fixed by refactoring to reuse shared logic.

Run: #28183162296 · tokens: in: 18874 · out: 1236 · total: 20110

@zJaaal

zJaaal commented Jun 25, 2026

Copy link
Copy Markdown
Member Author

Thanks for the automated review. I went through each finding against the actual code — the Critical/High items are false positives. Details below.

🔴 Critical — getMimeType null/empty passed to matcher (FileAssetBaseTypeToContentTypeStrategyImpl)
Not a bug. FileAssetAPI.getMimeType(...) returns FileAsset.UNKNOWN_MIME_TYPE (never null) when the type can't be determined. MimeTypeUtils.match(...) explicitly returns false when either side is UNKNOWN_MIME_TYPE, so the specific types (image/video/document) are correctly skipped and resolution falls through to the */* tier → the default FileAsset. That's the intended fallback, not an incorrect match. This is also the exact same path the existing DOTASSET strategy already uses.

🔴 Critical — .get() on optional without isPresent() → NPE (fallback)
Hallucinated — there is no .get() call. The code is Optional.ofNullable(APILocator.getContentTypeAPI(user).find(...)). find(...) returns a ContentType or throws NotFoundInDbException (caught), so there's no NPE.

🟠 High / 🟡 Medium (×2) — DotAssetAPI.ALL_MIME_TYPE not imported → NoClassDefFoundError (BaseTypeMimeTypeMatcher)
Not a bug. DotAssetAPI lives in the same package (com.dotcms.contenttype.business), so no import is required, and ALL_MIME_TYPE / PARTIAL_MIME_TYPE are public static final String interface constants (inlined at compile time). The module compiles cleanly (dotcms-core build + dotcms-integration test-compile both pass).

🟠 High — MimeTypeUtils.match() null mimeType → NPE
Not a bug. match() is null-safe (Try.of(() -> new MimeType(...)).getOrNull() plus null guards), and the input is UNKNOWN_MIME_TYPE, not null.

🟡 Medium — Logger.debug lacks stack trace
Fair nit. It's a deliberate non-fatal swallow (the existing DOTASSET strategy logs nothing here), but I'm happy to include the exception object if preferred.

Net: no code changes needed for the Critical/High findings. Will address the logging nit if reviewers want it.

— Posted by Claude on behalf of @zJaaal

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to mimeTypeMatcher.match() without validation — risks silent failure or incorrect content type resolution if file has no MIME type
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97APILocator.getContentTypeAPI(user).find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on Optional without isPresent() check — risks NPE if default File Asset content type is missing or not found
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:64 — Uses DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE without importing or defining them — will cause NoClassDefFoundError at runtime unless these constants exist in DotAssetAPI (not verified in diff)
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154MimeTypeUtils.match() called on mimeType without null check — risks NPE if file has no extension or MIME type is unset
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102Logger.debug() logs only message, not exception stack trace — obscures debugging context for failures
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:64DotAssetAPI.ALL_MIME_TYPE used but not defined in this class — assumption: it equals "*/*" and is public static; verify its definition in DotAssetAPI.java
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently

Existing

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94 — APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to matcher without validation, risking incorrect content type resolution or silent failure
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on optional without isPresent() check, risking NPE if default content type is missing
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:124 — Uses DotAssetAPI.ALL_MIME_TYPE which is not imported or defined in this class — will cause NoClassDefFoundError at runtime
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154 — MimeTypeUtils.match() called on mimeType without null check — risks NPE if file has no extension or mime type is unset
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102 — Logger.debug logs only message, not exception stack trace — obscures debugging context
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:124 — DotAssetAPI.ALL_MIME_TYPE used but not defined in this class — assumption: it equals "/" and is public static; verify its definition in DotAssetAPI.java

Resolved

  • dotCMS/src/main/java/com/dotcms/contenttype/business/DotAssetAPIImpl.java:37 — Previously duplicated MIME type matching logic; now delegated to BaseTypeMimeTypeMatcher, eliminating redundancy
  • dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeToContentTypeStrategyResolver.java:34 — Now properly registers FILEASSET strategy in addition to DOTASSET, fixing prior omission

Run: #28183917033 · tokens: in: 19632 · out: 1820 · total: 21452

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97APILocator.getContentTypeAPI(user).find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on Optional without checking isPresent(), risking NPE if default content type is missing
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60 — Uses DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE without importing or defining them — will cause NoClassDefFoundError at runtime unless these constants exist in DotAssetAPI
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154MimeTypeUtils.match() called on mimeType without null check — risks NPE if file has no extension or mime type is unset
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to mimeTypeMatcher.match() without validation — risks silent failure or incorrect resolution
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE used but not defined in this class — assumption: they are public static constants in DotAssetAPI; verify their existence and values
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154MimeTypeUtils.match() called on mimeType without null check — risks NPE if getMimeType() returns null or empty
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102Logger.debug logs only message, not exception stack trace — obscures debugging context for failures
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently

Existing

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94 — APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to matcher without validation, risking incorrect content type resolution or silent failure
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on optional without isPresent() check, risking NPE if default content type is missing
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:64 — Uses DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE without importing or defining them — will cause NoClassDefFoundError at runtime unless these constants exist in DotAssetAPI
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154 — MimeTypeUtils.match() called on mimeType without null check — risks NPE if file has no extension or mime type is unset
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102 — Logger.debug logs only message, not exception stack trace — obscures debugging context for failures
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently

Resolved

  • dotCMS/src/main/java/com/dotcms/contenttype/business/DotAssetAPIImpl.java:37 — Previously duplicated logic for mime type matching; now delegated to BaseTypeMimeTypeMatcher — resolved by refactoring
  • dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeToContentTypeStrategyResolver.java:31 — Previously hardcoded DOTASSET only; now includes FILEASSET — resolved by adding builder entry

Run: #28184607562 · tokens: in: 20526 · out: 1557 · total: 22083

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97APILocator.getContentTypeAPI(user).find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on Optional without isPresent(), risking NPE if default content type is missing
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60 — Uses DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE without importing or defining them — will cause NoClassDefFoundError at runtime
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154MimeTypeUtils.match() called on mimeType without null check — risks NPE if file has no extension or mime type is unset
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to matcher without validation, risking incorrect content type resolution or silent failure
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE used but not defined in this class — assumption: they are public static constants in DotAssetAPI; verify their existence and values
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154MimeTypeUtils.match() called on mimeType without null check — risks NPE if getMimeType() returns null or empty
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102Logger.debug logs only message, not exception stack trace — obscures debugging context for failures
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently

Existing

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — APILocator.getContentTypeAPI(user).find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on Optional without isPresent(), risking NPE if default content type is missing
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60 — Uses DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE without importing or defining them — will cause NoClassDefFoundError at runtime
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154 — MimeTypeUtils.match() called on mimeType without null check — risks NPE if file has no extension or mime type is unset
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94 — APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to matcher without validation, risking incorrect content type resolution or silent failure
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60 — DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE used but not defined in this class — assumption: they are public static constants in DotAssetAPI; verify their existence and values
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154 — MimeTypeUtils.match() called on mimeType without null check — risks NPE if getMimeType() returns null or empty
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102 — Logger.debug logs only message, not exception stack trace — obscures debugging context for failures
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently

Resolved

  • dotCMS/src/main/java/com/dotcms/contenttype/business/DotAssetAPIImpl.java:29 — MimeType matching logic refactored to use BaseTypeMimeTypeMatcher; original NPE-prone code removed
  • dotCMS/src/main/java/com/dotcms/contenttype/business/DotAssetBaseTypeToContentTypeStrategyImpl.java — Original implementation replaced; no longer contains prior NPE risks

Run: #28186926998 · tokens: in: 19900 · out: 1654 · total: 21554

Comment thread dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java Outdated
Comment thread dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java Outdated

@fabrizzio-dotCMS fabrizzio-dotCMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks good however I think the inner logic of the matcher could be more semantic making it a lot easrier to read/understand

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97APILocator.getContentTypeAPI(user).find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on Optional without checking .isPresent(), risking NPE if default content type is missing
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60 — Uses DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE without importing or defining them — will cause NoClassDefFoundError at runtime
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154MimeTypeUtils.match() called on mimeType without null check — risks NPE if file has no extension or mime type is unset
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to matcher without validation, risking incorrect content type resolution or silent failure
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE used but not defined in this class — assumption: they are public static constants in DotAssetAPI; verify their existence and values
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154MimeTypeUtils.match() called on mimeType without null check — risks NPE if getMimeType() returns null or empty
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102Logger.debug logs only message, not exception stack trace — obscures debugging context for failures
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently

Existing

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — APILocator.getContentTypeAPI(user).find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on Optional without isPresent(), risking NPE if default content type is missing
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60 — Uses DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE without importing or defining them — will cause NoClassDefFoundError at runtime
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154 — MimeTypeUtils.match() called on mimeType without null check — risks NPE if file has no extension or mime type is unset
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94 — APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to matcher without validation, risking incorrect content type resolution or silent failure
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60 — DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE used but not defined in this class — assumption: they are public static constants in DotAssetAPI; verify their existence and values
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154 — MimeTypeUtils.match() called on mimeType without null check — risks NPE if getMimeType() returns null or empty
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102 — Logger.debug logs only message, not exception stack trace — obscures debugging context for failures
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently

Resolved

  • dotCMS/src/main/java/com/dotcms/contenttype/business/DotAssetAPIImpl.java:29 — Previously duplicated logic for mime type matching; now delegated to BaseTypeMimeTypeMatcher, eliminating redundancy
  • dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeToContentTypeStrategyResolver.java:31 — Now correctly registers FILEASSET strategy alongside DOTASSET; fixed prior regression where only DOTASSET was ever registered

Run: #28193331592 · tokens: in: 19900 · out: 1671 · total: 21571

…avadocs

Address review feedback on the matcher's readability: replace the magic-index
Map[6] array with a MimeTypeBuckets class whose named fields (exact/partial/
wildcard x current-site/system-host) make the resolution precedence explicit.
Behavior is unchanged (same precedence order). Also add javadoc to the matcher's
private methods and to the FileAsset strategy's apply()/getBinary().

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:112APILocator.getContentTypeAPI(user).find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on Optional without isPresent() or orElse() — risks NPE if default FileAsset content type is missing or disabled.
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:100DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE are used without import or definition — will cause NoClassDefFoundError at runtime.
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154MimeTypeUtils.match() called on mimeType without null check — risks NPE if getMimeType(file) returns null or empty.
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to mimeTypeMatcher.match() without validation — risks silent failure or incorrect resolution.
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:100DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE used but not defined in this class — assumption: they are public static constants in DotAssetAPI; verify their existence and values.
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154MimeTypeUtils.match() called on mimeType without null check — risks NPE if getMimeType() returns null or empty.
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:112 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently.
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:100DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE are referenced but not declared — assumption: they are defined in DotAssetAPI; verify they are public static final String constants with correct values.

Existing

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — APILocator.getContentTypeAPI(user).find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on Optional without isPresent(), risking NPE if default content type is missing
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60 — Uses DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE without importing or defining them — will cause NoClassDefFoundError at runtime
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154 — MimeTypeUtils.match() called on mimeType without null check — risks NPE if file has no extension or mime type is unset
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:94 — APILocator.getFileAssetAPI().getMimeType(file) may return null or empty, passed directly to matcher without validation, risking incorrect content type resolution or silent failure
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:60 — DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE used but not defined in this class — assumption: they are public static constants in DotAssetAPI; verify their existence and values
  • 🟠 High: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:154 — MimeTypeUtils.match() called on mimeType without null check — risks NPE if getMimeType() returns null or empty
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:102 — Logger.debug logs only message, not exception stack trace — obscures debugging context for failures
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:97 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently

Resolved

  • dotCMS/src/main/java/com/dotcms/contenttype/business/DotAssetAPIImpl.java:29 — Previously duplicated logic for mime type matching; now delegated to BaseTypeMimeTypeMatcher — resolved by refactoring.
  • dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeToContentTypeStrategyResolver.java:34 — Previously hardcoded only DOTASSET; now correctly registers FILEASSET strategy — resolved by adding builder.put(BaseContentType.FILEASSET, new FileAssetBaseTypeToContentTypeStrategyImpl());.

Run: #28193987355 · tokens: in: 20060 · out: 2269 · total: 22329

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:112APILocator.getContentTypeAPI(user).find(FileAssetAPI.DEFAULT_FILE_ASSET_STRUCTURE_VELOCITY_VAR_NAME) calls .get() on Optional without isPresent(), risking NPE if default content type is missing
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:117mimeTypeFieldVariables = new String [] { DotAssetAPI.ALL_MIME_TYPE }; references DotAssetAPI.ALL_MIME_TYPE which is undefined in this class and not imported — will cause NoClassDefFoundError at runtime
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:185DotAssetAPI.ALL_MIME_TYPE.equals(accept) references undefined DotAssetAPI.ALL_MIME_TYPE — will cause NoClassDefFoundError at runtime
  • 🔴 Critical: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:189accept.endsWith(DotAssetAPI.PARTIAL_MIME_TYPE) references undefined DotAssetAPI.PARTIAL_MIME_TYPE — will cause NoClassDefFoundError at runtime

Existing

  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/FileAssetBaseTypeToContentTypeStrategyImpl.java:112 — Fallback to system default content type assumes it exists and is enabled — no validation that returned ContentType is not null or inactive; may return invalid content type silently
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:117DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE are referenced but not declared — assumption: they are defined in DotAssetAPI; verify they are public static final String constants with correct values
  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/contenttype/business/BaseTypeMimeTypeMatcher.java:185DotAssetAPI.ALL_MIME_TYPE and DotAssetAPI.PARTIAL_MIME_TYPE are referenced but not declared — assumption: they are defined in DotAssetAPI; verify they are public static final String constants with correct values

Resolved

  • dotCMS/src/main/java/com/dotcms/contenttype/business/DotAssetAPIImpl.java:37 — Previously duplicated logic for mime type matching; now delegated to BaseTypeMimeTypeMatcher, resolving prior NPE and undefined constant issues for DOTASSET path
  • dotCMS/src/main/java/com/dotcms/contenttype/business/DotAssetBaseTypeToContentTypeStrategyImpl.java:1 — Replaced duplicated logic with shared BaseTypeMimeTypeMatcher, resolving prior NPE and undefined constant issues for DOTASSET path

Run: #28194903370 · tokens: in: 21053 · out: 1158 · total: 22211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[TASK] Content Drive: Add a FILEASSET base-type → content-type resolution strategy

3 participants