dotCMS / dotCMS/core

Add pagination, sorting, filtering and permission checks to the Experiments list endpoint

Open
#36,823 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

dotCMS : Experiments Team : Falcon Type : Task
Dominant language
Java
Stars
970
Forks
486
Avg merge
3d 33m
Merged PRs (30d)
170

Description

Description

The new standalone Experiments portlet needs a site-wide, paginated experiments list. This issue puts the existing list endpoint onto dotCMS's canonical pagination stack, closes a security hole in it, and fixes two latent SQL bugs that the portlet's own UI hits immediately.

What already works

GET /api/v1/experiments already returns every experiment on every page and every sitepageId, name and status are all optional query params, and ExperimentsFactoryImpl.java:37 starts from SELECT * FROM experiment WHERE 1=1. A Postman test asserts exactly this (listExperimentsNoFilter_shouldIncludeAll, Experiments_Resource.postman_collection.json:6312).

So there is no new read path or persistence design to invent. What is missing is everything the portlet's toolbar and table need on top of it.

What is missing
# Gap Evidence
1 Pagination — no page/per_page, no LIMIT/OFFSET in the SQL ExperimentsFactoryImpl.java:94-131, AbstractExperimentFilter.java:20-24
2 Sorting — no orderby/direction, no ORDER BY. Result order is whatever Postgres returns, i.e. non-deterministic across calls same
3 Total count — no count() on ExperimentsAPI/ExperimentsFactory. The response's pagination field is always null because the views use the single-arg ResponseEntityView(entity) ctor ResponseEntityView.java:66-73, ResponseEntityExperimentView:8-10
4 Site scoping — no host/site column on the experiment table and no siteId on the filter postgres.sql:2531-2547
5 Permission filtering — see the security section below ExperimentsAPIImpl.java:504-510
6 No ExperimentsPaginatorcom/dotcms/util/pagination/ has 23 paginators, none experiment-related
🔴 Security: the list endpoint does no permission filtering

ExperimentsAPIImpl.list(ExperimentFilter, User) accepts a User and never uses it:

// ExperimentsAPIImpl.java:504-510
public List<Experiment> list(ExperimentFilter filter, User user) throws DotDataException {
    DotPreconditions.isTrue(hasValidLicense(), InvalidLicenseException.class, INVALID_LICENSE_MESSAGE_SUPPLIER);
    return factory.list(filter);
}

No permissionAPI call, no filtering. By contrast find(id, user) (:349-369) does call validatePageEditPermissions, which builds a PermissionableProxy of type "htmlpage" for the parent page and checks PermissionLevel.EDIT. The interface javadoc (ExperimentsAPI.java:66-74) promises no filtering either, so this is a genuine hole, not a doc mismatch.

The license precondition is not a mitigation: LicenseUtil.getLevel() is hardcoded return LicenseLevel.PLATFORM.level (LicenseUtil.java:179-181), so hasValidLicense() is always true.

Why this has not been exploited: no production caller ever omits pageId. dot-experiments.service.ts:64 hard-appends ?pageId=${pageId} and :79 appends ?pageId=${pageId}&status=${status}. The un-scoped list is exercised only by Postman. A site-wide portlet makes it reachable by any authenticated back-end user on day one — so this must be fixed in the same change that makes the endpoint usable site-wide.

🔴 Two SQL bugs, both untested, both hit by the portlet on day one

The portlet's primary interaction is a search box plus a multi-select status filter. That combination is broken today.

Bug A — name + status together emit malformed SQL. NAME_FILTER = "AND name LIKE ?" (ExperimentsFactoryImpl.java:41) is missing the trailing space that PAGE_ID_FILTER (:39) has. Line 106 appends "INTERSECT ", producing:

SELECT * FROM experiment WHERE 1=1 AND name LIKE ?INTERSECT SELECT * from experiment WHERE status = ?

pgjdbc rewrites ?$n with no separator, yielding $2INTERSECT, which PostgreSQL 15+ rejects.

Bug B — pageId + 2 or more statuses returns wrong rows. Lines 105-111 build base INTERSECT s1 UNION DISTINCT s2 …. INTERSECT binds tighter than UNION, so it evaluates as (base ∩ s1) ∪ s2every status after the first silently escapes the pageId/name scoping and returns experiments from every page and every site.

Neither has any coverage: Postman only ever sends one filter at a time (listExperimentsFilterBySingleStatus_shouldSucceed), and ExperimentAPIImpIntegrationTest never constructs an ExperimentFilter at all. Both are fixed by the same query rewrite this issue needs anyway.


Scope

Follow the canonical stack: PaginatorOrdered<T> + PaginationUtil.getPageView(...), modelled on /api/v2/tags (com.dotcms.rest.api.v2.tags.TagResource), which uses the current getPageView path.

⚠️ Do not model on LanguagesResource — it has no paginated endpoint at all (:80-89 returns a whole Map). Do not use the @Deprecated getPage overloads.

1. Extend AbstractExperimentFilter

Currently pageId(), name(), statuses() (:20-24). Add:

Optional<String> siteId();
Optional<String> filter();      // free text: name OR page path
int offset();                   // default 0
int limit();                    // default 40
Optional<String> orderBy();
Optional<Boolean> ascending();

Immutables generates defaults, so existing callers keep compiling: ExperimentsAPIImpl:692, 734, 763, 1317, 1529 and ExportStarterUtil:464.

2. Query-param contract
Param Behavior
pageId exact match on experiment.page_id. Preserved unchanged.
filter canonical free-text search, case-insensitive partial match on experiment name OR page path. This mirrors the portlet's search box, which matches either.
name legacy alias, name-only, case-insensitive. Kept so existing Postman requests and any external integration keep working. If both filter and name are sent, filter wins. Document name as deprecated in its @Parameter.
status repeatable enum, unchanged: RUNNING|SCHEDULED|ENDED|DRAFT|ARCHIVED
goal new (scope expansion, 2026-08-26). Repeatable goal-type enum, same semantics as status. Filters to experiments whose goal is of the given type(s).
siteId new. Optional — when omitted the listing spans all sites, preserving current behaviour for internal callers.
page / per_page standard PaginationUtil params
orderby / direction standard. Accept both orderby (lowercase, PaginationUtil.ORDER_BY, what the shared PaginatorService sends) and orderBy (camelCase, what v2 TagResource declares) — JAX-RS query params are case-sensitive and the two reference resources disagree. orderby is canonical.
3. Rewrite ExperimentsFactoryImpl.list() (:94-131)
  • Build a single parameterised WHERE chain. Delete the INTERSECT/UNION construction — replace STATUS_FILTER (:43) with AND status IN (?,?,?), and fix the missing trailing space on NAME_FILTER (:41). This kills Bug A and Bug B in one pass.

  • Switch the text match to case-insensitive (ILIKE, not LIKE).

  • Join identifier for both site scoping and the page-path search — no schema change needed:

    SELECT e.* FROM experiment e
      JOIN identifier i ON i.id = e.page_id
     WHERE 1=1
       [AND i.host_inode = ?]
       [AND (lower(e.name) LIKE ? OR lower(i.parent_path || i.asset_name) LIKE ?)]
       [AND e.page_id = ?]
       [AND e.status IN (?,?,?)]
    

    identifier(id, parent_path, asset_name, host_inode) is at postgres.sql:1020-1034. Prefer this over a denormalised site column, which would need an upgrade task plus a backfill and would drift when pages move between sites.

  • ORDER BY <whitelisted column> <ASC|DESC>, e.id ASC. Default: mod_date DESC. The id tiebreaker is what makes paging stable.

  • Whitelist orderBy against an enum of real sort keys — never interpolate a raw client string. (Scope expansion, 2026-08-26) the whitelist must cover every column the portlet's table sorts by: name, status, creation_date, mod_date, plus page (page path, via the identifier join this issue already introduces), goal (goal type) and schedule (scheduling start date). If the layer calls SQLUtil.sanitizeSortBy (:305-338), the column must also be in SQLUtil.ORDERBY_WHITELIST (:128-135) or the sort is silently dropped.

  • LIMIT ? OFFSET ? in the SQL. Do not use DotConnect.setStartRow/setMaxRows — they walk the ResultSet in Java (DotConnect.java:791-797), so the DB would still evaluate every row.

4. Add count(ExperimentFilter)

New method on ExperimentsFactory (:15-51) and ExperimentsAPI, sharing the same WHERE builder as list() so the two can never disagree.

5. Close the permission hole

Filter by the caller's permission on each experiment's parent page, consistent with validateExperimentPagePermissions (ExperimentsAPIImpl:1601-1614). Experiment already implements Permissionable and getParentPermissionable() resolves to the page contentlet (AbstractExperiment.java:144-150).

Prefer filtering in SQL — restrict page_id to pages the user can access by joining the permission tables in the same query. This is the only approach where the count and the returned page agree and page sizes stay uniform.

If filtering in Java instead, use the batch form PermissionAPI.filterCollection(List<P>, int, boolean, User) (PermissionAPI.java:834) — the repo's CLAUDE.md prescribes it over per-item doesUserHavePermission loops. ⚠️ Filtering after LIMIT/OFFSET makes page sizes ragged and the total wrong, so this needs fetch-filter-then-page (acceptable only while experiment volume stays small) or an approximate count. Both are worse than the SQL approach.

Use EDIT, matching find() — so every listed experiment is actually openable.

Add an explicit systemUser bypass rather than relying on callers. These must not start being filtered, or scheduled starts and starter exports silently drop rows:

  • ExperimentsAPIImpl:1317 cacheRunningExperiments
  • ExperimentsAPIImpl:1529 startScheduledToStartExperiments
  • ExportStarterUtil:464
6. Write ExperimentsPaginator

In dotCMS/src/main/java/com/dotcms/util/pagination/, implementing PaginatorOrdered<T> (PaginatorOrdered.java:73).

  • Must call result.setTotalResults(total) with the un-paged, permission-filtered total. Skipping it makes X-Pagination-Total-Entries and pagination.totalEntries come back 0 and the UI's pager dead.
  • limit = per_page; offset is already the computed 0-based offset (PaginationUtil.java:79-82) — do not re-derive it from page.
  • Declare a constant per extra param (SITE_ID_PARAM, STATUS_PARAM) and name the @QueryParam identically — the extraParam key also appears in the generated Link header URLs (PaginationUtil.java:390-413).
  • Wrap checked DotDataException/DotSecurityException in DotRuntimeException or PaginationException.
7. Wire it in the resource

new PaginationUtil(new ExperimentsPaginator())getPageView(PaginationUtilParams) returning ResponseEntityPaginatedDataView. Nothing to register anywhere: Jersey scans com.dotcms.rest (DotRestApplication.java:141-158) and paginators are plain new.

Gate with .requiredPortlet(PortletID.EXPERIMENTS.toString()) once the portlet id exists.


Acceptance Criteria

Pagination

  • GET /api/v1/experiments?page=2&per_page=20 returns page 2 and a body pagination: { currentPage: 2, perPage: 20, totalEntries: N }, where N is the total matching the filter — not the page size.
  • The five standard headers are emitted: Link, X-Pagination-Per-Page, X-Pagination-Current-Page, X-Pagination-Link-Pages, X-Pagination-Total-Entries.
  • page beyond the last page returns an empty entity with the correct totalEntries, not an error.
  • per_page=0 falls back to the configured default rather than returning zero rows.
  • Paginating through the whole result set returns every row exactly once — no duplicates, no omissions.

Sorting

  • Default order is mod_date DESC with an id ASC tiebreaker.
  • ?orderby=name&direction=DESC orders by name descending; the same holds for every whitelisted column.
  • Ordering is stable across pages — verified by fetching all pages and asserting no duplicates and no omissions.
  • An orderby value outside the whitelist is rejected or falls back to the default — never interpolated into SQL.
  • Both orderby and orderBy are accepted.

Filtering

  • ?filter= does a case-insensitive partial match on the experiment name.
  • ?filter= also matches the page path — e.g. ?filter=/features returns experiments whose page lives under /features, regardless of experiment name.
  • ?pageId=<identifier> still filters to exactly that page.
  • ?name= still works as a name-only alias; when both filter and name are sent, filter wins.
  • ?siteId=<host-inode> returns only experiments whose page lives on that site.
  • GET /api/v1/experiments with no params still returns all experiments across all pages and all sites.

SQL bug regressions

  • ?filter=x&status=DRAFT executes without a SQL error (Bug A).
  • ?pageId=X&status=DRAFT&status=RUNNING returns only experiments on page X — both statuses respect the pageId (Bug B).
  • The same holds for ?siteId=…&status=A&status=B and ?filter=…&status=A&status=B.
  • Both bugs have explicit regression tests.

Count

  • count(filter) equals the number of rows obtained by paginating list(filter), for every filter combination.
  • count() and list() share the same WHERE builder.

Permissions

  • A non-admin user with EDIT on page A and no permission on page B sees only page A's experiments when calling GET /api/v1/experiments with no filters.
  • pagination.totalEntries reflects the permission-filtered total, not the raw row count.
  • Page sizes are uniform — requesting per_page=20 returns 20 rows whenever 20 permitted rows remain.
  • A CMS Administrator sees everything (unchanged).
  • systemUser calls are not filtered — cacheRunningExperiments, startScheduledToStartExperiments and ExportStarterUtil return the same rows as before.
  • Permission resolution adds no per-row query — constant query count for a page of 20 rows, not O(n).
  • The ExperimentsAPI.list javadoc documents the filtering behaviour.

Backwards compatibility

  • Existing internal callers (ExperimentsAPIImpl:692, 734, 763, 1317, 1529, ExportStarterUtil:464) compile unchanged and behave identically.
  • The existing per-Page portlet (dot-experiments.service.ts getAll(pageId) / getByStatus(pageId, status)) keeps working with no frontend change.
  • dot-experiments.service.spec.ts (which asserts the ?pageId= URL shape at :43,48) still passes.

Tests

  • ExperimentsPaginatorTest asserts both items and totalResults (pattern: CategoriesPaginatorTest).
  • Integration coverage for the full filter matrix: pageId × filter × single status × multiple statuses × siteId. ExperimentAPIImpIntegrationTest has zero ExperimentFilter coverage today.
  • Integration coverage for permission filtering: limited user, administrator, systemUser, and its interaction with pagination.
  • listExperimentsNoFilter_shouldIncludeAll (Postman, collection line 6312) is updated deliberately in this PR — pagination and permission filtering change its behaviour. Do not let it be fixed reactively when CI goes red.
  • Postman requests added for pagination, ordering, combined filters and siteId, asserting the pagination block and the X-Pagination-* headers.

Priority

High


Additional Context

Performance

The experiment table has exactly one index — idx_exp_pageid on page_id (postgres.sql:2549). Nothing on status, name, creation_date or mod_date, so a site-wide listing sorted by mod_date will table-scan.

Worse: idx_exp_pageid exists only in the fresh-install postgres.sql — no upgrade task ever created it (verified across Task220829CreateExperimentsTable, Task220928AddLookbackWindowColumnToExperiment and Task230630CreateRunningIdsExperimentField), so upgraded instances may have zero secondary indexes on this table.

The indexes are a separate issue and should land before this reaches production. Record the EXPLAIN plan for the portlet's list query in this PR either way.

Out of scope

Tracked separately as part of the Experiments portlet effort:

  • Row enrichmentmoved into scope, see Scope Expansion (2026-08-26) below.
  • Count by statusmoved into scope, see Scope Expansion (2026-08-26) below.
  • DB indexes — see Performance above.
  • OpenAPI annotations — the resource has zero @Operation/@ApiResponse today, so all 18 operations are bare stubs with generic operationIds (list, create_2, delete_6), and the resource declares @Tag(name = "Experiment") while DotRestApplication:71 declares Experiments. Add @Operation/@Parameter/@ApiResponses for the params introduced here and regenerate openapi.yaml; the full sweep is separate.
  • targetingConditions on list rows — will always be null. They live in Rules and are hydrated only by find() (ExperimentTransformer.transform never sets them). Not needed by the portlet.
  • Results/metrics on list rows — deliberately not added. GET /{id}/results costs 2 CubeJS round-trips plus a 1000-sample Monte-Carlo per experiment, is @NoCache, and has no batch form. The portlet's list shows no metrics by design.
Notes
  • No feature flag and no license gate. There is no flag mechanism in the portlet XML schema, and LicenseUtil.getLevel() is hardcoded to PLATFORM so hasValidLicense() is always true.
  • dotcms.paginator.rows and dotcms.paginator.links are not present in dotmarketing-config.properties, so the effective server defaults are the hardcoded fallbacks 10 and 5. The Angular PaginatorService independently defaults to 40, so the portlet will send per_page explicitly.

Scope Expansion (2026-08-26)

Decided while writing the Spec-Kit spec for #37007 (the consumer of this endpoint). #37007 lands as a single complete drop, so everything its UI already ships must be servable by this endpoint in the same release. Three additions:

A. Sort whitelist covers every portlet column

The portlet's URL contract already ships orderby=name|page|goal|schedule|status|modDate. The whitelist in Scope item 3 grows accordingly: page sorts by the page path (identifier.parent_path || identifier.asset_name, from the join this issue already introduces), goal by goal type, schedule by the scheduled start date. Same rules as before: enum-mapped sort keys, never raw client strings, id ASC tiebreaker.

B. New goal filter parameter + per-goal counts

The portlet ships a goal filter with a goal URL param and per-goal counts. Add:

  • goal query param — repeatable goal-type enum, composing with every other filter exactly like status does (same WHERE builder, same test matrix treatment).
  • Per-goal counts over the whole filtered set, delivered alongside the per-status counts (see C).
C. Row enrichment + per-status counts (moved from Out of scope)

Both were listed as "tracked separately"; no follow-up issue was ever filed, and #37007 needs them to delete its interim client-side lookups. They land here:

  • Row enrichment: each list row carries pageTitle, pagePath, siteId, siteName, resolved via the same identifier join used for site scoping and path search. Today the row carries only pageId (AbstractExperiment.java:87-88).
  • Counts: per-status counts (and per-goal counts, see B) computed over the whole permission-filtered, filter-matching set — not the current page — so the portlet's status chips and goal dropdown reflect the full result set.
Added Acceptance Criteria
  • ?orderby=page|goal|schedule each order correctly (with direction), stable across pages; still whitelisted, never interpolated.
  • ?goal=<type> filters by goal type; repeatable; composes with status, filter, pageId, siteId without SQL errors or scoping leaks (same regression shape as Bugs A/B).
  • Every list row carries pageTitle, pagePath, siteId, siteName matching the page the experiment belongs to.
  • The response exposes per-status and per-goal counts computed over the whole filtered, permission-filtered set; they agree with pagination.totalEntries when summed per dimension.
  • Enrichment and counts add no per-row queries — constant query count for a page of rows.
  • ?created_by=<userId> returns only that creator's experiments; composes with filter, status, goal, pageId, siteId without SQL errors or scoping leaks.
  • ?running_from=…&running_to=… returns experiments whose running window overlaps the range, bounds inclusive; an experiment with several runs matches if any run overlaps; a never-run scheduled experiment matches on its scheduled window.
  • Counts and totals agree with the creator and date filters, same as for every other filter.
D. Creator and running-window filters

Two stakeholder-requested filters the portlet ships as toolbar controls, so the endpoint must serve them:

  • created_by query param — exact match on the experiment's creator (the row payload already carries it). Composes with every other filter through the same WHERE builder.
  • running_from / running_to query params — ISO dates, inclusive on both bounds, overlap semantics against the experiment's running windows: actual runs (including an open-ended run extending to now), or the scheduled window when the experiment has never run. Orthogonal to status — it filters by when the experiment ran or will run, not by its current state.
  • All counts (per-status, per-goal) and pagination.totalEntries reflect these filters like any other.
Consumer

#37007's spec (specs/37007-list-server-side-swap/spec.md, branch oidacra/experiments-portlet-list-server-side-swap) encodes this expanded contract as its entry condition and is blocked until this issue lands on main.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with AbstractExperimentFilter, ExperimentsFactoryImpl.list(), ExperimentsAPI, and the v2 tags TagResource pattern for PaginationUtil.getPageView; then inspect ExperimentsAPIImpl permission paths and the identifier schema. Run the referenced Postman collection and ExperimentAPIImpIntegrationTest while adding coverage for combined filters, pagination, sorting, counts, and permissions. Done means paginated responses and headers have correct totals, stable filtering and sorting, and unauthorized experiments are excluded without breaking system-user callers.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, postgresql
Domain
api, backend, databases, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.