feat: auto-convert fragments referenced by a form during job execution

[im-shiv]

After converting a form, scan its JCR tree for fragRef/fragmentPath references. For each found fragment: copy to /fragments/, convert in place using the same rule pipeline, then update all path references in the converted form. A convertedFragments set skips duplicates within the same bucket. Warnings (path||message) are stored in the bucket resource for any node that still carries an AF1 sling:resourceType after conversion.

[iamsudhanshu]

@im-shiv Could you share a summary of the issue and approach.

Does it cover below scenarios:

• In place migration
• Out of place migration (since customers may want to keep both forms for review and incorporate rule changes)
• Leveraging already migrated fragments into the new form - for out of place migration (without this fragments would not be reused)

[im-shiv]

@im-shiv Could you share a summary of the issue and approach.

Does it cover below scenarios:

• In place migration
• Out of place migration (since customers may want to keep both forms for review and incorporate rule changes)
• Leveraging already migrated fragments into the new form - for out of place migration (without this fragments would not be reused)

In-place migration
When page handling is not COPY, convertFragmentInPlace is called — it runs componentService.apply() directly on the existing fragment page at its current path. The original fragment is converted without being moved or copied.

Out-of-place migration
When page handling is COPY:

1. The form page is copied to targetRoot via RewriteUtils.copyPage
2. Each referenced fragment is copied to /fragments/ (both the AF page and the corresponding DAM asset node)
3. The copy is converted in place using the same rule pipeline
4. updateFragmentPaths then rewrites fragmentPath properties in the converted form to point to /fragments/

Leveraging already-migrated fragments (out-of-place)
Currently handled partially — the convertedFragments set deduplicates within a single bucket run (if two forms reference the same fragment, it's only copied and converted once). However, it does not check whether a fragment already exists at the target from a previous job run. If the job is run again, the fragment would be re-copied and re-converted.

[iamsudhanshu]

@im-shiv Could you share a summary of the issue and approach.
Does it cover below scenarios:

• In place migration
• Out of place migration (since customers may want to keep both forms for review and incorporate rule changes)
• Leveraging already migrated fragments into the new form - for out of place migration (without this fragments would not be reused)

In-place migration When page handling is not COPY, convertFragmentInPlace is called — it runs componentService.apply() directly on the existing fragment page at its current path. The original fragment is converted without being moved or copied.

Out-of-place migration When page handling is COPY:

1. The form page is copied to targetRoot via RewriteUtils.copyPage
2. Each referenced fragment is copied to /fragments/ (both the AF page and the corresponding DAM asset node)
3. The copy is converted in place using the same rule pipeline
4. updateFragmentPaths then rewrites fragmentPath properties in the converted form to point to /fragments/

Leveraging already-migrated fragments (out-of-place) Currently handled partially — the convertedFragments set deduplicates within a single bucket run (if two forms reference the same fragment, it's only copied and converted once). However, it does not check whether a fragment already exists at the target from a previous job run. If the job is run again, the fragment would be re-copied and re-converted.

This needs to be resolved - since someone may update the fragment after first creation - which would result in effort loss. There should be an option to leverage already existing fragments.

[iamsudhanshu]

Review comments from claude:

PR #240 — Code Review

feat: auto-convert fragments referenced by a form during job execution

Overview

The PR extends FormConversionJobExecutor.doProcess to automatically detect, copy, and convert AF1 form fragments that are referenced by a form. Fragments are collected by scanning the JCR tree for fragRef properties, copied into a fragments/
subfolder at the target root (for COPY mode), converted using the same component rules, and tracked to avoid duplicate conversion within a bucket. Warnings about unconverted AF1 resource types are written back to the bucket node.

---

Critical Bugs

1. fragRef is scanned but only fragmentPath is updated — likely a path-update miss

collectFragmentRefs finds nodes with fragRef:
if (node.hasProperty(FRAG_REF_PROPERTY)) // "fragRef"
But updateFragmentPaths rewrites a different property:
if (node.hasProperty(FRAGMENT_PATH_PROPERTY)) // "fragmentPath"
If a component node uses only fragRef to hold its reference (no fragmentPath), the post-copy path rewrite silently does nothing — the converted form still points at the original source fragment. The PR description claims both properties are
scanned; the implementation only scans one and updates the other.

2. Potential NullPointerException on adaptTo(Node.class)

Both call sites chain directly off the result without null checks:
// convertReferencedFragments:
formPage.adaptTo(Node.class).getNode(JcrConstants.JCR_CONTENT)

// doProcess:
page.adaptTo(Node.class).getNode(JcrConstants.JCR_CONTENT)
adaptTo(Node.class) returns null when the resource resolver is session-less or the adapter is unavailable. This will NPE, bypassing the outer catch (RepositoryException) block.

3. Session never saved after normalizeTargetRoot and ensureFolderExists

Both helpers call JcrUtil.createPath(..., false) — the final false disables auto-save. Folder creation is queued in the pending session but never explicitly flushed. The first subsequent JCR read can see it within the same session, but if the
session is flushed or closed before the copy operations run, the folder doesn't exist.

4. getProperty(…).getString() on potentially multi-valued properties

In collectFragmentRefs and updateFragmentPaths:
node.getProperty(FRAG_REF_PROPERTY).getString()
node.getProperty(FRAGMENT_PATH_PROPERTY).getString()
If either property is multi-valued, this throws javax.jcr.ValueFormatException, which is an unchecked RepositoryException — it surfaces as a warning that silently skips the entire form's fragment scan (collectFragmentRefs throws through,
caught in convertReferencedFragments). For updateFragmentPaths the exception propagates to doProcess and is caught, logging a warning rather than failing hard.

5. Flat name extraction in updateFragmentPaths causes collisions

String fragName = oldPath.substring(oldPath.lastIndexOf('/') + 1);
node.setProperty(FRAGMENT_PATH_PROPERTY, fragmentsTargetFolder + "/" + fragName);
If two forms reference fragments with the same leaf name in different source folders (e.g. /content/forms/af/a/frag and /content/forms/af/b/frag), both are copied to the same fragments/frag path. The second copy will fail or overwrite
silently.

---

Design Issues

6. No version created for fragment pages before in-place conversion

For COPY mode, fragments are copied then converted — the source is untouched. But for non-COPY modes (convertFragmentInPlace), RewriteUtils.createVersion is never called on the fragment page. A failed fragment conversion leaves the fragment
in a partially converted state with no rollback point, unlike how forms themselves are treated.

7. Fragment conversion happens before form's own component-rule pass

convertReferencedFragments(...); // ← fragments converted first
componentService.apply(...); // ← form converted second
If the form-level rule pass modifies fragRef/fragmentPath values (e.g. re-mapping them), those new values won't be caught by the already-completed collectFragmentRefs scan. If instead the intent is that fragments must be done first so the
form can reference them, that should be documented.

8. rr.adaptTo(Session.class).save() saves ALL pending changes globally

The single .save() at the end of the COPY+fragment block commits everything pending in the session — including partial changes from earlier failed iterations within the same bucket. This is consistent with how JCR sessions work, but it's
different from what the rest of the executor does (relies on rr.commit() at the boundary), and it can make failure recovery harder to reason about.

9. convertedFragments deduplication key is the raw fragRef value

If one form uses a DAM path (/content/dam/formsanddocuments/…) and another uses the AF path (/content/forms/af/…) to reference the same fragment, the same fragment is converted twice and copied to the same destination — the second copy will
throw RewriteException("Target page already exists").

---

Missing Test Coverage

The PR adds 182 lines of new logic but no new tests. The existing tests (doProcessNoReprocess, doProcessReprocess) have no fixture data with fragRef/fragmentPath properties, so the new paths are entirely untested. At minimum, tests are needed
for:

• Form with one/multiple fragment refs → fragments copied and converted
• Duplicate fragment refs across forms in same bucket → only one conversion
• Fragment page not found → warning, form still succeeds
• Fragment conversion fails → form-level warning, form still succeeds
• In-place fragment conversion (non-COPY mode)
• fragRef and fragmentPath path-update verification post-COPY

---

Minor / Style

• Excessive logging verbosity: Per-form timing logs (startMs, ruleMs) and context.log() for each sub-step match debug output, not production job logs. The existing convention is a single log per success/failure.
• 10-parameter method signature: convertReferencedFragments takes 10 arguments. Consider a small inner-class or record to group the shared mutable state (convertedFragments, bucketWarnings).
• || delimiter in warnings is fragile: path + "||" + message — if either part contains ||, UI parsing breaks. Use a structured type or at least a clearly illegal separator.
• fragIdx pre-incremented: Counter is incremented before the continue check, so skipped (already-converted) fragments are included in the fragIdx/fragRefs.size() display. Intentional or off-by-one presentation issue?
• Typo carried from original, then fixed: "skipping skipping" → good fix, but the log message for the empty-rules path is now inconsistent in style from the rest of the file's conventions.
• HashSet vs LinkedHashSet inconsistency: convertedFragments is HashSet (unordered); collectFragmentRefs returns LinkedHashSet (ordered). Since insertion order matters for user-visible log sequence, convertedFragments should also be
  LinkedHashSet or the choice should be documented.

[bstopp]

See PR review from Claude provided by @iamsudhanshu