Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,39 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)

---

## [1.0.0] — Unreleased
## [1.0.1-RC]

A performance and hardening release on top of the 1.0.0 rebrand: Reimport and Clone now run in parallel through JMS queues, mixed-operation batches route correctly, the listing query is parameterized, and workflow history is consistent across all three operations. The `/xapi/transfer` request format and the Java packages are unchanged from 1.0.0.

### Performance

- **Parallel processing via JMS queues** — Reimport and Clone no longer run in a single sequential loop. Reimport is dispatched per image session and Clone per subject onto in-process (`vm://`) JMS queues; an in-memory `BatchTransferMonitor` fans the results back in and emits a single terminal *Transfer Complete* / *Warning* event once every item — across every operation in the batch — has finished.
- **Admin-tunable consumer concurrency** — new site-admin settings (and `GET` / `POST /xapi/batch_transfer/jms_queues`) set each queue's `min–max` consumer count. Defaults: **Reimport 4–8**, **Clone 1–2**; set `min = max = 1` for a serial kill-switch.
- **Faster Clone file copy** — the archive directory copy no longer opens every file to check whether it is a catalog; only catalog/XML files are read and the rest are hard-linked directly, sharply cutting per-file I/O on large multi-scan sessions.

> **Concurrency & storage note** — Reimport throughput is bound by prearchive **storage** and the anonymization pipeline, not CPU or plugin locking. On slow or bind-mounted storage, raising consumer concurrency is counter-productive (concurrent writers contend, and one slot can out-throughput many); tune concurrency up only on fast local/SAN prearchive storage. See `docs/ROADMAP.md`.

### Changed

- **Operation-aware batch routing** — a batch containing a mix of Share / Clone / Reimport items is now split by operation and each group routed to its correct path (Reimport queue, Clone queue, or the in-process sequential path for Share). Previously every item in a batch was assumed to share a single operation.
- **Unified workflow history** — the completed-action workflow recorded on the **source** item now uses one consistent past-tense label for all three operations: *"Cloned / Shared / Reimported `<item>` to project `<dest>`"*.

### Removed

- **Redundant "Files Cloned" workflow** — each cloned item previously produced a second, destination-side *"Files Cloned"* workflow on top of the source-side *"Cloned …"* workflow (four workflow DB writes per item). The destination-side record is gone (two writes now); clone provenance is still captured by the source workflow plus the preserved `original-project` field, and rollback-on-failure is unchanged.

### Fixed

- **Reimport expand arrows** — a session whose only expandable children are out-of-scope image assessors no longer shows a dead expand arrow in Reimport mode. Share / Clone are unaffected.
- **Stale anonymization status** — the destination project's anonymization status is re-queried on each selection change instead of being read once at page load, so the *About this transfer* banner always reflects the project actually selected.

### Security

- **Parameterized listing query** — the subject / experiment listing SQL behind the transfer screen is now fully parameterized (`:userId`, `:project`, `:ids`) via `NamedParameterJdbcTemplate`; request-derived values are no longer concatenated into the SQL string, closing a SQL-injection vector in the project / id filters.

---

## [1.0.0]

### Renamed — Batch Share → Batch Transfer

Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ plugins {
}

group "org.nrg.xnatx.plugins"
version "1.0.0"
version "1.0.1-RC"
description "XNAT Batch Transfer Plugin"

sourceCompatibility = 1.8
Expand Down
16 changes: 12 additions & 4 deletions docs/ROADMAP.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
# Batch Transfer Plugin — Roadmap

**Plugin Version**: 1.0.0-SNAPSHOT (formerly Batch Share Plugin 2.0.0-SNAPSHOT; renamed and versioned reset at 1.0.0)
**Plugin Version**: 1.0.1-RC (rebranded from Batch Share Plugin 2.0.0-SNAPSHOT; version line reset at 1.0.0)
**Target XNAT**: 1.9.3.3
**Last Updated**: 2026-05-12
**Last Updated**: 2026-06-22

The Batch Transfer Plugin enables bulk data operations across XNAT projects. Users can Share, Clone, or Reimport subjects, sessions, and assessors in batch from a single interface. **Share** adds data into a destination project without copying (XNAT's standard sharing relationship); **Clone** duplicates data into the destination, producing an independent editable copy; **Reimport** re-ingests image sessions through the destination project's anonymization pipeline.

---

## Completed

### 1.0.1-RC — Performance & hardening
- **Parallel processing via JMS queues** — Reimport (per session) and Clone (per subject) dispatched onto in-process (`vm://`) JMS queues instead of a single sequential loop; in-memory `BatchTransferMonitor` fan-in emits one terminal event per batch
- **Admin-tunable queue concurrency** — site-admin settings + `GET`/`POST /xapi/batch_transfer/jms_queues`; defaults Reimport 4–8, Clone 1–2 (`min = max = 1` for serial)
- **Operation-aware batch routing** — mixed Share/Clone/Reimport batches split by operation and routed to the correct queue or the sequential path
- **Faster Clone copy** — archive copy reads only catalog/XML files and hard-links the rest (no longer opens every file)
- **Unified workflow history** — consistent past-tense source-item labels ("Cloned/Shared/Reimported `<item>` to project `<dest>`"); removed the redundant destination-side "Files Cloned" workflow (4 → 2 workflow writes per cloned item)
- **Parameterized listing query** — subject/experiment listing SQL fully parameterized (`:userId`, `:project`, `:ids`); closes a SQL-injection vector

### UI Modernization
- Two-panel layout replacing legacy jsTree-based form (sidebar + data table)
- Hierarchical data table with expand/collapse, search filtering, and bulk select
Expand Down Expand Up @@ -39,17 +47,17 @@ The Batch Transfer Plugin enables bulk data operations across XNAT projects. Use
## Planned

### Operations & Workflow
- Expand Reimport type support to include scans-based operations
- Implement UX cohort building filters for large (>10k+) datasets where the select/deselect item workflow would be tedious
- Launch Advanced Search from destination project Action Menu
- Include all XNAT experiment data types
- Computed size estimate in the operation-detail panel — replace the static duplication warning with a real byte count for the current selection (e.g. "≈ 4.2 GB will be duplicated to *DestProject*"). Requires either a new XAPI endpoint that walks the archive, or per-row `data-size` attributes emitted by `XDATScreen_batch_transfer.java`, plus rollup logic in `batchTransfer.js`.

### Backend & API
- BatchTransferEvent refactor to limit event/DB traffic
- BatchTransferEvent refactor to limit event/DB traffic (partially addressed in 1.0.1-RC: in-memory `BatchTransferMonitor` fan-in collapses per-batch terminal events to one)
- Rethink BatchTransferEvent status to reflect import process as reflected in prearch import.
- "Transfer Queued (%s items)" and "Transfer Complete" do not yet reflect true Share/Clone/Reimport status nuance
- REST API extension to support directional sharing workflow (transfer-to vs. transfer-from) once the inbound flow is built
- Reimport task distribution via external MQ to allow scaling across multiple XNAT nodes
- Extend PROJECT_SHARING_FEATURE and PROJECT_COPYING_FEATURE controls to include a separate PROJECT_IMPORTING_FEATURE (feature-flag naming intentionally deferred from the rebrand)
- Per-experiment Reimport timeout to bound importer hangs. `BatchTransferServiceImpl.runImporter` invokes `DicomZipImporter.call()` synchronously, with no upper bound — a stuck prearchive write (wedged NFS mount, deadlocked rebuild queue, slow remote disk) parks the batch-loop thread indefinitely and blocks all subsequent requests in the batch. The 5-minute heartbeat logger makes a hang observable, but recovery still requires a Tomcat restart. Possible fix: wrap `importExperiment` in `Future.get(timeoutMs)` on a separate executor; on timeout, cancel the future, emit a Failed event, and let the batch loop continue with the next request. Caveats: `cancel(true)` cannot reliably stop uninterruptible I/O so the underlying thread leaks; the prearchive has no transactional rollback so a cancelled import may leave partial state; choosing a default timeout is environment-specific (large legitimate sessions can run 30+ min). Defer until a customer incident motivates it; if added, expose the timeout as a plugin config and document explicitly that it means "we gave up waiting", not "we cleaned up".

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.nrg.xft.XFTTable;
import org.nrg.xft.security.UserI;
import org.restlet.data.Status;
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;

import java.util.*;
import java.util.stream.Collectors;
Expand All @@ -40,7 +41,7 @@ public class XDATScreen_batch_transfer extends SecureScreen {
"LEFT JOIN xdat_user u ON map.groups_groupid_xdat_user_xdat_user_id=u.xdat_user_id\n";

private static final String READABLE_EXPERIMENTS = "FROM xdat_user_groupid gid\n" +
"LEFT JOIN xdat_usergroup grp ON gid.groupid=grp.id AND gid.groups_groupid_xdat_user_xdat_user_id={USER_ID}\n" +
"LEFT JOIN xdat_usergroup grp ON gid.groupid=grp.id AND gid.groups_groupid_xdat_user_xdat_user_id=:userId\n" +
"LEFT JOIN xdat_element_access xea ON grp.xdat_usergroup_id=xea.xdat_usergroup_xdat_usergroup_id AND xea.element_name NOT IN ('xnat:projectData','xnat:subjectData')\n" +
"LEFT JOIN xdat_field_mapping_set fms ON xea.xdat_element_access_id=fms.permissions_allow_set_xdat_elem_xdat_element_access_id\n" +
"LEFT JOIN xdat_field_mapping xfm ON fms.xdat_field_mapping_set_id=xfm.xdat_field_mapping_set_xdat_field_mapping_set_id AND xfm.read_element=1\n" +
Expand Down Expand Up @@ -111,11 +112,27 @@ protected void doBuildTemplate(RunData data, Context context) throws Exception {
itemIds = table.convertColumnToArrayList(FIELD_ID);
}

boolean byProject = (sourceProjectId != null);
String safeProj = byProject ? sourceProjectId.replaceAll("[^A-Za-z0-9_\\-]", "") : "";
String ownedScope = byProject ? "WHERE subj.project='" + safeProj + "'\n" : "";
String sharedScope = byProject ? "WHERE pp.project='" + safeProj + "'\n" : "";
String finalFilter = byProject ? "" : ("WHERE " + column + " IN ('" + buildWhereClause(itemIds) + "')");
boolean byProject = (sourceProjectId != null);

// Parameterized query: values are bound, not concatenated. ":userId" is reused throughout;
// ":project" / ":ids" are bound per entry point. "column" in finalFilter is a SQL identifier
// (subj.id / SADS.id / IAD.id, chosen above from xsiType) and cannot be bound, so it stays
// concatenated — safe because it is code-controlled.
final MapSqlParameterSource params =
new MapSqlParameterSource("userId", ((XDATUser) user).getXdatUserId());
String ownedScope = "", sharedScope = "", finalFilter = "";
if (byProject) {
params.addValue("project", sourceProjectId);
ownedScope = "WHERE subj.project = :project\n";
sharedScope = "WHERE pp.project = :project\n";
} else {
if (itemIds == null || itemIds.isEmpty()) {
context.put("sharingMsg", "None of the requested data is available.");
return;
}
params.addValue("ids", itemIds);
finalFilter = "WHERE " + column + " IN (:ids)";
}

// Build the query to retrieve information about the items we want listed.
String query = "SELECT DISTINCT ON (SUBJ.ID, SADS.ID, IAD.ID) secondary_id AS project_label,subj.id AS subject_id, subj.label AS subject_label, subj.project AS subject_project, SADS.id AS session_id, SADS.label AS session_label, SADS.project AS session_project, SADS.date AS session_expt_date, SADS.element_name as session_element, IAD.id AS assessor_id, IAD.label AS assessor_label, IAD.project AS assessor_project, IAD.date AS assess_expt_date, IAD.element_name AS assessor_element\n" +
Expand All @@ -124,15 +141,15 @@ protected void doBuildTemplate(RunData data, Context context) throws Exception {
"FROM xnat_subjectData subj\n" +
"JOIN (\n" +
READABLE_SUBJECTS +
"WHERE xdat_user_id={USER_ID} AND read_element=1 AND element_name='xnat:subjectData' AND field='xnat:subjectData/project') OWNED ON subj.project=OWNED.tag\n" +
"WHERE xdat_user_id=:userId AND read_element=1 AND element_name='xnat:subjectData' AND field='xnat:subjectData/project') OWNED ON subj.project=OWNED.tag\n" +
ownedScope +
"UNION\n" +
"SELECT subj.id, pp.label, subj.project\n" +
"FROM xnat_subjectData subj\n" +
" JOIN xnat_projectParticipant pp ON subj.id=pp.subject_id\n" +
"JOIN (\n" +
READABLE_SUBJECTS +
"WHERE xdat_user_id={USER_ID} AND read_element=1 AND element_name='xnat:subjectData' AND field='xnat:subjectData/sharing/share/project' ) SHARED ON pp.project=SHARED.tag\n" +
"WHERE xdat_user_id=:userId AND read_element=1 AND element_name='xnat:subjectData' AND field='xnat:subjectData/sharing/share/project' ) SHARED ON pp.project=SHARED.tag\n" +
sharedScope +
") SUBJ\n" +
"LEFT JOIN (\n" +
Expand Down Expand Up @@ -169,14 +186,12 @@ protected void doBuildTemplate(RunData data, Context context) throws Exception {
finalFilter +
";";

query = query.replace("{USER_ID}", ((XDATUser) user).getXdatUserId().toString());

// Execute the query to get a list of experiments
XFTTable experiments = XFTTable.Execute(query, user.getDBName(), user.getUsername());
// Execute the parameterized query to get a list of experiments.
List<Map<String, Object>> rows = XDAT.getNamedParameterJdbcTemplate().queryForList(query, params);

// Build ONE item set (all readable items), then compute per-project eligibility once.
// Share/Clone are feature-gated; Reimport is always allowed, so the set is unfiltered.
Map<String, ItemContainer> items = buildItems(experiments, search);
Map<String, ItemContainer> items = buildItems(rows);
if (items.isEmpty()) {
context.put("sharingMsg", "None of the requested data is available.");
return;
Expand Down Expand Up @@ -231,13 +246,13 @@ private void addShareIdDisplayField(SchemaElement rootElement, DisplaySearch sea
* @param search - DisplaySearch
* @return Hashtable<String, ItemContainer>
*/
private Map<String, ItemContainer> buildItems(XFTTable t, DisplaySearch search) {
private Map<String, ItemContainer> buildItems(List<Map<String, Object>> rows) {
Map<String, ItemContainer> items = new HashMap<>();

// For each experiment row, build the project→subject→session→assessor tree. The set is
// unfiltered — per-project Share/Clone eligibility is computed once in doBuildTemplate,
// and Reimport is always allowed.
for(Hashtable exp : t.rowHashs()){
for (Map<String, Object> exp : rows) {

String project = (String) exp.get("subject_project");
String subject_id = (String) exp.get("subject_id");
Expand Down Expand Up @@ -287,15 +302,6 @@ private Map<String, ItemContainer> buildItems(XFTTable t, DisplaySearch search)
return sortItemContainersByLabel(items);
}

/**
* Function builds a comma delimited string of id's to be
* inserted in the where clause of a sql query.
* @param ids - a list of ids
* @return a comma delimited string of id's
*/
private String buildWhereClause(List<String> ids){
return StringUtils.join(ids, "','");
}

private static Map<String, ItemContainer> sortItemContainersByLabel(Map<String, ItemContainer> items) {
items.forEach((key, value) -> value.sortChildren());
Expand Down
Loading
Loading