diff --git a/core-web/libs/data-access/src/index.ts b/core-web/libs/data-access/src/index.ts index 75c3d5ae76d0..48c408f9d800 100644 --- a/core-web/libs/data-access/src/index.ts +++ b/core-web/libs/data-access/src/index.ts @@ -6,6 +6,7 @@ export * from './lib/dot-alert-confirm/dot-alert-confirm.service'; export * from './lib/dot-analytics-search/dot-analytics-search.service'; export * from './lib/dot-analytics-tracker/dot-analytics-tracker.service'; export * from './lib/dot-apps/dot-apps.service'; +export * from './lib/dot-bulk-refresh/dot-bulk-refresh.service'; export * from './lib/dot-categories/dot-categories.service'; export * from './lib/dot-containers/dot-containers.service'; export * from './lib/dot-content-drive/dot-content-drive.service'; diff --git a/core-web/libs/data-access/src/lib/dot-bulk-refresh/dot-bulk-refresh.service.spec.ts b/core-web/libs/data-access/src/lib/dot-bulk-refresh/dot-bulk-refresh.service.spec.ts new file mode 100644 index 000000000000..806bb27a427a --- /dev/null +++ b/core-web/libs/data-access/src/lib/dot-bulk-refresh/dot-bulk-refresh.service.spec.ts @@ -0,0 +1,63 @@ +import { createHttpFactory, HttpMethod, SpectatorHttp } from '@openng/spectator/jest'; + +import { DotBulkRefreshSubmitResponse } from '@dotcms/dotcms-models'; + +import { DotBulkRefreshService } from './dot-bulk-refresh.service'; + +describe('DotBulkRefreshService', () => { + let spectator: SpectatorHttp; + + const createHttp = createHttpFactory(DotBulkRefreshService); + + const SUBMIT_URL = '/api/v1/content/_bulkrefresh'; + + beforeEach(() => { + spectator = createHttp(); + }); + + it('should submit the inodes without asking for per-item results', () => { + spectator.service.refresh(['inode-1', 'inode-2']).subscribe(); + + const req = spectator.expectOne(SUBMIT_URL, HttpMethod.POST); + // Counters are all the completion event carries, and the per-item records are no longer + // readable over REST now that the status endpoint is gone. + expect(req.request.body).toEqual({ + contentletIds: ['inode-1', 'inode-2'], + includeItemResults: false + }); + }); + + it('should emit the accepted job handle', () => { + let emitted: DotBulkRefreshSubmitResponse | null | undefined; + spectator.service.refresh(['inode-1']).subscribe((result) => (emitted = result)); + + const entity: DotBulkRefreshSubmitResponse = { + jobId: 'job-1', + submitted: 1 + }; + spectator.expectOne(SUBMIT_URL, HttpMethod.POST).flush({ entity }); + + expect(emitted).toEqual(entity); + }); + + it('should complete after the submit rather than waiting on the job', () => { + // The whole point of the push model: nothing here follows the run. A subscriber gets one value + // and the observable completes, so there is no interval left behind to leak or to cancel. + let completed = false; + spectator.service.refresh(['inode-1']).subscribe({ complete: () => (completed = true) }); + + spectator.expectOne(SUBMIT_URL, HttpMethod.POST).flush({ + entity: { jobId: 'job-1', submitted: 1 } + }); + + expect(completed).toBe(true); + }); + + it('should not call the endpoint at all for an empty selection', () => { + let emitted: DotBulkRefreshSubmitResponse | null | undefined; + spectator.service.refresh([]).subscribe((result) => (emitted = result)); + + spectator.controller.expectNone(SUBMIT_URL); + expect(emitted).toBeNull(); + }); +}); diff --git a/core-web/libs/data-access/src/lib/dot-bulk-refresh/dot-bulk-refresh.service.ts b/core-web/libs/data-access/src/lib/dot-bulk-refresh/dot-bulk-refresh.service.ts new file mode 100644 index 000000000000..92b70777f099 --- /dev/null +++ b/core-web/libs/data-access/src/lib/dot-bulk-refresh/dot-bulk-refresh.service.ts @@ -0,0 +1,50 @@ +import { Observable, of } from 'rxjs'; + +import { HttpClient } from '@angular/common/http'; +import { inject, Injectable } from '@angular/core'; + +import { map } from 'rxjs/operators'; + +import { DotBulkRefreshSubmitResponse } from '@dotcms/dotcms-models'; + +const BULK_REFRESH_URL = '/api/v1/content/_bulkrefresh'; + +/** + * Submits a selection of contentlets to be reindexed through `POST /api/v1/content/_bulkrefresh`. + * + * The endpoint is job-backed and answers `202` immediately: the reindex continues in the background and + * this service does not wait for it. Completion arrives by push — a `BULK_REFRESH_COMPLETED` system event + * over the websocket the admin UI already holds open — so there is deliberately nothing here that polls a + * status endpoint. Asking every 1.5 seconds for up to five minutes is what this replaced. + */ +@Injectable({ + providedIn: 'root' +}) +export class DotBulkRefreshService { + readonly #http = inject(HttpClient); + + /** + * Asks for every given contentlet to be reindexed. + * + * @param inodes Contentlet inodes. The server collapses them by identifier, so the `total` it later + * reports can be lower than what was sent. + * @returns The accepted job's handle, or `null` for an empty selection — which is not worth a request. + */ + refresh(inodes: string[]): Observable { + if (!inodes.length) { + return of(null); + } + + return this.#http + .post<{ + entity: DotBulkRefreshSubmitResponse; + }>(BULK_REFRESH_URL, { + contentletIds: inodes, + // Counters are all the completion event carries, and nothing in this feature reads the + // per-item records. They remain reachable through the generic job-status endpoint if a + // drill-down is ever built. + includeItemResults: false + }) + .pipe(map((response) => response.entity)); + } +} diff --git a/core-web/libs/data-access/src/lib/dot-websocket/dot-system-event-type.model.ts b/core-web/libs/data-access/src/lib/dot-websocket/dot-system-event-type.model.ts index caa78aff4eef..7d24d4360d87 100644 --- a/core-web/libs/data-access/src/lib/dot-websocket/dot-system-event-type.model.ts +++ b/core-web/libs/data-access/src/lib/dot-websocket/dot-system-event-type.model.ts @@ -15,7 +15,9 @@ export enum DotSystemEventType { DELETE_SITE = 'DELETE_SITE', SWITCH_SITE = 'SWITCH_SITE', UPDATE_SITE_PERMISSIONS = 'UPDATE_SITE_PERMISSIONS', - UPDATE_PORTLET_LAYOUTS = 'UPDATE_PORTLET_LAYOUTS' + UPDATE_PORTLET_LAYOUTS = 'UPDATE_PORTLET_LAYOUTS', + /** A bulk content reindex finished; the payload carries the run's counters. */ + BULK_REFRESH_COMPLETED = 'BULK_REFRESH_COMPLETED' } /** diff --git a/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts b/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts index 93677993ea0d..875c5d3bcf95 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-content-drive.model.ts @@ -304,3 +304,48 @@ export interface DotContentDriveSearchResponse { nextContentCursor: number; nextFolderCursor: number; } + +/** + * The `202 Accepted` body of `POST /api/v1/content/_bulkrefresh`. + * + * Reindexing a selection is job-backed: the submit call only accepts the work, so this carries the + * handle to follow it rather than any outcome. + */ +export interface DotBulkRefreshSubmitResponse { + /** The job's id — the handle for the cancel call. */ + jobId: string; + /** + * Inodes accepted, before the server collapses them by identifier. The de-duplicated `total` + * arrives with the result and is often smaller, so this is not a count of reindexed items. + */ + submitted: number; +} + +/** + * Counters a finished bulk refresh reports. + * + * `successCount + failedCount + skippedCount === total` in every terminal state, which is what lets a + * caller know it can stop waiting and settle every row it asked about. + */ +export interface DotBulkRefreshCounts { + /** Unique identifiers reindexed, after de-duplication — not the submitted inode count. */ + total: number; + successCount: number; + failedCount: number; + /** Never attempted, because the run was cancelled before reaching them. */ + skippedCount: number; + /** Index writes across the whole run; higher than `total` when content has several versions. */ + versionsIndexed: number; +} + +/** + * The payload of a `BULK_REFRESH_COMPLETED` system event. + * + * Pushed over the websocket when a run settles, scoped to whoever submitted it. The counter fields are + * all optional: a job that finished without reporting any carries only `state`, and a caller must treat + * that as a failure rather than as a clean run over nothing, which is what all-zero counters would look + * like. + */ +export interface DotBulkRefreshCompletedEvent extends Partial { + state: string; +} diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.spec.ts index b822fed85f28..1debdd54a6bc 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.spec.ts @@ -5,6 +5,8 @@ import { of, throwError } from 'rxjs'; import { provideHttpClient } from '@angular/common/http'; import { signal } from '@angular/core'; +import { MessageService } from 'primeng/api'; + import { AddToBundleService, DotCurrentUserService, @@ -192,8 +194,12 @@ describe('DotContentDriveActionCenterComponent', () => { executeQuickAction: jest.fn(), executeWorkflowAction: jest.fn(), executeAddToBundle: jest.fn(), - executePushPublish: jest.fn() + executePushPublish: jest.fn(), + executeRefresh: jest.fn() }), + // The trigger toast for a backgrounded reindex goes through PrimeNG's MessageService, + // which in the app resolves to the shell's instance so the toast outlives this dialog. + mockProvider(MessageService, { add: jest.fn() }), mockProvider(DotMessageService, { get: jest.fn().mockImplementation((key: string) => key) }), @@ -535,7 +541,7 @@ describe('DotContentDriveActionCenterComponent', () => { } }); - it('should render Refresh as a disabled placeholder', () => { + it('should render Refresh as a selectable action', () => { spectator.detectChanges(); const row = spectator.query( @@ -543,10 +549,8 @@ describe('DotContentDriveActionCenterComponent', () => { ) as HTMLButtonElement; expect(row).toBeTruthy(); - expect(row.disabled).toBe(true); - expect( - spectator.query('[data-testid="quick-action-coming-soon-REFRESH"]') - ).toBeTruthy(); + expect(row.disabled).toBe(false); + expect(spectator.query('[data-testid="quick-action-coming-soon-REFRESH"]')).toBeNull(); }); it('should disable Push Publish without the coming-soon badge when no environment exists', () => { @@ -568,13 +572,12 @@ describe('DotContentDriveActionCenterComponent', () => { spectator.detectChanges(); // Disabled, so a real click cannot land — called directly to prove the guard holds if - // one ever does. Both blocked states are covered: Refresh is a placeholder, Push - // Publish has nowhere to send to. - for (const id of ['REFRESH', 'PUSH_PUBLISH']) { - spectator.component['onSelectQuickAction']( - spectator.component['$quickActions']().find((action) => action.id === id)! - ); - } + // one ever does. Push Publish has nowhere to send to; Refresh is no longer blocked. + spectator.component['onSelectQuickAction']( + spectator.component['$quickActions']().find( + (action) => action.id === 'PUSH_PUBLISH' + )! + ); spectator.detectChanges(); @@ -582,6 +585,104 @@ describe('DotContentDriveActionCenterComponent', () => { expect(store.executeQuickAction).not.toHaveBeenCalled(); }); + it('should execute Refresh through its own store method, not the workflow fire', () => { + // Refresh speaks inodes like Lock and Unlock but goes to a job-backed endpoint of its + // own, so routing it through `executeQuickAction` would fire a system action that does + // not exist. + executeQuickAction('REFRESH'); + + expect(store.executeRefresh).toHaveBeenCalledWith(expect.any(String), [ + 'inode-1', + 'inode-2' + ]); + expect(store.executeQuickAction).not.toHaveBeenCalled(); + }); + + it('should send only the rows left checked in the Refresh preview', () => { + openQuickActionPreview('REFRESH'); + toggleRow(0); + spectator.click('[data-testid="action-preview-execute"]'); + spectator.detectChanges(); + + const [, inodes] = (store.executeRefresh as unknown as jest.Mock).mock.calls[0]; + + expect(inodes).toEqual(['inode-2']); + }); + + it('should not ask for configuration before refreshing', () => { + // Nothing to collect: a reindex takes no assignee, no destination and no environments, + // so it goes straight to the preview like Lock and Unlock do. + openQuickActionPreview('REFRESH'); + + expect(spectator.query('[data-testid="action-preview"]')).toBeTruthy(); + }); + + it('should toast at trigger that the reindex runs in the background', () => { + // The only feedback the user gets now: there is no "Applying ..." indicator for a reindex, + // because it runs for minutes and cannot report progress. + const messageService = spectator.inject(MessageService); + + executeQuickAction('REFRESH'); + + expect(messageService.add).toHaveBeenCalledWith( + expect.objectContaining({ + severity: 'info', + summary: 'content-drive.action-center.toast.reindex-started' + }) + ); + }); + + it('should not claim a reindex started when nothing was submitted', () => { + // Toasting with nothing submitted tells the user their reindex is running when it is not, + // and the hand-off clears their selection on the way out, so they lose the rows too. With + // the in-flight guard gone, an emptied preview is the remaining way to reach that. + const messageService = spectator.inject(MessageService); + + openQuickActionPreview('REFRESH'); + toggleRow(0); + toggleRow(1); + spectator.click('[data-testid="action-preview-execute"]'); + spectator.detectChanges(); + + expect(store.executeRefresh).not.toHaveBeenCalled(); + expect(messageService.add).not.toHaveBeenCalled(); + expect(store.setSelectedItems).not.toHaveBeenCalled(); + }); + + it('should not toast at trigger for the synchronous actions', () => { + // They settle in seconds and report through the toolbar indicator, so a "runs in the + // background" toast would be both wrong and noisy. + const messageService = spectator.inject(MessageService); + + executeQuickAction('LOCK'); + + expect(messageService.add).not.toHaveBeenCalled(); + }); + + it('should clear the grid selection when an action is handed off', () => { + // Once an action is fired the selection has served its purpose, and leaving the boxes + // ticked invited firing a second action over rows already being changed. + executeQuickAction('LOCK'); + + expect(store.setSelectedItems).toHaveBeenCalledWith([]); + }); + + it('should clear the grid selection when a reindex is handed off', () => { + // Matters most here: a reindex runs for minutes, so without this the rows stay ticked for + // the whole run. + executeQuickAction('REFRESH'); + + expect(store.setSelectedItems).toHaveBeenCalledWith([]); + }); + + it('should keep the selection when the dialog is merely dismissed', () => { + // Dismissing is not firing. Clearing here would lose a selection the user is still + // building. + openQuickActionPreview('LOCK'); + + expect(store.setSelectedItems).not.toHaveBeenCalled(); + }); + it('should keep Add to Bundle selectable', () => { spectator.detectChanges(); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.ts index d641d3b4546d..c9e3578d4035 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dialogs/dot-content-drive-action-center/dot-content-drive-action-center.component.ts @@ -11,6 +11,7 @@ import { import { FormsModule } from '@angular/forms'; import { AccordionModule } from 'primeng/accordion'; +import { MessageService } from 'primeng/api'; import { BadgeModule } from 'primeng/badge'; import { ButtonModule } from 'primeng/button'; import { MessageModule } from 'primeng/message'; @@ -51,6 +52,7 @@ import { ADD_TO_BUNDLE_ACTION_ID, DotActionCenterQuickAction, PUSH_PUBLISH_ACTION_ID, + REFRESH_ACTION_ID, DotActionInputKind, eligibleContentlets, excludeFolders, @@ -79,9 +81,10 @@ type DotActionCenterConfigureKind = DotActionInputKind | 'bundle'; * Two sections: * * 1. **Quick Actions** — the bulk operations the old search toolbar offered outside its workflow - * dropdown: Lock, Unlock, Add to Bundle, and placeholders for Push Publish and Refresh. Lock and + * dropdown: Lock, Unlock, Add to Bundle, Refresh, and a placeholder for Push Publish. Lock and * Unlock fire over the whole eligible selection in one request via - * `POST /api/v1/workflow/actions/default/fire/{systemAction}`. Counts are derived client-side + * `POST /api/v1/workflow/actions/default/fire/{systemAction}`; Refresh goes to its own job-backed + * `POST /api/v1/content/_bulkrefresh`, and its completion is pushed over the websocket. Counts are derived client-side * from row state (see `getQuickActions`). * 2. **Workflow Actions** — one collapsible panel per workflow scheme, from * `POST /api/v1/workflow/contentlet/actions/bulk`, queried **once per content type** in the @@ -169,6 +172,11 @@ type DotActionCenterConfigureKind = DotActionInputKind | 'bundle'; export class DotContentDriveActionCenterComponent implements OnInit { readonly #store = inject(DotContentDriveStore); readonly #dotMessageService = inject(DotMessageService); + /** + * Resolves to the shell's instance, so a toast added here survives this dialog closing immediately + * afterwards. + */ + readonly #messageService = inject(MessageService); readonly #workflowsActionsService = inject(DotWorkflowsActionsService); readonly #pushPublishService = inject(PushPublishService); @@ -710,6 +718,32 @@ export class DotContentDriveActionCenterComponent implements OnInit { return; } + // Refresh speaks inodes like the workflow quick actions, but goes to its own job-backed + // endpoint rather than the system-action fire, so it branches here rather than falling through. + if (quickAction.id === REFRESH_ACTION_ID) { + const actionName = this.#dotMessageService.get(quickAction.name); + this.#store.executeRefresh(actionName, inodes); + + // The only feedback for a reindex until it finishes. It gets no "Applying ..." indicator, + // because it runs for minutes and the endpoint reports no progress — so saying up front + // that it is backgrounded is the honest substitute, and it is why the Action Center is left + // usable rather than locked. + this.#messageService.add({ + severity: 'info', + summary: this.#dotMessageService.get( + 'content-drive.action-center.toast.reindex-started' + ), + detail: this.#dotMessageService.get( + 'content-drive.action-center.toast.reindex-started-detail', + actionName, + String(inodes.length) + ) + }); + this.handOffToToolbar(); + + return; + } + this.#store.executeQuickAction( quickAction.id, this.#dotMessageService.get(quickAction.name), @@ -774,12 +808,21 @@ export class DotContentDriveActionCenterComponent implements OnInit { * it is modal, so it dims the toolbar that is reporting the run, and it blocks the grid while * work happens that no longer needs the dialog to be alive. Closing here is what makes the * toolbar indicator observable — otherwise the only window to see it is the milliseconds between - * the user manually closing the dialog and the request settling. + * the user manually closing the dialog and the request settling. Refresh is the exception: it shows + * no indicator at all, having already said by toast that it runs in the background. * * Counts are also stale from this point on: the contentlets are moving to a new step, so the * numbers this dialog is showing no longer hold. */ private handOffToToolbar(): void { + // The selection has served its purpose the moment an action is fired, and leaving the rows + // ticked invited firing a second action over content already being changed. Cleared here rather + // than after the run settles, because the settle path never runs on an error or a timeout — and + // for a reindex it is minutes away, so the boxes would sit checked for the whole job. + // + // Deliberately only on hand-off: dismissing the dialog with X, ESC or the mask keeps the + // selection, because the user may still be building it. + this.#store.setSelectedItems([]); this.#store.closeDialog(); } diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html index 4e0fb3cf7815..8b6707471d81 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.html @@ -37,6 +37,7 @@ class="col-start-2 row-start-3 overflow-auto"> { ); }); + it('should use an action-specific partial copy when the result names one', () => { + // A reindex falls short for different reasons than a workflow fire — content that could + // not be read or indexed, and a cancelled run. Borrowing the default copy would blame + // permissions, locks and workflow steps, none of which apply, and send the user off to + // fix something that was never the problem. + settle({ + actionName: 'Refresh', + successCount: 2, + skippedCount: 1, + failCount: 1, + partialDetailKey: 'content-drive.action-center.toast.refreshed-partial' + }); + + expect(dotMessageService.get).toHaveBeenCalledWith( + 'content-drive.action-center.toast.refreshed-partial', + 'Refresh', + '2', + '1', + '1' + ); + }); + + it('should keep the default partial copy for results that name none', () => { + settle({ + actionName: 'Publish', + successCount: 1, + skippedCount: 0, + failCount: 1 + }); + + expect(dotMessageService.get).toHaveBeenCalledWith( + 'content-drive.action-center.toast.executed-partial', + 'Publish', + '1', + '1', + '0' + ); + }); + + it('should ignore the action-specific copy on a clean run', () => { + // Nothing fell short, so there is no cause to name — the plain success copy is right + // whatever the action would have said about a shortfall. + settle({ + actionName: 'Refresh', + successCount: 3, + skippedCount: 0, + failCount: 0, + partialDetailKey: 'content-drive.action-center.toast.refreshed-partial' + }); + + expect(messageService.add).toHaveBeenCalledWith( + expect.objectContaining({ + severity: 'success', + detail: 'content-drive.action-center.toast.executed-detail' + }) + ); + }); + it('should refresh the grid, close the dialog and consume the result', () => { settle({ actionName: 'Publish', @@ -941,6 +999,26 @@ describe('DotContentDriveShellComponent', () => { }); }); + describe('grid selection binding', () => { + it('should drive the grid from the store so clearing it unchecks the rows', () => { + // The grid is in controlled mode purely so this holds. Left uncontrolled it keeps its own + // checked set and only drops it when the items reference changes, which meant a selection + // cleared on action hand-off stayed visibly ticked until the next search returned. + store.selectedItems.mockReturnValue([MOCK_ITEMS[0]]); + spectator.detectChanges(); + + const listView = spectator.query(DotFolderListViewComponent); + + expect(listView).toBeTruthy(); + expect(listView.$selection()).toEqual([MOCK_ITEMS[0]]); + + // Not asserting the clear here: `selectedItems` is mocked as a plain jest.fn rather than a + // signal, so changing its return value cannot notify change detection. What matters is + // that the input is bound to store state at all — the propagation is Angular's, and the + // store's own spec covers that loadItems and hand-off empty that state. + }); + }); + describe('onSelectItems', () => { it('should update selectedItems in store when selectionChange is emitted', () => { const folderListView = spectator.debugElement.query( diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts index cdbbe976ee24..d42ae02e837c 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts @@ -225,6 +225,16 @@ export class DotContentDriveShellComponent { */ protected readonly $activeDialog = signal(undefined); + /** + * The grid's checked rows, driven from the store. + * + * Passing this puts `dot-folder-list-view` in its controlled mode, which is what makes clearing the + * store actually uncheck the boxes. Left uncontrolled, the grid keeps its own selection and only + * drops it when the `items` reference changes — so a selection cleared on action hand-off stayed + * visibly ticked until the next search returned. + */ + protected readonly $selectedItems = this.#store.selectedItems; + /** Folder payload for the folder dialog (narrowed from the dialog payload union by type). */ readonly $folderPayload = computed(() => { const dialog = this.$activeDialog(); @@ -503,7 +513,7 @@ export class DotContentDriveShellComponent { return; } - const { actionName, successCount, skippedCount, failCount } = result; + const { actionName, successCount, skippedCount, failCount, partialDetailKey } = result; // Skips and failures are not mutually exclusive: one bulk fire over a mixed-type selection // can skip items whose scheme does not own the action *and* be refused on items that are @@ -518,7 +528,9 @@ export class DotContentDriveShellComponent { const detail = isPartial ? this.#dotMessageService.get( - 'content-drive.action-center.toast.executed-partial', + // Actions whose failures and skips mean something other than permissions, locks and + // workflow steps say so themselves — see `partialDetailKey`. + partialDetailKey ?? 'content-drive.action-center.toast.executed-partial', actionName, String(successCount), String(failCount), diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts index 1c8912b435ed..d81ede2a6b8a 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/shared/models.ts @@ -157,6 +157,16 @@ export interface DotContentDriveActionExecutionResult { successCount: number; skippedCount: number; failCount: number; + /** + * i18n key for the partial-outcome copy, when the default does not fit. + * + * The default names workflow-specific causes next to each number — permissions and locks for + * failures, "not on their workflow step" for skips. Those are the right causes for a bulk fire and + * the wrong ones for anything else, and a shortfall explained by the wrong cause sends the user off + * to fix something that was never the problem. An action whose failures and skips mean something + * different supplies its own copy rather than borrowing that one. + */ + partialDetailKey?: string; } /** diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.spec.ts index dd14f310cfa9..dc9b94cad99d 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/dot-content-drive.store.spec.ts @@ -9,10 +9,14 @@ import { NEVER, of, Subject, throwError } from 'rxjs'; import { Location } from '@angular/common'; import { HttpErrorResponse, provideHttpClient } from '@angular/common/http'; +import { fakeAsync, tick } from '@angular/core/testing'; import { ActivatedRoute } from '@angular/router'; import { AddToBundleService, + DotBulkRefreshService, + DotEventsSocket, + DotMessageService, PushPublishService, DotContentDriveService, DotLanguagesService, @@ -24,6 +28,7 @@ import { } from '@dotcms/data-access'; import { DotAjaxActionResponseView, + DotBulkRefreshCompletedEvent, DotContentDriveItem, DotContentDriveSearchResponse, DotCurrentUser, @@ -96,6 +101,7 @@ describe('DotContentDriveStore', () => { // Also required by `withActionExecution`, which fires Add to Bundle from the store. mockProvider(AddToBundleService), mockProvider(PushPublishService), + mockProvider(DotBulkRefreshService), mockProvider(DotHttpErrorManagerService), // The store subscribes to Location (popstate re-hydration); capture the handler here. mockProvider(Location, { @@ -993,6 +999,7 @@ describe('DotContentDriveStore - onInit', () => { // Also required by `withActionExecution`, which fires Add to Bundle from the store. mockProvider(AddToBundleService), mockProvider(PushPublishService), + mockProvider(DotBulkRefreshService), mockProvider(DotHttpErrorManagerService), // The store subscribes to Location (popstate re-hydration); capture the handler here. mockProvider(Location, { @@ -1062,6 +1069,7 @@ describe('DotContentDriveStore - Browser Back/Forward (popstate) re-hydration', // Also required by `withActionExecution`, which fires Add to Bundle from the store. mockProvider(AddToBundleService), mockProvider(PushPublishService), + mockProvider(DotBulkRefreshService), mockProvider(DotHttpErrorManagerService), // withFlags fetches feature flags on init; stub so no real HTTP fires. mockProvider(DotPropertiesService, { @@ -1308,6 +1316,7 @@ describe('DotContentDriveStore - Content Loading Effect', () => { // Also required by `withActionExecution`, which fires Add to Bundle from the store. mockProvider(AddToBundleService), mockProvider(PushPublishService), + mockProvider(DotBulkRefreshService), mockProvider(DotHttpErrorManagerService), // The store subscribes to Location (popstate re-hydration); capture the handler here. mockProvider(Location, { @@ -1578,6 +1587,9 @@ describe('DotContentDriveStore - withActionExecution', () => { let store: InstanceType; let fireService: jest.Mocked; let httpErrorManager: jest.Mocked; + let bulkRefreshService: jest.Mocked; + /** Declared outside the factory so a test can push into the hook's subscription. */ + const bulkRefreshEvents$ = new Subject(); const createService = createServiceFactory({ service: DotContentDriveStore, @@ -1603,6 +1615,13 @@ describe('DotContentDriveStore - withActionExecution', () => { // Add to Bundle leaves the workflow path entirely and posts to the legacy bundle servlet. mockProvider(AddToBundleService, { addToBundle: jest.fn() }), mockProvider(PushPublishService, { pushPublishAssets: jest.fn() }), + // Refresh is the one quick action that is job-backed: the service submits and returns, so + // the store only ever sees a single-emission observable. + mockProvider(DotBulkRefreshService, { refresh: jest.fn() }), + // The completion event is pushed, so the socket is the seam the run settles through. + // A Subject lets the tests below emit one without a server. + mockProvider(DotEventsSocket, { on: jest.fn(() => bulkRefreshEvents$) }), + mockProvider(DotMessageService, { get: jest.fn((key: string) => key) }), mockProvider(DotHttpErrorManagerService, { handle: jest.fn() }), // The store subscribes to Location (popstate re-hydration); stub so it is inert here. mockProvider(Location, { @@ -1641,6 +1660,223 @@ describe('DotContentDriveStore - withActionExecution', () => { of({ results: [], summary: { affected: 2, successCount: 2, failCount: 0, time: 1 } }) ); fireService.bulkFire.mockReturnValue(of({ successCount: 2, skippedCount: 0, fails: [] })); + + bulkRefreshService = spectator.inject( + DotBulkRefreshService + ) as jest.Mocked; + bulkRefreshService.refresh.mockReturnValue(of({ jobId: 'job-1', submitted: 1 })); + }); + + describe('executeRefresh', () => { + it('should not publish a running action, because the reindex is backgrounded', () => { + // actionExecution drives the toolbar's "Applying ... to N item(s)" indicator and locks the + // Action Center. A reindex reports itself by toast at trigger and again by push at the end, + // so an indicator it cannot update, and a lock lasting minutes, are both wrong for it. + store.executeRefresh('Refresh', ['inode-1', 'inode-2']); + + expect(store.actionExecution()).toBeUndefined(); + }); + + it('should let a second reindex be fired', () => { + // No in-flight guard: the only thing it could protect against is a double-fire, and firing + // clears the selection, so a second run takes a deliberate re-selection. Guarding it needed + // a timeout to un-wedge the flag when a completion event went missing, and that timeout was + // the larger cost - a 504 minutes later, about a job that had most likely succeeded, with + // nothing on screen waiting for it. + store.executeRefresh('Refresh', ['inode-1']); + store.executeRefresh('Refresh', ['inode-2']); + + expect(bulkRefreshService.refresh).toHaveBeenCalledTimes(2); + }); + + it('should not block the other actions while a reindex runs', () => { + // The whole point of backgrounding it: a reindex takes minutes and shares nothing with + // these, so locking them out for its duration was the bug. + store.executeRefresh('Refresh', ['inode-1']); + + store.executeQuickAction('LOCK', 'Lock', ['inode-2']); + expect(fireService.fireDefaultAction).toHaveBeenCalled(); + + store.executeWorkflowAction('wf-1', 'Publish', ['inode-3']); + expect(fireService.bulkFire).toHaveBeenCalled(); + }); + + it('should send the inodes to the bulk refresh service', () => { + store.executeRefresh('Refresh', ['inode-1', 'inode-2']); + + expect(bulkRefreshService.refresh).toHaveBeenCalledWith(['inode-1', 'inode-2']); + }); + + it('should not settle on the submit response', () => { + // The 202 says accepted, not done. Settling here is what would produce the misleading + // success this endpoint exists to remove. + store.executeRefresh('Refresh', ['inode-1']); + + expect(store.actionExecutionResult()).toBeUndefined(); + }); + + it('should not fire when there are no inodes', () => { + store.executeRefresh('Refresh', []); + + expect(bulkRefreshService.refresh).not.toHaveBeenCalled(); + }); + + it('should report a submit that fails outright', () => { + // The one failure a client can see directly: no job was created, so no completion event is + // ever coming and the error toast is the only report the user gets. + bulkRefreshService.refresh.mockReturnValue( + throwError(() => new HttpErrorResponse({ status: 403 })) + ); + + store.executeRefresh('Refresh', ['inode-1']); + + expect(httpErrorManager.handle).toHaveBeenCalled(); + expect(store.actionExecutionResult()).toBeUndefined(); + }); + + it('should leave nothing waiting on a timer', fakeAsync(() => { + // There is no completion deadline. A reindex is reported by push, and by a notification the + // server writes whether or not the socket delivered - so a client-side deadline could only + // ever invent a failure for a run it has no information about. + store.executeRefresh('Refresh', ['inode-1']); + + tick(60 * 60 * 1000); + + expect(httpErrorManager.handle).not.toHaveBeenCalled(); + expect(store.actionExecutionResult()).toBeUndefined(); + })); + }); + + describe('bulk refresh completion push', () => { + it('should settle the run when the completion event arrives on the socket', () => { + // Proves the wiring, not just the reporter: without the hook subscribing, a finished run + // would leave the reindex marked in flight forever and never toast. + store.executeRefresh('Refresh', ['inode-1']); + + bulkRefreshEvents$.next({ + state: 'SUCCESS', + total: 1, + successCount: 1, + failedCount: 0, + skippedCount: 0, + versionsIndexed: 1 + }); + + expect(store.actionExecutionResult()).toEqual({ + actionName: 'Refresh', + successCount: 1, + skippedCount: 0, + failCount: 0, + partialDetailKey: 'content-drive.action-center.toast.refreshed-partial' + }); + }); + }); + + describe('reportRefreshCompleted', () => { + it('should not clear an unrelated action that is still in flight', () => { + // Now that a reindex no longer locks the dialog, another action can genuinely be running + // when the reindex event lands. Blanket-clearing actionExecution here would un-gate that + // action early and let a second one fire over the same rows. + fireService.fireDefaultAction.mockReturnValue(NEVER); + store.executeRefresh('Refresh', ['inode-1']); + store.executeQuickAction('LOCK', 'Lock', ['inode-2']); + + const lockInFlight = store.actionExecution(); + expect(lockInFlight).toEqual({ actionName: 'Lock', total: 1 }); + + store.reportRefreshCompleted('Refresh', { + state: 'FAILED_PERMANENTLY', + total: 0, + successCount: 0, + failedCount: 0, + skippedCount: 0, + versionsIndexed: 0 + }); + + expect(store.actionExecution()).toBe(lockInFlight); + }); + + it('should report an unusable outcome rather than settling on it', () => { + store.executeRefresh('Refresh', ['inode-1']); + + store.reportRefreshCompleted('Refresh', { state: 'SUCCESS' }); + + expect(httpErrorManager.handle).toHaveBeenCalled(); + expect(store.actionExecutionResult()).toBeUndefined(); + }); + + it('should settle with the pushed counters and its own partial copy', () => { + store.reportRefreshCompleted('Refresh', { + state: 'SUCCESS', + total: 4, + successCount: 2, + failedCount: 1, + skippedCount: 1, + versionsIndexed: 3 + }); + + expect(store.actionExecutionResult()).toEqual({ + actionName: 'Refresh', + successCount: 2, + skippedCount: 1, + failCount: 1, + partialDetailKey: 'content-drive.action-center.toast.refreshed-partial' + }); + }); + + it('should still report a cancelled run, whose counters do account for every item', () => { + store.reportRefreshCompleted('Refresh', { + state: 'CANCELED', + total: 4, + successCount: 1, + failedCount: 0, + skippedCount: 3, + versionsIndexed: 1 + }); + + expect(httpErrorManager.handle).not.toHaveBeenCalled(); + expect(store.actionExecutionResult()?.skippedCount).toBe(3); + }); + + it('should report an error, not a success toast, when the job failed', () => { + // A job that died mid-run still carries the counters it had reached, so an all-zero result + // from FAILED_PERMANENTLY is indistinguishable from a clean run over nothing unless the + // state is checked. + store.reportRefreshCompleted('Refresh', { + state: 'FAILED_PERMANENTLY', + total: 0, + successCount: 0, + failedCount: 0, + skippedCount: 0, + versionsIndexed: 0 + }); + + expect(httpErrorManager.handle).toHaveBeenCalled(); + expect(store.actionExecutionResult()).toBeUndefined(); + }); + + it('should report an error when the counters do not account for every item', () => { + // A run that stopped after 3 of 10 reports successCount 3 with nothing failed or skipped. + // Settling on that would silently drop the 7 never attempted. + store.reportRefreshCompleted('Refresh', { + state: 'SUCCESS', + total: 10, + successCount: 3, + failedCount: 0, + skippedCount: 0, + versionsIndexed: 3 + }); + + expect(httpErrorManager.handle).toHaveBeenCalled(); + expect(store.actionExecutionResult()).toBeUndefined(); + }); + + it('should report an error when the event carried no counters at all', () => { + store.reportRefreshCompleted('Refresh', { state: 'SUCCESS' }); + + expect(httpErrorManager.handle).toHaveBeenCalled(); + expect(store.actionExecutionResult()).toBeUndefined(); + }); }); describe('executeQuickAction', () => { diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/action-execution/withActionExecution.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/action-execution/withActionExecution.ts index 8a73178d422d..112a6a178e32 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/action-execution/withActionExecution.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/store/features/action-execution/withActionExecution.ts @@ -1,20 +1,33 @@ -import { patchState, signalStoreFeature, type, withMethods, withState } from '@ngrx/signals'; +import { + patchState, + signalStoreFeature, + type, + withHooks, + withMethods, + withState +} from '@ngrx/signals'; import { EMPTY, Observable } from 'rxjs'; import { HttpErrorResponse } from '@angular/common/http'; -import { inject } from '@angular/core'; +import { DestroyRef, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { catchError, take } from 'rxjs/operators'; import { AddToBundleService, + DotBulkRefreshService, + DotEventsSocket, DotHttpErrorManagerService, + DotMessageService, + DotSystemEventType, DotWorkflowActionsFireService, PushPublishService } from '@dotcms/data-access'; import { DotActionBulkRequestOptions, DotAjaxActionResponseView, + DotBulkRefreshCompletedEvent, DotBundle, DotWorkflowPushPublishValue } from '@dotcms/dotcms-models'; @@ -64,7 +77,9 @@ export function withActionExecution() { workflowActionsFireService = inject(DotWorkflowActionsFireService), httpErrorManagerService = inject(DotHttpErrorManagerService), addToBundleService = inject(AddToBundleService), - pushPublishService = inject(PushPublishService) + pushPublishService = inject(PushPublishService), + bulkRefreshService = inject(DotBulkRefreshService), + destroyRef = inject(DestroyRef) ) => { /** * Settles a finished run by publishing its result for the shell to present. @@ -233,6 +248,122 @@ export function withActionExecution() { }); }, + /** + * Reindexes the given contentlet inodes. + * + * Submit-and-forget: the endpoint answers 202 and nothing here waits or guards. A + * second reindex is allowed to be fired — firing clears the selection, so it takes a + * deliberate re-selection, and reindexing the same rows again is wasteful rather + * than wrong. + * + * Reported through the same {@link onSettled} path as everything else, with its own + * partial-outcome copy: a failure here is content that could not be read or indexed + * and a skip is a cancelled run, neither of which is what the default copy blames. + * + * Only SUCCESS and CANCELED are reported as outcomes, and only when the counters + * close over `total`. A job that died mid-run still carries counters describing how + * far it got, and reporting those as a result would turn a failure into a green + * toast - the exact misleading success this endpoint exists to remove. + */ + executeRefresh: (actionName: string, inodes: string[]): void => { + if (!inodes.length) { + return; + } + + // Note what is NOT set: actionExecution. That field shows an "Applying …" + // indicator and locks the Action Center, and neither fits a job that runs for + // minutes and cannot report progress. The user is told at trigger that this is + // backgrounded, and told again when it finishes. + // + // Note also what is not started: a completion deadline. Nothing on screen is + // waiting, so there is nothing for one to unblock - and a client that gave up + // after N minutes would be reporting a failure it has no evidence of, over a + // run the server records in the notification bell either way. + + // Submit and stop. The endpoint answers 202 and the reindex continues in the + // background; the outcome arrives on the socket subscription below rather than + // by asking for it. Nothing here waits. + bulkRefreshService + .refresh(inodes) + .pipe( + take(1), + catchError((error) => { + // The only reindex failure a client sees directly: no job was + // created, so no completion event is coming for it either. + httpErrorManagerService.handle(error); + + return EMPTY; + }), + takeUntilDestroyed(destroyRef) + ) + .subscribe(); + }, + + /** + * Reports a finished bulk refresh, from the pushed completion event. + * + * Feeds the same {@link onSettled} path as every other action, so the toast copy, + * severity, grid reload and selection clear all behave identically — with its own + * partial-outcome wording, because a reindex falls short for different reasons than a + * workflow fire. + * + * Three ways a run can arrive with nothing honest to report, all of which would + * otherwise render as a green success toast: + * + * 1. No counters at all. + * 2. A state whose counters describe only how far the job got before dying — a + * permanently failed job still carries the counters it had reached, so an all-zero + * result is indistinguishable from a clean run over nothing unless state is checked. + * 3. Counters that do not close over `total`, meaning the run did not account for + * every item and the shortfall is unexplained. + */ + reportRefreshCompleted: ( + actionName: string, + event: DotBulkRefreshCompletedEvent + ): void => { + const closes = + undefined !== event.total && + (event.successCount ?? 0) + + (event.failedCount ?? 0) + + (event.skippedCount ?? 0) === + event.total; + + if ('SUCCESS' !== event.state && 'CANCELED' !== event.state) { + // Note what is not touched: actionExecution. It may belong to a different + // action that is still running - a reindex no longer locks the dialog, so + // that is an ordinary situation, and clearing it here would un-gate that + // action early. + httpErrorManagerService.handle( + new HttpErrorResponse({ + status: 500, + statusText: `The reindex did not report a usable outcome (state: ${event.state})` + }) + ); + + return; + } + + if (!closes) { + httpErrorManagerService.handle( + new HttpErrorResponse({ + status: 500, + statusText: + 'The reindex counters did not account for every item' + }) + ); + + return; + } + + onSettled({ + actionName, + successCount: event.successCount ?? 0, + skippedCount: event.skippedCount ?? 0, + failCount: event.failedCount ?? 0, + partialDetailKey: 'content-drive.action-center.toast.refreshed-partial' + }); + }, + /** * Fires the selected workflow action over the given contentlet inodes. * @@ -370,6 +501,25 @@ export function withActionExecution() { } }; } - ) + ), + withHooks({ + onInit(store) { + const eventsSocket = inject(DotEventsSocket); + const dotMessageService = inject(DotMessageService); + const destroyRef = inject(DestroyRef); + + // The socket is already open app-wide, so subscribing costs nothing. This is what + // replaced polling: the run reports itself when it settles instead of being asked. + eventsSocket + .on(DotSystemEventType.BULK_REFRESH_COMPLETED) + .pipe(takeUntilDestroyed(destroyRef)) + .subscribe((event) => { + // Resolve the label here rather than server-side: the backend should not be + // composing user-facing copy, and this keeps the wording with the rest of the + // Action Center's i18n. + store.reportRefreshCompleted(dotMessageService.get('Refresh'), event); + }); + } + }) ); } diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.spec.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.spec.ts index 16871d85c42c..aadb7270797e 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.spec.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.spec.ts @@ -391,12 +391,43 @@ describe('action-center utils', () => { } }); - it('should flag Refresh as coming soon', () => { + it('should offer Refresh as a wired action', () => { const byId = new Map( getQuickActions([contentlet({ inode: 'a' })]).map((action) => [action.id, action]) ); - expect(byId.get(REFRESH_ACTION_ID)?.comingSoon).toBe(true); + expect(byId.get(REFRESH_ACTION_ID)?.comingSoon).toBe(false); + }); + + it('should offer Refresh regardless of live, archived or locked state', () => { + // The one quick action whose eligibility owes nothing to row state: none of these + // affect whether the index copy of a contentlet is stale, which is all a reindex fixes. + const items = [ + contentlet({ inode: 'a', live: true }), + contentlet({ inode: 'b', archived: true }), + contentlet({ inode: 'c', locked: true }), + contentlet({ inode: 'd', live: true, locked: true }) + ]; + + const refresh = getQuickActions(items).find( + (action) => action.id === REFRESH_ACTION_ID + ); + + expect(refresh?.count).toBe(4); + expect(refresh?.eligibleInodes).toEqual(['a', 'b', 'c', 'd']); + }); + + it('should drop folders from the Refresh selection', () => { + // The endpoint takes contentlet inodes only; a folder inode would come back as a + // per-item failure and make the count the dialog promised a lie. + const refresh = getQuickActions([ + contentlet({ inode: 'a' }), + folder('f1'), + contentlet({ inode: 'b' }) + ]).find((action) => action.id === REFRESH_ACTION_ID); + + expect(refresh?.count).toBe(2); + expect(refresh?.eligibleInodes).toEqual(['a', 'b']); }); it('should block Push Publish on the environments, not on coming-soon', () => { @@ -456,6 +487,7 @@ describe('action-center utils', () => { expect(byId.get(WORKFLOW_ACTION_ID.LOCK)?.comingSoon).toBe(false); expect(byId.get(WORKFLOW_ACTION_ID.UNLOCK)?.comingSoon).toBe(false); expect(byId.get(ADD_TO_BUNDLE_ACTION_ID)?.comingSoon).toBe(false); + expect(byId.get(REFRESH_ACTION_ID)?.comingSoon).toBe(false); }); it('should count the coming-soon actions over the whole selection', () => { @@ -470,7 +502,6 @@ describe('action-center utils', () => { const byId = new Map(getQuickActions(items).map((action) => [action.id, action.count])); expect(byId.get(PUSH_PUBLISH_ACTION_ID)).toBe(2); - expect(byId.get(REFRESH_ACTION_ID)).toBe(2); }); }); diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.ts index bda6e1b6879e..acc40ff6573b 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/action-center.ts @@ -38,8 +38,19 @@ export const PUSH_PUBLISH_ACTION_ID = 'PUSH_PUBLISH'; /** * Reindex the selection — the old search toolbar's "Refresh", backed by `_bulkrefresh`. * - * Placeholder: rendered but not wired. The endpoint streams progress over SSE and is not job-backed, - * so it cannot reuse the synchronous `bulkFire` path the other quick actions run on. + * Clears each contentlet's cache entry and rewrites every version of it to the index. Useful when + * content is right in the database but stale or missing from search results. **Not** a full index + * rebuild: that is `POST /api/v1/esindex/reindex`, a different operation over the whole index. + * + * The only quick action that is job-backed rather than synchronous — the endpoint answers `202` with a + * job id, and completion is pushed back over the websocket. So it does not share the `bulkFire` path the + * workflow quick actions use, but it reports through the same result toast. + * + * Unlike the others, eligibility does not depend on row state: reindexing applies to live, archived and + * locked content alike, because none of those affect whether the index copy is correct. + * + * Not gated client-side, though the endpoint requires CMS Power User or CMS Administrator — see + * {@link QUICK_ACTIONS}. */ export const REFRESH_ACTION_ID = 'REFRESH'; @@ -160,8 +171,8 @@ export const isLockedByAnotherUser = ( * Publish and Refresh. * * Lock/Unlock fire through the system-action endpoint; Add to Bundle posts to the legacy bundle - * servlet and collects a target first. Push Publish and Refresh are placeholders — see - * {@link DotActionCenterQuickAction.comingSoon}. + * servlet and collects a target first. Refresh is job-backed and reports completion by push. Push Publish is + * still a placeholder — see {@link DotActionCenterQuickAction.comingSoon}. */ const QUICK_ACTIONS: DotActionCenterQuickActionDef[] = [ { @@ -202,8 +213,15 @@ const QUICK_ACTIONS: DotActionCenterQuickActionDef[] = [ id: REFRESH_ACTION_ID, nameKey: 'Refresh', icon: 'refresh', - eligibleWhen: () => true, - comingSoon: true + // No row state disqualifies a reindex. Live, archived, locked — none of them change whether + // the index copy of a contentlet is stale, which is the only thing this fixes. + // + // Deliberately not role-gated here either, even though the endpoint requires CMS Power User or + // CMS Administrator. The client knows whether the user is an admin but has no idea whether they + // are a Power User, so the only gate available would hide Refresh from exactly the users the + // legacy button was written for. A visible action that answers 403 is a better failure than a + // capability silently withheld from people who have it. + eligibleWhen: () => true } ]; diff --git a/dotCMS/src/main/java/com/dotcms/api/system/event/SystemEventType.java b/dotCMS/src/main/java/com/dotcms/api/system/event/SystemEventType.java index cd520a41314f..a29ae44209b8 100644 --- a/dotCMS/src/main/java/com/dotcms/api/system/event/SystemEventType.java +++ b/dotCMS/src/main/java/com/dotcms/api/system/event/SystemEventType.java @@ -282,6 +282,15 @@ public enum SystemEventType { ANALYTICS_APP, /** A Contentlet has been updated by the AI Service */ - AI_CONTENT_PROMPT + AI_CONTENT_PROMPT, + + /** + * A bulk content reindex ({@code POST /api/v1/content/_bulkrefresh}) has finished. + *

+ * Carries the run's counters so the client can report the outcome without asking for it. Pushed + * with {@link Visibility#USER} scoped to whoever submitted the run, because a reindex is nobody + * else's business — unlike the legacy batch reindex, which told every CMS Administrator. + */ + BULK_REFRESH_COMPLETED } diff --git a/dotCMS/src/main/java/com/dotcms/jobs/business/processor/impl/BulkRefreshContentletsProcessor.java b/dotCMS/src/main/java/com/dotcms/jobs/business/processor/impl/BulkRefreshContentletsProcessor.java new file mode 100644 index 000000000000..ec6ee5ab1897 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/jobs/business/processor/impl/BulkRefreshContentletsProcessor.java @@ -0,0 +1,413 @@ +package com.dotcms.jobs.business.processor.impl; + +import com.dotcms.content.elasticsearch.business.ContentletIndexAPI; +import com.dotcms.exception.ExceptionUtil; +import com.dotcms.jobs.business.error.JobCancellationException; +import com.dotcms.jobs.business.error.JobProcessingException; +import com.dotcms.jobs.business.error.JobValidationException; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.jobs.business.processor.Cancellable; +import com.dotcms.jobs.business.processor.JobProcessor; +import com.dotcms.jobs.business.processor.NoRetryPolicy; +import com.dotcms.jobs.business.processor.ProgressTracker; +import com.dotcms.jobs.business.processor.Queue; +import com.dotcms.jobs.business.processor.Validator; +import com.dotcms.rest.api.v1.content.bulkrefresh.BulkRefreshItemResult; +import com.dotcms.rest.api.v1.content.bulkrefresh.BulkRefreshItemStatus; +import com.dotmarketing.beans.Identifier; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.CacheLocator; +import com.dotmarketing.business.IdentifierAPI; +import com.dotmarketing.business.UserAPI; +import com.dotmarketing.portlets.contentlet.business.ContentletAPI; +import com.dotmarketing.portlets.contentlet.business.ContentletCache; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.contentlet.model.IndexPolicy; +import com.dotmarketing.util.Logger; +import com.dotmarketing.util.UtilMethods; +import com.google.common.annotations.VisibleForTesting; +import com.liferay.portal.model.User; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import javax.enterprise.context.Dependent; + +/** + * Reindexes a selection of contentlets, one identifier at a time. + *

+ * Backs {@code POST /api/v1/content/_bulkrefresh}. For each identifier it clears the contentlet cache + * and writes every version to the index synchronously: the default {@link IndexPolicy#DEFER} + * only enqueues into {@code dist_reindex_journal} and returns, which is exactly why the single-item + * {@code _refresh} can answer {@code true} with nothing actually reindexed. Waiting for the write is + * what makes "done" mean the work was attempted rather than merely queued. + *

+ * Scope of that guarantee. It covers enqueue-versus-write, not the fate of each document: the + * underlying bulk call logs per-document failures without throwing, so a mapping error on one version + * can still be counted a success. A {@code SUCCESS} here means every identifier was submitted to the + * index and waited on, not that every document is certainly searchable. + *

+ * Reindexing is not retried — {@link NoRetryPolicy}. A failed item is a per-item record, and replaying + * a whole batch to re-attempt one identifier costs far more than it recovers. + * + * @author dotCMS + */ +@Dependent +@Queue("bulkRefreshContentlets") +@NoRetryPolicy +public class BulkRefreshContentletsProcessor implements JobProcessor, Validator, Cancellable { + + /** Job parameter: the submitted contentlet inodes. */ + public static final String PARAM_CONTENTLET_IDS = "contentletIds"; + + /** Job parameter: whether related content is reindexed alongside each item. */ + public static final String PARAM_INCLUDE_DEPENDENCIES = "includeDependencies"; + + /** Job parameter: whether per-item records are kept and reported. */ + public static final String PARAM_INCLUDE_ITEM_RESULTS = "includeItemResults"; + + /** Job parameter: the submitting user, so the run reindexes with their permissions. */ + public static final String PARAM_USER_ID = "userId"; + + private static final String SKIP_REASON = + "The reindex was cancelled before these items were attempted"; + + private final ContentletAPI contentletAPI; + private final IdentifierAPI identifierAPI; + private final ContentletIndexAPI contentletIndexAPI; + private final ContentletCache contentletCache; + private final UserAPI userAPI; + + private final AtomicBoolean cancellationRequested = new AtomicBoolean(false); + + /** + * Per-item records, in submission order, populated only when the job asked for them. + *

+ * Nothing in this feature asks: the client sends {@code includeItemResults: false}, so in practice + * this stays empty. Kept because {@link #getResultMetadata(Job)} persists it with the job, which + * makes it readable through the generic job-status endpoint if a drill-down is ever built. + */ + private final List itemResults = new CopyOnWriteArrayList<>(); + + private final AtomicInteger total = new AtomicInteger(); + private final AtomicInteger successCount = new AtomicInteger(); + private final AtomicInteger failedCount = new AtomicInteger(); + private final AtomicInteger skippedCount = new AtomicInteger(); + private final AtomicInteger versionsIndexed = new AtomicInteger(); + + /** + * Required by {@link com.dotcms.jobs.business.api.JobProcessorFactory}, which instantiates + * processors through their no-arg constructor. + */ + public BulkRefreshContentletsProcessor() { + this(APILocator.getContentletAPI(), APILocator.getIdentifierAPI(), + APILocator.getContentletIndexAPI(), CacheLocator.getContentletCache(), + APILocator.getUserAPI()); + } + + @VisibleForTesting + BulkRefreshContentletsProcessor(final ContentletAPI contentletAPI, + final IdentifierAPI identifierAPI, final ContentletIndexAPI contentletIndexAPI, + final ContentletCache contentletCache, final UserAPI userAPI) { + this.contentletAPI = contentletAPI; + this.identifierAPI = identifierAPI; + this.contentletIndexAPI = contentletIndexAPI; + this.contentletCache = contentletCache; + this.userAPI = userAPI; + } + + @Override + public void process(final Job job) throws JobProcessingException { + + final Map parameters = job.parameters(); + final boolean includeDependencies = flag(parameters, PARAM_INCLUDE_DEPENDENCIES); + final boolean recordItemResults = flag(parameters, PARAM_INCLUDE_ITEM_RESULTS); + final User user = user(parameters); + + final List workItems = resolve(contentletIds(parameters), user); + this.total.set(workItems.size()); + + final ProgressTracker progressTracker = job.progressTracker().orElseThrow( + () -> new JobProcessingException(job.id(), "Progress tracker not found")); + + Logger.info(this, String.format( + "Bulk refresh job [%s]: reindexing %d identifier(s) for user [%s], " + + "includeDependencies=%s", job.id(), workItems.size(), user.getUserId(), + includeDependencies)); + + for (final WorkItem workItem : workItems) { + + if (this.cancellationRequested.get()) { + skip(workItem, recordItemResults); + } else { + refresh(workItem, user, includeDependencies, recordItemResults); + } + + progressTracker.updateProgress(processed() / (float) workItems.size()); + } + + if (workItems.isEmpty()) { + // Unreachable through the endpoint (validate() rejects an empty selection), but dividing + // by zero above would report NaN progress rather than a finished job. + progressTracker.updateProgress(1.0f); + } + + Logger.info(this, String.format( + "Bulk refresh job [%s] finished: %d succeeded, %d failed, %d skipped, " + + "%d version(s) indexed", job.id(), this.successCount.get(), + this.failedCount.get(), this.skippedCount.get(), this.versionsIndexed.get())); + } + + /** + * Reindexes every version of one identifier. + *

+ * Any failure is caught here rather than propagated: a permission problem or a vanished row on one + * identifier is that item's outcome, and failing the job over it would discard the results of + * every item already reindexed. + */ + private void refresh(final WorkItem workItem, final User user, + final boolean includeDependencies, final boolean recordItemResults) { + + if (workItem.unresolved()) { + fail(workItem, workItem.resolutionError, recordItemResults); + return; + } + + int indexed = 0; + try { + final Identifier identifier = this.identifierAPI.find(workItem.identifier); + + // The contentlet cache is keyed by INODE, not identifier: ContentletCacheImpl stores under + // add(inode, contentlet) and remove(Contentlet) delegates to remove(getInode()). Removing + // by identifier evicts nothing at all — and it matters here more than almost anywhere, + // because findAllVersions reads back *through* that cache + // (ESContentFactoryImpl.findContentlets serves hits straight from it). So an identifier- + // keyed eviction leaves a stale cached version to be written to the index, which is the + // precise failure this endpoint exists to repair. + // + // Hence: evict the inodes we know, read the versions to learn the rest, evict those too, + // then read again. The second read is the one whose result gets indexed, and it is cold, + // so what reaches the index came from the database rather than from the cache. + workItem.inodes.forEach(this.contentletCache::remove); + this.contentletAPI.findAllVersions(identifier, false, user, false) + .forEach(this.contentletCache::remove); + + final List versions = + this.contentletAPI.findAllVersions(identifier, false, user, false); + + for (final Contentlet version : versions) { + // Without this the write is only enqueued, and "done" would be a lie. + version.setIndexPolicy(IndexPolicy.WAIT_FOR); + this.contentletIndexAPI.addContentToIndex(version, includeDependencies); + // Counted per write rather than after the loop, so a failure partway still reports + // the versions that did land instead of silently discarding them. + indexed++; + this.versionsIndexed.incrementAndGet(); + } + + this.successCount.incrementAndGet(); + + if (recordItemResults) { + this.itemResults.add(BulkRefreshItemResult.builder() + .identifier(workItem.identifier) + .inodes(workItem.inodes) + .status(BulkRefreshItemStatus.SUCCESS) + .versionsIndexed(indexed) + .build()); + } + } catch (final Exception e) { + Logger.warn(this, String.format("Unable to reindex identifier [%s]: %s", + workItem.identifier, e.getMessage()), e); + fail(workItem, message(e), recordItemResults); + } + } + + private void fail(final WorkItem workItem, final String errorMessage, + final boolean recordItemResults) { + + this.failedCount.incrementAndGet(); + if (recordItemResults) { + this.itemResults.add(BulkRefreshItemResult.builder() + .identifier(Optional.ofNullable(workItem.identifier)) + .inodes(workItem.inodes) + .status(BulkRefreshItemStatus.FAILED) + .errorMessage(errorMessage) + .build()); + } + } + + private void skip(final WorkItem workItem, final boolean recordItemResults) { + + this.skippedCount.incrementAndGet(); + if (recordItemResults) { + this.itemResults.add(BulkRefreshItemResult.builder() + .identifier(Optional.ofNullable(workItem.identifier)) + .inodes(workItem.inodes) + .status(BulkRefreshItemStatus.SKIPPED) + .build()); + } + } + + /** + * Turns the submitted inodes into the units of work, collapsing several inodes of the same + * identifier into one. + *

+ * "Missing from search" is rarely confined to one language, so reindexing is done per identifier + * across all its versions — three language rows of the same content are one reindex, not three. + * Submission order is preserved so a client sees its rows settle roughly in the order it sent them. + * An inode that no longer resolves becomes its own item and is reported as a failure, because a + * selection can go stale between the click and the submit and that must not cost the caller the + * rest of the batch. + */ + private List resolve(final List inodes, final User user) { + + final Map> inodesByIdentifier = new LinkedHashMap<>(); + final List workItems = new ArrayList<>(); + + for (final String inode : inodes) { + + String identifier = null; + String error = null; + try { + final Contentlet contentlet = this.contentletAPI.find(inode, user, false); + if (null == contentlet || !UtilMethods.isSet(contentlet.getIdentifier())) { + error = String.format("No contentlet found for inode %s", inode); + } else { + identifier = contentlet.getIdentifier(); + } + } catch (final Exception e) { + error = String.format("Unable to resolve inode %s: %s", inode, message(e)); + } + + if (null == identifier) { + workItems.add(new WorkItem(null, List.of(inode), error)); + continue; + } + + final List existing = inodesByIdentifier.get(identifier); + if (null != existing) { + // Same content, another language or version — one reindex covers all of them, but the + // client still needs every inode named back so it can settle each selected row. + existing.add(inode); + } else { + final List collected = new ArrayList<>(); + collected.add(inode); + inodesByIdentifier.put(identifier, collected); + workItems.add(new WorkItem(identifier, collected, null)); + } + } + + return workItems; + } + + @Override + public void validate(final Map parameters) throws JobValidationException { + + final Object contentletIds = null == parameters ? null : parameters.get(PARAM_CONTENTLET_IDS); + if (!(contentletIds instanceof Collection) || ((Collection) contentletIds).isEmpty()) { + final String errorMessage = "A non-empty list of contentlet inodes is required"; + Logger.error(this.getClass(), errorMessage); + throw new JobValidationException(errorMessage); + } + } + + @Override + public void cancel(final Job job) throws JobCancellationException { + + Logger.info(this.getClass(), "Bulk refresh cancellation requested: " + job.id()); + this.cancellationRequested.set(true); + } + + @Override + public Map getResultMetadata(final Job job) { + + final Map metadata = new HashMap<>(); + metadata.put("total", this.total.get()); + metadata.put("processed", processed()); + metadata.put("successCount", this.successCount.get()); + metadata.put("failedCount", this.failedCount.get()); + metadata.put("skippedCount", this.skippedCount.get()); + metadata.put("versionsIndexed", this.versionsIndexed.get()); + metadata.put("includeDependencies", flag(job.parameters(), PARAM_INCLUDE_DEPENDENCIES)); + + if (this.skippedCount.get() > 0) { + metadata.put("skipReason", SKIP_REASON); + } + + // Only when asked for: this map is persisted with the job, so a 500-entry array nobody + // requested is storage spent on nothing. When it is requested this is the only place the + // records survive, so it has to be complete. + if (flag(job.parameters(), PARAM_INCLUDE_ITEM_RESULTS)) { + metadata.put("results", List.copyOf(this.itemResults)); + } + + return metadata; + } + + private int processed() { + return this.successCount.get() + this.failedCount.get() + this.skippedCount.get(); + } + + @SuppressWarnings("unchecked") + private static List contentletIds(final Map parameters) { + final Object contentletIds = parameters.get(PARAM_CONTENTLET_IDS); + return contentletIds instanceof Collection + ? List.copyOf((Collection) contentletIds) + : List.of(); + } + + private static boolean flag(final Map parameters, final String name) { + final Object value = null == parameters ? null : parameters.get(name); + return value instanceof Boolean + ? (Boolean) value + : Boolean.parseBoolean(String.valueOf(value)); + } + + private User user(final Map parameters) { + final String userId = String.valueOf(parameters.get(PARAM_USER_ID)); + try { + return this.userAPI.loadUserById(userId); + } catch (final Exception e) { + throw new JobProcessingException("Unable to load the submitting user " + userId, e); + } + } + + /** + * The root cause's message, localized the same way {@code ActionFail} does, so a per-item failure + * names the actual problem rather than whatever wrapper it arrived in. + */ + private static String message(final Exception e) { + final Throwable rootCause = ExceptionUtil.getRootCause(e); + return UtilMethods.isSet(rootCause.getMessage()) + ? rootCause.getMessage() + : rootCause.toString(); + } + + /** + * One unit of work: an identifier and every submitted inode that resolved to it, or — when nothing + * resolved — the lone inode and why it failed. + */ + private static final class WorkItem { + + private final String identifier; + private final List inodes; + private final String resolutionError; + + WorkItem(final String identifier, final List inodes, + final String resolutionError) { + this.identifier = identifier; + this.inodes = inodes; + this.resolutionError = resolutionError; + } + + boolean unresolved() { + return null == this.identifier; + } + } +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/ResponseEntityBulkRefreshSubmitView.java b/dotCMS/src/main/java/com/dotcms/rest/ResponseEntityBulkRefreshSubmitView.java new file mode 100644 index 000000000000..f724f6d79e06 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/ResponseEntityBulkRefreshSubmitView.java @@ -0,0 +1,13 @@ +package com.dotcms.rest; + +import com.dotcms.rest.api.v1.content.bulkrefresh.BulkRefreshSubmitResponse; + +/** + * This class encapsulates the {@link javax.ws.rs.core.Response} object to include the expected + * {@link BulkRefreshSubmitResponse} as the entity in the response. + */ +public class ResponseEntityBulkRefreshSubmitView extends ResponseEntityView { + public ResponseEntityBulkRefreshSubmitView(final BulkRefreshSubmitResponse entity) { + super(entity); + } +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/AbstractBulkRefreshItemResult.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/AbstractBulkRefreshItemResult.java new file mode 100644 index 000000000000..6842c85c79f1 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/AbstractBulkRefreshItemResult.java @@ -0,0 +1,55 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import java.util.List; +import java.util.Optional; +import org.immutables.value.Value; + +/** + * The outcome of reindexing one contentlet identifier during a bulk refresh run. + *

+ * Results are reported per identifier, not per submitted inode: several language rows of the + * same content collapse into a single record whose {@link #inodes()} lists every inode the caller + * submitted for it — which is what a client would need to mark the right grid rows, if one ever consumed + * these records. None does today; see {@code BulkRefreshContentletsProcessor}'s note on why they are + * still produced. + * + * @author dotCMS + */ +@Value.Style(typeImmutable = "*", typeAbstract = "Abstract*") +@Value.Immutable +@JsonSerialize(as = BulkRefreshItemResult.class) +@JsonDeserialize(as = BulkRefreshItemResult.class) +public interface AbstractBulkRefreshItemResult { + + /** + * The resolved contentlet identifier, or empty when the submitted inode could not be resolved + * (a row that went stale between selection and submit). + */ + Optional identifier(); + + /** + * The submitted inodes that resolved to this identifier. Never empty — a record exists only + * because the caller asked about at least one inode. + */ + List inodes(); + + /** Whether this identifier was reindexed, failed, or was never attempted. */ + BulkRefreshItemStatus status(); + + /** + * Present on {@link BulkRefreshItemStatus#FAILED} only. Root cause unwrapped, so the message + * names the actual problem instead of a wrapper exception. + */ + Optional errorMessage(); + + /** + * How many versions were written to the index for this identifier. Lets a UI report "12 + * selected, 31 versions reindexed" honestly rather than implying one write per selected row. + */ + @Value.Default + default int versionsIndexed() { + return 0; + } +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/AbstractBulkRefreshSubmitResponse.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/AbstractBulkRefreshSubmitResponse.java new file mode 100644 index 000000000000..f2883b00302e --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/AbstractBulkRefreshSubmitResponse.java @@ -0,0 +1,31 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import org.immutables.value.Value; + +/** + * The {@code 202 Accepted} body returned when a bulk refresh is submitted. + *

+ * Deliberately not {@link com.dotcms.rest.api.v1.job.JobStatusResponse}: that carries a status URL to + * poll, and this endpoint has none — completion is pushed over the websocket as a + * {@code BULK_REFRESH_COMPLETED} system event. {@link #submitted()} lets a caller tell how many inodes + * it sent apart from the de-duplicated {@code total} the completion event reports. + * + * @author dotCMS + */ +@Value.Style(typeImmutable = "*", typeAbstract = "Abstract*") +@Value.Immutable +@JsonSerialize(as = BulkRefreshSubmitResponse.class) +@JsonDeserialize(as = BulkRefreshSubmitResponse.class) +public interface AbstractBulkRefreshSubmitResponse { + + /** The job's id — the handle for the cancel call. */ + String jobId(); + + /** + * The raw count of inodes accepted, before identifier de-duplication. The de-duplicated + * {@code total} is reported by the job result, and is often smaller. + */ + int submitted(); +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshCompletionListener.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshCompletionListener.java new file mode 100644 index 000000000000..de9c51e13186 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshCompletionListener.java @@ -0,0 +1,204 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +import com.dotcms.api.system.event.Payload; +import com.dotcms.api.system.event.SystemEventsAPI; +import com.dotcms.api.system.event.SystemEventType; +import com.dotcms.api.system.event.Visibility; +import com.dotcms.jobs.business.api.events.JobCompletedEvent; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.jobs.business.job.JobResult; +import com.dotcms.jobs.business.job.JobState; +import com.dotcms.jobs.business.processor.impl.BulkRefreshContentletsProcessor; +import com.dotcms.notifications.bean.NotificationLevel; +import com.dotcms.notifications.business.NotificationAPI; +import com.dotcms.notifications.bean.NotificationType; +import com.dotcms.system.event.local.model.EventSubscriber; +import com.dotcms.util.I18NMessage; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.UserAPI; +import com.dotmarketing.util.Logger; +import com.dotmarketing.util.UtilMethods; +import com.google.common.annotations.VisibleForTesting; +import io.vavr.control.Try; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +/** + * Tells the user who submitted a bulk refresh that it finished. + * + *

The endpoint answers {@code 202} long before the work is done, so something has to close the loop. + * This listener does it by push rather than by the client asking: on a terminal job it emits a + * {@link SystemEventType#BULK_REFRESH_COMPLETED} system event carrying the run's counters, which reaches + * the browser over the websocket the admin UI already holds open, and it records a notification so the + * outcome survives navigating away or closing the tab. + * + *

Why {@link JobCompletedEvent} and not the processor itself. That event fires for every + * terminal state — success, cancellation, and permanent failure + * ({@code JobQueueManagerAPIImpl} marks the last of these completed too) — so the failure path needs no + * special handling in the processor. {@code JobFailedEvent} is deliberately not used: it signals a + * retryable failure, where the job goes back on the queue and is not finished at all. + * + *

Why we push our own event. Job events never reach a browser: they travel as + * {@code CLUSTER_WIDE_EVENT}, which is explicitly excluded from the websocket. Subscribing locally and + * pushing a purpose-built event also keeps the payload to plain counters, which crosses nodes safely, + * rather than depending on the job event classes deserializing on another node. + * + *

Registered at startup by {@code LocalSystemEventSubscribersInitializer}, deliberately not as + * a CDI bean subscribing to itself in {@code @PostConstruct}. CDI beans are lazy: nothing injects this + * class, so it would never have been constructed, the subscription would never have happened, and a + * finished reindex would simply never have been reported — and every test stayed green, because the unit + * tests construct it directly. {@code test_bulkRefresh_completionNotifiesTheSubmitter} was added to close + * that gap: it requires the submitter's notification count to rise, which only happens if this listener + * really was registered and really did fire. + * + * @author dotCMS + */ +public class BulkRefreshCompletionListener implements EventSubscriber { + + static final String EVENT_TOTAL = "total"; + static final String EVENT_SUCCESS_COUNT = "successCount"; + static final String EVENT_FAILED_COUNT = "failedCount"; + static final String EVENT_SKIPPED_COUNT = "skippedCount"; + static final String EVENT_VERSIONS_INDEXED = "versionsIndexed"; + static final String EVENT_STATE = "state"; + + private static final String NOTIFICATION_TITLE_KEY = "notification.bulkrefresh.title"; + private static final String NOTIFICATION_SUCCESS_KEY = "notification.bulkrefresh.success"; + private static final String NOTIFICATION_PARTIAL_KEY = "notification.bulkrefresh.partial"; + private static final String NOTIFICATION_FAILED_KEY = "notification.bulkrefresh.failed"; + + private final SystemEventsAPI systemEventsAPI; + private final NotificationAPI notificationAPI; + private final UserAPI userAPI; + + public BulkRefreshCompletionListener() { + this(APILocator.getSystemEventsAPI(), APILocator.getNotificationAPI(), + APILocator.getUserAPI()); + } + + @VisibleForTesting + BulkRefreshCompletionListener(final SystemEventsAPI systemEventsAPI, + final NotificationAPI notificationAPI, final UserAPI userAPI) { + this.systemEventsAPI = systemEventsAPI; + this.notificationAPI = notificationAPI; + this.userAPI = userAPI; + } + + /** + * Reports a finished bulk refresh, and ignores every other queue's jobs. + */ + @Override + public void notify(final JobCompletedEvent event) { + + final Job job = null == event ? null : event.getJob(); + if (null == job + || !BulkRefreshHelper.BULK_REFRESH_QUEUE_NAME.equals(job.queueName())) { + return; + } + + final String userId = submitter(job); + if (!UtilMethods.isSet(userId)) { + // Without a submitter there is nobody to tell. Worth a line in the log rather than a + // silent return, because it means the job was created without its user parameter. + Logger.warn(this, String.format( + "Bulk refresh job [%s] finished with no submitting user recorded; " + + "no notification sent", job.id())); + return; + } + + final Map counters = counters(job); + + // Both channels are best-effort: a failed notification must not bring down the job queue's + // event dispatch, and the run itself has already succeeded by this point. + Try.run(() -> this.systemEventsAPI.pushAsync( + SystemEventType.BULK_REFRESH_COMPLETED, + new Payload(counters, Visibility.USER, userId))) + .onFailure(e -> Logger.error(this, String.format( + "Unable to push the bulk refresh completion event for job [%s]", job.id()), e)); + + Try.run(() -> notify(job, counters, userId)) + .onFailure(e -> Logger.error(this, String.format( + "Unable to record the bulk refresh notification for job [%s]", job.id()), e)); + } + + /** + * Records the durable notification, worded on what actually happened. + *

+ * Addressed to the submitter rather than to the CMS Administrator role, and gated on the outcome — + * the legacy batch reindex did neither, announcing success to every administrator even when every + * single item had failed. + */ + private void notify(final Job job, final Map counters, final String userId) + throws Exception { + + final int failed = intValue(counters, EVENT_FAILED_COUNT); + final int skipped = intValue(counters, EVENT_SKIPPED_COUNT); + final int succeeded = intValue(counters, EVENT_SUCCESS_COUNT); + + final String messageKey; + final NotificationLevel level; + if (JobState.SUCCESS != job.state() || (0 == succeeded && failed > 0)) { + messageKey = NOTIFICATION_FAILED_KEY; + level = NotificationLevel.ERROR; + } else if (failed > 0 || skipped > 0) { + messageKey = NOTIFICATION_PARTIAL_KEY; + level = NotificationLevel.WARNING; + } else { + messageKey = NOTIFICATION_SUCCESS_KEY; + level = NotificationLevel.INFO; + } + + // The I18NMessage overload so the counts travel as arguments and the text is resolved in the + // recipient's own locale. Legacy localized with the *system* user's locale, which meant the + // message could arrive in a language the reader does not use. + this.notificationAPI.generateNotification( + new I18NMessage(NOTIFICATION_TITLE_KEY), + new I18NMessage(messageKey, null, succeeded, failed, skipped), + null, + level, + NotificationType.GENERIC, + Visibility.USER, + userId, + userId, + this.userAPI.loadUserById(userId).getLocale() + ); + } + + /** + * The run's counters, taken from the persisted job result. + *

+ * Empty when the job carried none — a client is expected to treat that as a failure rather than as a + * clean run over nothing, which is what all-zero counters would look like. + */ + private Map counters(final Job job) { + + final Optional> metadata = + job.result().flatMap(JobResult::metadata); + + final Map counters = new HashMap<>(); + counters.put(EVENT_STATE, job.state()); + metadata.ifPresent(found -> { + counters.put(EVENT_TOTAL, found.get(EVENT_TOTAL)); + counters.put(EVENT_SUCCESS_COUNT, found.get(EVENT_SUCCESS_COUNT)); + counters.put(EVENT_FAILED_COUNT, found.get(EVENT_FAILED_COUNT)); + counters.put(EVENT_SKIPPED_COUNT, found.get(EVENT_SKIPPED_COUNT)); + counters.put(EVENT_VERSIONS_INDEXED, found.get(EVENT_VERSIONS_INDEXED)); + }); + + return counters; + } + + private static String submitter(final Job job) { + final Object userId = job.parameters() + .get(BulkRefreshContentletsProcessor.PARAM_USER_ID); + + return null == userId ? null : String.valueOf(userId); + } + + private static int intValue(final Map counters, final String key) { + final Object value = counters.get(key); + + return value instanceof Number ? ((Number) value).intValue() : 0; + } +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshForm.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshForm.java new file mode 100644 index 000000000000..dd838a4be6a8 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshForm.java @@ -0,0 +1,77 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +import com.dotcms.rest.api.Validated; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.List; +import javax.validation.constraints.NotNull; +import javax.validation.constraints.Size; + +/** + * JSON body of {@code POST /api/v1/content/_bulkrefresh}. + *

+ * {@code contentletIds} carries contentlet inodes — the name matches {@code + * FireBulkActionsForm} and what the Action Center already sends. No Lucene query is accepted: a + * query resolves to an unbounded set, and combined with synchronous indexing that is a self-inflicted + * full reindex. + *

+ * Duplicates are allowed and collapsed by identifier server-side. Inodes that no longer resolve are + * not a request error — they count toward {@code failedCount} rather than rejecting the batch, + * because a selection can go stale between the click and the submit. + * + * @author dotCMS + */ +public class BulkRefreshForm extends Validated { + + @NotNull(message = "A non-empty list of contentlet inodes is required") + @Size(min = 1, message = "A non-empty list of contentlet inodes is required") + private final List contentletIds; + + private final boolean includeDependencies; + + private final boolean includeItemResults; + + @JsonCreator + public BulkRefreshForm( + @JsonProperty("contentletIds") final List contentletIds, + @JsonProperty("includeDependencies") final Boolean includeDependencies, + @JsonProperty("includeItemResults") final Boolean includeItemResults) { + super(); + this.contentletIds = contentletIds; + this.includeDependencies = Boolean.TRUE.equals(includeDependencies); + this.includeItemResults = Boolean.TRUE.equals(includeItemResults); + this.checkValid(); + } + + /** Contentlet inodes to reindex. Capped by {@code CONTENT_BULK_REFRESH_MAX_ITEMS}. */ + public List getContentletIds() { + return contentletIds; + } + + /** + * Whether related content is reindexed alongside each item. Defaults to {@code false}, which + * diverges from the single-item {@code _refresh}: that one always includes dependencies, + * but at batch size a {@code loadDeps()} fan-out per item is a different cost profile. + */ + public boolean isIncludeDependencies() { + return includeDependencies; + } + + /** + * Whether per-item records are recorded and reported. Counters are returned either way; only a + * drill-down needs the breakdown. Set once at submit and irreversible for that job, since the + * processor either keeps the records or does not. + */ + public boolean isIncludeItemResults() { + return includeItemResults; + } + + @Override + public String toString() { + return "BulkRefreshForm{" + + "contentletIds=" + (null == contentletIds ? 0 : contentletIds.size()) + " item(s)" + + ", includeDependencies=" + includeDependencies + + ", includeItemResults=" + includeItemResults + + '}'; + } +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshHelper.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshHelper.java new file mode 100644 index 000000000000..b06c869e69d5 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshHelper.java @@ -0,0 +1,136 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +import com.dotcms.jobs.business.api.JobQueueManagerAPI; +import com.dotcms.jobs.business.processor.impl.BulkRefreshContentletsProcessor; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.Role; +import com.dotmarketing.business.RoleAPI; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.util.Config; +import com.dotmarketing.util.Logger; +import com.liferay.portal.model.User; +import java.util.HashMap; +import java.util.Map; +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; + +/** + * Job creation, authorization and validation for bulk refresh. + *

+ * Kept out of {@link BulkRefreshResource} so the resource stays a thin HTTP layer, following the + * {@code ContentImportResource} / {@code ContentImportHelper} split. + * + * @author dotCMS + */ +@ApplicationScoped +public class BulkRefreshHelper { + + /** Queue name; must match {@code @Queue} on the processor. */ + public static final String BULK_REFRESH_QUEUE_NAME = "bulkRefreshContentlets"; + + /** Ceiling on inodes per submission. Synchronous indexing makes an unbounded batch expensive. */ + public static final String MAX_ITEMS_CONFIG_PROPERTY = "CONTENT_BULK_REFRESH_MAX_ITEMS"; + + public static final int MAX_ITEMS_DEFAULT = 500; + + + private final JobQueueManagerAPI jobQueueManagerAPI; + + /** + * Required for CDI proxying of this normal-scoped bean, and identical to the one on + * {@code ContentImportHelper} (:93) that this class follows. + *

+ * It does hand out an instance whose API reference is null, which would NPE if anything called it + * directly — nothing does, and Weld builds client proxies without invoking a constructor. Kept + * rather than removed because dropping it risks an unproxyable-bean deployment failure for a + * cosmetic gain. + */ + public BulkRefreshHelper() { + this.jobQueueManagerAPI = null; + } + + @Inject + public BulkRefreshHelper(final JobQueueManagerAPI jobQueueManagerAPI) { + this.jobQueueManagerAPI = jobQueueManagerAPI; + } + + /** + * Authorizes the user, validates the form and enqueues the job. + * + * @param form the submitted selection and flags + * @param user the submitting backend user + * @return the accepted job's handle. Completion is pushed, not fetched. + * @throws DotSecurityException if the user is neither a CMS Power User nor a CMS Administrator + * @throws IllegalArgumentException if the selection is empty or over the configured cap + */ + public BulkRefreshSubmitResponse submit(final BulkRefreshForm form, final User user) + throws DotDataException, DotSecurityException { + + if (!canRefresh(user)) { + throw new DotSecurityException(String.format( + "User [%s] must be a CMS Power User or a CMS Administrator to reindex content", + user.getUserId())); + } + + final int submitted = form.getContentletIds().size(); + final int maxItems = maxItems(); + if (submitted > maxItems) { + throw new IllegalArgumentException(String.format( + "A bulk refresh accepts at most %d items; %d were submitted", + maxItems, submitted)); + } + + final Map jobParameters = new HashMap<>(); + jobParameters.put(BulkRefreshContentletsProcessor.PARAM_CONTENTLET_IDS, + form.getContentletIds()); + jobParameters.put(BulkRefreshContentletsProcessor.PARAM_INCLUDE_DEPENDENCIES, + form.isIncludeDependencies()); + jobParameters.put(BulkRefreshContentletsProcessor.PARAM_INCLUDE_ITEM_RESULTS, + form.isIncludeItemResults()); + jobParameters.put(BulkRefreshContentletsProcessor.PARAM_USER_ID, user.getUserId()); + + final String jobId = this.jobQueueManagerAPI.createJob( + BULK_REFRESH_QUEUE_NAME, jobParameters); + + Logger.info(this, String.format( + "Bulk refresh job [%s] created by user [%s] for %d inode(s)", + jobId, user.getUserId(), submitted)); + + return BulkRefreshSubmitResponse.builder() + .jobId(jobId) + .submitted(submitted) + .build(); + } + + /** + * Whether the user may reindex content. + *

+ * Matches the legacy gate in {@code view_contentlets.jsp} so nobody who could press the old + * Refresh button loses access, and nobody who could not gains it. "Always available" in the + * ticket means not gated by content state — that still holds; this is a role gate, and + * reindexing is expensive enough to want one. + *

+ * This gates submission only. It does not protect a submitted job's contents: the + * generic {@code GET /api/v1/jobs/{jobId}/status} requires only a backend user and returns the + * whole job, parameters included — a pre-existing exposure this feature neither creates nor + * closes. + * + * @param user the user to check + * @return true for a CMS Power User or a CMS Administrator + */ + public boolean canRefresh(final User user) throws DotDataException { + + final RoleAPI roleAPI = APILocator.getRoleAPI(); + return roleAPI.doesUserHaveRole(user, roleAPI.loadRoleByKey(Role.CMS_POWER_USER)) + || roleAPI.doesUserHaveRole(user, roleAPI.loadCMSAdminRole()); + } + + /** + * The effective per-submission cap. + */ + public int maxItems() { + return Config.getIntProperty(MAX_ITEMS_CONFIG_PROPERTY, MAX_ITEMS_DEFAULT); + } + +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshItemStatus.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshItemStatus.java new file mode 100644 index 000000000000..dde3611adcff --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshItemStatus.java @@ -0,0 +1,24 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +/** + * Outcome of a single item in a bulk refresh (reindex) run. + *

+ * One status-discriminated enum rather than separate success / failure / skip collections, so every + * outcome is described uniformly and a fourth outcome later needs no new field. + * + * @author dotCMS + */ +public enum BulkRefreshItemStatus { + + /** The identifier's versions were reindexed. */ + SUCCESS, + + /** The item could not be reindexed. {@code errorMessage} carries the reason. */ + FAILED, + + /** + * The item was never attempted — the run was cancelled before reaching it. De-duplication of + * several inodes onto one identifier is not a skip. + */ + SKIPPED +} diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshResource.java new file mode 100644 index 000000000000..7a274c465361 --- /dev/null +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshResource.java @@ -0,0 +1,118 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +import com.dotcms.rest.InitDataObject; +import com.dotcms.rest.ResponseEntityBulkRefreshSubmitView; +import com.dotcms.rest.WebResource; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.util.Logger; +import com.liferay.portal.model.User; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.tags.Tag; +import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.Consumes; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Context; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +/** + * Reindexes a selection of contentlets — the bulk counterpart of + * {@code PUT /api/v1/content/_refresh/{identifierOrInode}}, which is unchanged. + *

+ * This is not a full index rebuild. {@code POST /api/v1/esindex/reindex} rebuilds the entire + * index and is a different operation; nothing here touches it. + *

+ * Job-backed and push-reported: the {@code POST} returns a {@code jobId} and the work continues in the + * background. Completion is announced over the websocket the admin UI already holds open, as a + * {@code BULK_REFRESH_COMPLETED} system event carrying the run's counters, plus a notification so the + * outcome survives the user navigating away. There is deliberately no status endpoint to poll — a client + * asking every second or so for five minutes was the cost this replaced. + *

+ * Reindexing is not modelled as a workflow action or a {@code SystemAction}: bulk fire resolves + * its target set by searching the index, which is circular for an operation whose whole purpose is to + * fix the index — content missing from the index cannot be found by an index search, so the items that + * most need this would be exactly the ones such a path could never reach. + * + * @author dotCMS + */ +@Path("/v1/content/_bulkrefresh") +@Tag(name = "Content", description = "Bulk reindex of selected contentlets") +public class BulkRefreshResource { + + private final WebResource webResource; + private final BulkRefreshHelper bulkRefreshHelper; + + @Inject + public BulkRefreshResource(final BulkRefreshHelper bulkRefreshHelper) { + this(new WebResource(), bulkRefreshHelper); + } + + public BulkRefreshResource(final WebResource webResource, + final BulkRefreshHelper bulkRefreshHelper) { + this.webResource = webResource; + this.bulkRefreshHelper = bulkRefreshHelper; + } + + /** + * Submits a selection of contentlets to be reindexed. + */ + @POST + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + @Operation(operationId = "bulkRefreshContent", summary = "Reindex a selection of contentlets", + description = "Clears the contentlet cache and reindexes every selected contentlet, all " + + "versions of each. Returns a job id. This is accepted work, not finished work: " + + "completion is pushed to the submitting user over the websocket. Not a full " + + "index rebuild.", + tags = {"Content"}, + responses = { + @ApiResponse(responseCode = "202", description = "Accepted - reindex job enqueued", + content = @Content(mediaType = "application/json", + schema = @Schema(implementation = ResponseEntityBulkRefreshSubmitView.class))), + @ApiResponse(responseCode = "400", description = "Bad request - empty selection or over the configured item cap"), + @ApiResponse(responseCode = "401", description = "Unauthorized - no backend user session"), + @ApiResponse(responseCode = "403", description = "Forbidden - not a CMS Power User or CMS Administrator"), + @ApiResponse(responseCode = "415", description = "Unsupported Media Type"), + @ApiResponse(responseCode = "500", description = "Internal Server Error - the job could not be created") + }) + public Response bulkRefresh(@Context final HttpServletRequest request, + @Context final HttpServletResponse response, + final BulkRefreshForm form) throws DotDataException, DotSecurityException { + + final User user = init(request, response).getUser(); + + Logger.debug(this, () -> String.format("User %s is submitting %d inode(s) to be reindexed", + user.getUserId(), form.getContentletIds().size())); + + final BulkRefreshSubmitResponse submitted = + this.bulkRefreshHelper.submit(form, user); + + // 202, not 200: the work is accepted, not done. A client must not be able to read this as + // "reindexed" - telling the user otherwise is exactly the misleading success this endpoint + // exists to avoid. + return Response.status(Response.Status.ACCEPTED) + .entity(new ResponseEntityBulkRefreshSubmitView(submitted)) + .build(); + } + + /** + * Requires a backend user; anonymous callers are rejected before any content is touched. + */ + private InitDataObject init(final HttpServletRequest request, + final HttpServletResponse response) { + return new WebResource.InitBuilder(this.webResource) + .requiredBackendUser(true) + .requiredFrontendUser(false) + .requestAndResponse(request, response) + .rejectWhenNoUser(true) + .init(); + } +} diff --git a/dotCMS/src/main/java/com/dotcms/system/event/local/business/LocalSystemEventSubscribersInitializer.java b/dotCMS/src/main/java/com/dotcms/system/event/local/business/LocalSystemEventSubscribersInitializer.java index 4731d5f04b30..08aa116dd1d7 100644 --- a/dotCMS/src/main/java/com/dotcms/system/event/local/business/LocalSystemEventSubscribersInitializer.java +++ b/dotCMS/src/main/java/com/dotcms/system/event/local/business/LocalSystemEventSubscribersInitializer.java @@ -6,8 +6,10 @@ import com.dotcms.config.DotInitializer; import com.dotcms.content.elasticsearch.business.event.ContentletCheckinEvent; import com.dotcms.graphql.listener.ContentTypeAndFieldsModsListeners; +import com.dotcms.jobs.business.api.events.JobCompletedEvent; import com.dotcms.publishing.listener.PushPublishKeyResetEventListener; import com.dotcms.rendering.velocity.services.MacroCacheRefresherJob; +import com.dotcms.rest.api.v1.content.bulkrefresh.BulkRefreshCompletionListener; import com.dotcms.rest.api.v1.system.logger.ChangeLoggerLevelEvent; import com.dotcms.security.apps.AppSecretSavedEvent; import com.dotcms.security.apps.AppsKeyResetEventListener; @@ -74,6 +76,12 @@ public void notify(final ChangeLoggerLevelEvent event) { APILocator.getLocalSystemEventsAPI().subscribe(AppSecretSavedEvent.class, AIAppListener.Instance.get()); APILocator.getLocalSystemEventsAPI().subscribe(AppSecretSavedEvent.class, ContentAnalyticsAppListener.Instance.get()); + // Tells whoever submitted a bulk content reindex that it finished. Registered here rather than + // self-subscribing from a CDI @PostConstruct: nothing injects that class, and CDI beans are lazy, + // so it would never have been constructed and completion would never have been reported. + APILocator.getLocalSystemEventsAPI().subscribe(JobCompletedEvent.class, + new BulkRefreshCompletionListener()); + this.initDotVelocityMacrosVtlFiles(); } diff --git a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties index 5f4e6c2cf941..e7f7487c9787 100644 --- a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties +++ b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties @@ -7238,6 +7238,13 @@ content-drive.action-center.busy=Wait for the running action to finish before st content-drive.action-center.toast.executed=Action executed content-drive.action-center.toast.executed-detail={0} ran on {1} item(s). content-drive.action-center.toast.executed-partial={0}: {1} completed, {2} failed (you may not have permission, or the content is locked by another user), {3} skipped (this action is not on their workflow step). +content-drive.action-center.toast.refreshed-partial={0}: {1} reindexed, {2} failed (the content could not be read or written to the index), {3} skipped (the reindex was cancelled before reaching them). +content-drive.action-center.toast.reindex-started=Reindex started +content-drive.action-center.toast.reindex-started-detail={0} is reindexing {1} item(s) in the background. You can keep working — we'll let you know when it finishes. +notification.bulkrefresh.title=Reindex Finished +notification.bulkrefresh.success={0} item(s) reindexed. +notification.bulkrefresh.partial={0} item(s) reindexed, {1} failed, {2} skipped. +notification.bulkrefresh.failed=The reindex did not complete: {0} item(s) reindexed, {1} failed, {2} skipped. content-drive.action-center.unlock.locked-by-others={0} of these are locked by another user, which may require administrator permission to unlock. Any that can't be unlocked will be reported. content-drive.list-view.locked-by-another-user=Locked by another user content-drive.list-view.shared-asset=Shared across all sites diff --git a/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml b/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml index 0f409e597c54..3cb4e9fbafa2 100644 --- a/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml +++ b/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml @@ -6150,6 +6150,38 @@ paths: summary: Copies a container to the current host tags: - Containers + /v1/content/_bulkrefresh: + post: + description: "Clears the contentlet cache and reindexes every selected contentlet,\ + \ all versions of each. Returns a job id. This is accepted work, not finished\ + \ work: completion is pushed to the submitting user over the websocket. Not\ + \ a full index rebuild." + operationId: bulkRefreshContent + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/BulkRefreshForm" + responses: + "202": + content: + application/json: + schema: + $ref: "#/components/schemas/ResponseEntityBulkRefreshSubmitView" + description: Accepted - reindex job enqueued + "400": + description: Bad request - empty selection or over the configured item cap + "401": + description: Unauthorized - no backend user session + "403": + description: Forbidden - not a CMS Power User or CMS Administrator + "415": + description: Unsupported Media Type + "500": + description: Internal Server Error - the job could not be created + summary: Reindex a selection of contentlets + tags: + - Content /v1/content/_canlock/{inodeOrIdentifier}: get: description: Checks if the contentlet specified by its inode or identifier can @@ -25034,6 +25066,29 @@ components: successCount: type: integer format: int64 + BulkRefreshForm: + type: object + properties: + contentletIds: + type: array + items: + type: string + maxItems: 2147483647 + minItems: 1 + includeDependencies: + type: boolean + includeItemResults: + type: boolean + required: + - contentletIds + BulkRefreshSubmitResponse: + type: object + properties: + jobId: + type: string + submitted: + type: integer + format: int32 BulkResultView: type: object properties: @@ -32311,6 +32366,29 @@ components: type: array items: type: string + ResponseEntityBulkRefreshSubmitView: + type: object + properties: + entity: + $ref: "#/components/schemas/BulkRefreshSubmitResponse" + errors: + type: array + items: + $ref: "#/components/schemas/ErrorEntity" + i18nMessagesMap: + type: object + additionalProperties: + type: string + messages: + type: array + items: + $ref: "#/components/schemas/MessageEntity" + pagination: + $ref: "#/components/schemas/Pagination" + permissions: + type: array + items: + type: string ResponseEntityBulkResultView: type: object properties: diff --git a/dotCMS/src/test/java/com/dotcms/jobs/business/processor/impl/BulkRefreshContentletsProcessorTest.java b/dotCMS/src/test/java/com/dotcms/jobs/business/processor/impl/BulkRefreshContentletsProcessorTest.java new file mode 100644 index 000000000000..70db37f3af48 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/jobs/business/processor/impl/BulkRefreshContentletsProcessorTest.java @@ -0,0 +1,459 @@ +package com.dotcms.jobs.business.processor.impl; + +import static com.dotcms.jobs.business.processor.impl.BulkRefreshContentletsProcessor.PARAM_CONTENTLET_IDS; +import static com.dotcms.jobs.business.processor.impl.BulkRefreshContentletsProcessor.PARAM_INCLUDE_DEPENDENCIES; +import static com.dotcms.jobs.business.processor.impl.BulkRefreshContentletsProcessor.PARAM_INCLUDE_ITEM_RESULTS; +import static com.dotcms.jobs.business.processor.impl.BulkRefreshContentletsProcessor.PARAM_USER_ID; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.dotcms.content.elasticsearch.business.ContentletIndexAPI; +import com.dotcms.jobs.business.error.JobValidationException; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.jobs.business.job.JobState; +import com.dotcms.jobs.business.processor.DefaultProgressTracker; +import com.dotcms.rest.api.v1.content.bulkrefresh.BulkRefreshItemResult; +import com.dotcms.rest.api.v1.content.bulkrefresh.BulkRefreshItemStatus; +import com.dotmarketing.beans.Identifier; +import com.dotmarketing.business.IdentifierAPI; +import com.dotmarketing.business.UserAPI; +import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.portlets.contentlet.business.ContentletAPI; +import com.dotmarketing.portlets.contentlet.business.ContentletCache; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.contentlet.model.IndexPolicy; +import com.liferay.portal.model.User; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; + +/** + * Unit tests for {@link BulkRefreshContentletsProcessor}. + *

+ * The processor's collaborators are injected so these tests can exercise the parts that carry the + * real risk — per-identifier de-duplication, failure isolation, the cancel boundary and the delta + * cursor — without a database or an index. + */ +public class BulkRefreshContentletsProcessorTest { + + private static final String USER_ID = "user-1"; + + private ContentletAPI contentletAPI; + private IdentifierAPI identifierAPI; + private ContentletIndexAPI contentletIndexAPI; + private ContentletCache contentletCache; + private UserAPI userAPI; + private BulkRefreshContentletsProcessor processor; + + @Before + public void setUp() throws Exception { + contentletAPI = mock(ContentletAPI.class); + identifierAPI = mock(IdentifierAPI.class); + contentletIndexAPI = mock(ContentletIndexAPI.class); + contentletCache = mock(ContentletCache.class); + userAPI = mock(UserAPI.class); + + final User user = mock(User.class); + when(user.getUserId()).thenReturn(USER_ID); + when(userAPI.loadUserById(USER_ID)).thenReturn(user); + + processor = new BulkRefreshContentletsProcessor(contentletAPI, identifierAPI, + contentletIndexAPI, contentletCache, userAPI); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#process(Job)} + *

+ * Given scenario: Three inodes are submitted that are three language versions of the same + * contentlet identifier. + *

+ * Expected result: The identifier is reindexed once, {@code total} is 1, and the single record + * lists all three submitted inodes. This is what lets a client mark every selected grid row from + * one result — reporting per inode instead would either reindex the same content three times or + * leave two rows with no outcome. + */ + @Test + public void test_process_collapsesLanguageVersionsOntoOneIdentifier() throws Exception { + final String identifier = "ident-A"; + stubIdentifier(identifier, List.of("inode-en", "inode-es", "inode-fr"), 3); + + final Map metadata = + runAndReadResult(job(List.of("inode-en", "inode-es", "inode-fr"), false, true)); + + assertEquals("Three language rows are one identifier", 1, metadata.get("total")); + assertEquals(1, metadata.get("successCount")); + assertEquals("All three versions were written", 3, metadata.get("versionsIndexed")); + + assertEquals(1, records(metadata).size()); + final BulkRefreshItemResult result = records(metadata).get(0); + assertEquals(identifier, result.identifier().orElse(null)); + assertEquals("Every submitted inode must be named", + Set.of("inode-en", "inode-es", "inode-fr"), Set.copyOf(result.inodes())); + assertEquals(BulkRefreshItemStatus.SUCCESS, result.status()); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#process(Job)} + *

+ * Given scenario: A batch of two inodes, one of which no longer resolves — a grid row that went + * stale between the click and the submit. + *

+ * Expected result: The good identifier is reindexed, the stale inode becomes a FAILED record with + * no identifier, and the job completes rather than aborting. A stale row must not cost the caller + * the rest of their selection. + */ + @Test + public void test_process_unresolvableInodeFailsThatItemOnly() throws Exception { + stubIdentifier("ident-A", List.of("inode-good"), 1); + when(contentletAPI.find(eq("inode-gone"), any(User.class), anyBoolean())).thenReturn(null); + + final Map metadata = + runAndReadResult(job(List.of("inode-good", "inode-gone"), false, true)); + + assertEquals(2, metadata.get("total")); + assertEquals(1, metadata.get("successCount")); + assertEquals(1, metadata.get("failedCount")); + assertEquals(0, metadata.get("skippedCount")); + + final BulkRefreshItemResult failed = records(metadata).stream() + .filter(r -> r.status() == BulkRefreshItemStatus.FAILED) + .findFirst().orElseThrow(); + assertTrue("An unresolved inode has no identifier", failed.identifier().isEmpty()); + assertEquals(List.of("inode-gone"), failed.inodes()); + assertTrue("The failure must say what went wrong", + failed.errorMessage().orElse("").contains("inode-gone")); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#process(Job)} + *

+ * Given scenario: Reading the versions of one identifier throws {@link DotSecurityException}. + *

+ * Expected result: That identifier is a FAILED item and the run continues. A permission problem on + * one identifier is a per-item outcome — treating it as a job failure would discard the results of + * every item already reindexed. + */ + @Test + public void test_process_securityExceptionIsPerItemNotPerJob() throws Exception { + stubIdentifier("ident-A", List.of("inode-ok"), 1); + + final Contentlet denied = contentlet("inode-denied", "ident-B"); + when(contentletAPI.find(eq("inode-denied"), any(User.class), anyBoolean())) + .thenReturn(denied); + final Identifier identB = mock(Identifier.class); + when(identifierAPI.find("ident-B")).thenReturn(identB); + when(contentletAPI.findAllVersions(eq(identB), anyBoolean(), any(User.class), anyBoolean())) + .thenThrow(new DotSecurityException("no access")); + + final Map metadata = + runAndReadResult(job(List.of("inode-ok", "inode-denied"), false, true)); + + assertEquals(1, metadata.get("successCount")); + assertEquals(1, metadata.get("failedCount")); + assertEquals("Every item must be accounted for", + (int) metadata.get("total"), processedSum(metadata)); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#process(Job)} + *

+ * Given scenario: A contentlet is reindexed. + *

+ * Expected result: Its index policy is WAIT_FOR before the index write. The default DEFER policy + * only enqueues into {@code dist_reindex_journal} and returns — that is precisely why the + * single-item endpoint can report success with nothing reindexed, and the whole point of this + * endpoint is that "done" means done. + */ + @Test + public void test_process_indexesSynchronously() throws Exception { + stubIdentifier("ident-A", List.of("inode-en"), 1); + + processor.process(job(List.of("inode-en"), false, false)); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Contentlet.class); + verify(contentletIndexAPI).addContentToIndex(captor.capture(), anyBoolean()); + assertEquals("Indexing must be synchronous, not deferred", + IndexPolicy.WAIT_FOR, captor.getValue().getIndexPolicy()); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#process(Job)} + *

+ * Given scenario: An identifier whose versions may be sitting in the contentlet cache. + *

+ * Expected result: Eviction happens by inode, never by identifier, and the versions that + * get indexed come from a read taken after that eviction. + *

+ * This is the assertion that would have caught the original bug: the cache is keyed by inode, so + * {@code remove(identifier)} evicted nothing while {@code findAllVersions} kept reading back + * through that same cache — quietly writing a stale version to the index, which is the exact + * failure the endpoint exists to repair. + */ + @Test + public void test_process_evictsTheCacheByInodeNotByIdentifier() throws Exception { + stubIdentifier("ident-A", List.of("inode-en"), 2); + + processor.process(job(List.of("inode-en"), false, false)); + + // The submitted inode, evicted as a String key. + verify(contentletCache).remove("inode-en"); + // Never the identifier: that key is never written, so removing it is a no-op. + verify(contentletCache, never()).remove("ident-A"); + // Every version evicted through the Contentlet overload, which resolves to its inode and also + // invalidates the page, host and relationship caches derived from it. + verify(contentletCache, atLeast(2)).remove(any(Contentlet.class)); + // Two reads: the first learns the version inodes, the second is taken cold and is what gets + // indexed. + verify(contentletAPI, times(2)) + .findAllVersions(any(Identifier.class), anyBoolean(), any(User.class), anyBoolean()); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#process(Job)} + *

+ * Given scenario: The job is submitted with {@code includeDependencies} false, then true. + *

+ * Expected result: The flag reaches the index call unchanged. This is a deliberate divergence from + * the single-item {@code _refresh}, which always includes dependencies; at batch size a + * {@code loadDeps()} fan-out per item is a different cost profile, so it must be opt-in. + */ + @Test + public void test_process_passesIncludeDependenciesThrough() throws Exception { + stubIdentifier("ident-A", List.of("inode-en"), 1); + processor.process(job(List.of("inode-en"), false, false)); + verify(contentletIndexAPI).addContentToIndex(any(Contentlet.class), eq(false)); + + setUp(); + stubIdentifier("ident-A", List.of("inode-en"), 1); + processor.process(job(List.of("inode-en"), true, false)); + verify(contentletIndexAPI).addContentToIndex(any(Contentlet.class), eq(true)); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#cancel(Job)} + *

+ * Given scenario: Cancellation is requested before the run starts, then the run executes. + *

+ * Expected result: Nothing is indexed, every identifier is counted skipped, and the counters still + * sum to {@code total}. A cancelled run must leave no item reported as pending — a client showing + * per-row state has to be able to settle every row it was told about. + */ + @Test + public void test_cancel_marksRemainingItemsSkippedAndKeepsCountersWhole() throws Exception { + stubIdentifier("ident-A", List.of("inode-a"), 1); + stubIdentifier("ident-B", List.of("inode-b"), 1); + + final Job job = job(List.of("inode-a", "inode-b"), false, true); + processor.cancel(job); + final Map metadata = runAndReadResult(job); + + verify(contentletIndexAPI, never()).addContentToIndex(any(Contentlet.class), anyBoolean()); + assertEquals(2, metadata.get("skippedCount")); + assertEquals((int) metadata.get("total"), processedSum(metadata)); + assertNotNull("A skip must explain itself", metadata.get("skipReason")); + assertTrue("Skipped items are recorded, not omitted", records(metadata).stream() + .allMatch(r -> r.status() == BulkRefreshItemStatus.SKIPPED)); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#getResultMetadata(Job)} + *

+ * Given scenario: The job ran with {@code includeItemResults} false. + *

+ * Expected result: Counters are present, the per-item array is absent. Counters are what a + * progress bar and a tally need; the breakdown is only for a drill-down, and it is persisted with + * the job, so recording it unasked would store a 500-entry array nobody reads. + */ + @Test + public void test_getResultMetadata_omitsItemResultsWhenNotRequested() throws Exception { + stubIdentifier("ident-A", List.of("inode-a"), 2); + + final Map metadata = + runAndReadResult(job(List.of("inode-a"), false, false)); + + assertNotNull(metadata); + assertEquals(1, metadata.get("total")); + assertEquals(1, metadata.get("successCount")); + assertEquals(0, metadata.get("failedCount")); + assertEquals(0, metadata.get("skippedCount")); + assertEquals(2, metadata.get("versionsIndexed")); + assertEquals(false, metadata.get("includeDependencies")); + assertFalse("results must be absent when not requested", metadata.containsKey("results")); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#getResultMetadata(Job)} + *

+ * Given scenario: The job ran with {@code includeItemResults} true. + *

+ * Expected result: The persisted metadata carries one record per identifier. This is the only + * place the records survive the run, so it has to be complete here. + */ + @Test + public void test_getResultMetadata_carriesEveryRecordWhenRequested() throws Exception { + stubIdentifier("ident-A", List.of("inode-a"), 1); + stubIdentifier("ident-B", List.of("inode-b"), 1); + + final Map metadata = + runAndReadResult(job(List.of("inode-a", "inode-b"), false, true)); + + final List results = records(metadata); + assertNotNull("results must be present when requested", results); + assertEquals("One record per identifier", 2, results.size()); + assertEquals("Records follow submission order, so a client can settle rows as it sent them", + List.of("ident-A", "ident-B"), results.stream() + .map(r -> r.identifier().orElseThrow()).collect(Collectors.toList())); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#process(Job)} + *

+ * Given scenario: Two identifiers are reindexed. + *

+ * Expected result: Progress is reported as work completes and reaches 1.0. Without this the client + * has a job id and no way to show anything moving. + */ + @Test + public void test_process_reportsProgressToCompletion() throws Exception { + stubIdentifier("ident-A", List.of("inode-a"), 1); + stubIdentifier("ident-B", List.of("inode-b"), 1); + + final DefaultProgressTracker tracker = new DefaultProgressTracker(); + processor.process(job(List.of("inode-a", "inode-b"), false, false, tracker)); + + assertEquals(1.0f, tracker.progress(), 0.001f); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#validate(Map)} + *

+ * Given scenario: The parameters carry an empty inode list. + *

+ * Expected result: Rejected before the job is created, so no job id is handed out for work that + * can never happen. + */ + @Test(expected = JobValidationException.class) + public void test_validate_rejectsEmptySelection() { + processor.validate(Map.of(PARAM_CONTENTLET_IDS, List.of(), PARAM_USER_ID, USER_ID)); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#validate(Map)} + *

+ * Given scenario: The parameters carry no inode list at all. + *

+ * Expected result: Rejected, same as an empty list. + */ + @Test(expected = JobValidationException.class) + public void test_validate_rejectsMissingSelection() { + processor.validate(Map.of(PARAM_USER_ID, USER_ID)); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#validate(Map)} + *

+ * Given scenario: A well-formed set of parameters. + *

+ * Expected result: Accepted without throwing. + */ + @Test + public void test_validate_acceptsAWellFormedSelection() { + processor.validate(Map.of( + PARAM_CONTENTLET_IDS, List.of("inode-a"), + PARAM_USER_ID, USER_ID)); + } + + /** + * Runs the job and returns what the framework will persist as its result. + *

+ * The counters and records are read back the same way production reads them — through + * {@link BulkRefreshContentletsProcessor#getResultMetadata(Job)} — rather than through a + * test-only accessor, so these assertions cover the path a client actually sees. + */ + private Map runAndReadResult(final Job job) { + processor.process(job); + return processor.getResultMetadata(job); + } + + @SuppressWarnings("unchecked") + private static List records(final Map metadata) { + return (List) metadata.get("results"); + } + + private static int processedSum(final Map metadata) { + return (int) metadata.get("successCount") + + (int) metadata.get("failedCount") + + (int) metadata.get("skippedCount"); + } + + /** + * Stubs the inode → contentlet → identifier → versions chain for one identifier. + * + * @param identifier the identifier every given inode resolves to + * @param inodes the submitted inodes that resolve to it + * @param versionCount how many versions {@code findAllVersions} returns + */ + private void stubIdentifier(final String identifier, final List inodes, + final int versionCount) throws Exception { + for (final String inode : inodes) { + when(contentletAPI.find(eq(inode), any(User.class), anyBoolean())) + .thenReturn(contentlet(inode, identifier)); + } + final Identifier id = mock(Identifier.class); + when(id.getId()).thenReturn(identifier); + when(identifierAPI.find(identifier)).thenReturn(id); + + final List versions = new ArrayList<>(); + for (int i = 0; i < versionCount; i++) { + versions.add(contentlet(identifier + "-v" + i, identifier)); + } + when(contentletAPI.findAllVersions(eq(id), anyBoolean(), any(User.class), anyBoolean())) + .thenReturn(versions); + } + + private static Contentlet contentlet(final String inode, final String identifier) { + final Contentlet contentlet = new Contentlet(); + contentlet.setInode(inode); + contentlet.setIdentifier(identifier); + return contentlet; + } + + private static Job job(final List inodes, final boolean includeDependencies, + final boolean includeItemResults) { + return job(inodes, includeDependencies, includeItemResults, new DefaultProgressTracker()); + } + + private static Job job(final List inodes, final boolean includeDependencies, + final boolean includeItemResults, final DefaultProgressTracker tracker) { + final Map parameters = new HashMap<>(); + parameters.put(PARAM_CONTENTLET_IDS, inodes); + parameters.put(PARAM_INCLUDE_DEPENDENCIES, includeDependencies); + parameters.put(PARAM_INCLUDE_ITEM_RESULTS, includeItemResults); + parameters.put(PARAM_USER_ID, USER_ID); + + return Job.builder() + .id("job-1") + .queueName("bulkRefreshContentlets") + .state(JobState.RUNNING) + .parameters(parameters) + .progressTracker(tracker) + .build(); + } +} diff --git a/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshCompletionListenerTest.java b/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshCompletionListenerTest.java new file mode 100644 index 000000000000..b942484d3135 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshCompletionListenerTest.java @@ -0,0 +1,224 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.dotcms.api.system.event.Payload; +import com.dotcms.api.system.event.SystemEventType; +import com.dotcms.api.system.event.SystemEventsAPI; +import com.dotcms.api.system.event.Visibility; +import com.dotcms.jobs.business.api.events.JobCompletedEvent; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.jobs.business.job.JobResult; +import com.dotcms.jobs.business.job.JobState; +import com.dotcms.jobs.business.processor.impl.BulkRefreshContentletsProcessor; +import com.dotcms.notifications.bean.NotificationLevel; +import com.dotcms.notifications.business.NotificationAPI; +import com.dotcms.util.I18NMessage; +import com.dotmarketing.business.UserAPI; +import com.google.common.collect.ImmutableMap; +import com.liferay.portal.model.User; +import java.time.LocalDateTime; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; + +/** + * Unit tests for {@link BulkRefreshCompletionListener}. + *

+ * This is the piece that replaced polling, so what matters is that a finished run genuinely announces + * itself: to the right person, with the counts it actually produced, and worded on what happened rather + * than assuming success. + */ +public class BulkRefreshCompletionListenerTest { + + private static final String USER_ID = "user-1"; + + private SystemEventsAPI systemEventsAPI; + private NotificationAPI notificationAPI; + private BulkRefreshCompletionListener listener; + + @Before + public void setUp() throws Exception { + systemEventsAPI = mock(SystemEventsAPI.class); + notificationAPI = mock(NotificationAPI.class); + + final UserAPI userAPI = mock(UserAPI.class); + final User user = mock(User.class); + when(user.getLocale()).thenReturn(java.util.Locale.ENGLISH); + when(userAPI.loadUserById(USER_ID)).thenReturn(user); + + listener = new BulkRefreshCompletionListener(systemEventsAPI, notificationAPI, userAPI); + } + + /** + * Method to test: {@link BulkRefreshCompletionListener#notify} + *

+ * Given scenario: A bulk refresh job finishes successfully. + *

+ * Expected result: An event carrying the run's counters is pushed to the submitting user only. + * Scoping it to the user is deliberate — the legacy batch reindex announced completion to every CMS + * Administrator, which is nobody else's business. + */ + @Test + public void test_onJobCompleted_pushesTheCountersToTheSubmitter() throws Exception { + listener.notify(event(JobState.SUCCESS, counters(3, 3, 0, 0, 5))); + + final ArgumentCaptor payload = ArgumentCaptor.forClass(Payload.class); + verify(systemEventsAPI) + .pushAsync(eq(SystemEventType.BULK_REFRESH_COMPLETED), payload.capture()); + + assertEquals(Visibility.USER, payload.getValue().getVisibility()); + assertEquals(USER_ID, payload.getValue().getVisibilityValue()); + + @SuppressWarnings("unchecked") + final Map data = (Map) payload.getValue().getData(); + assertEquals(3, data.get(BulkRefreshCompletionListener.EVENT_TOTAL)); + assertEquals(3, data.get(BulkRefreshCompletionListener.EVENT_SUCCESS_COUNT)); + assertEquals(5, data.get(BulkRefreshCompletionListener.EVENT_VERSIONS_INDEXED)); + assertEquals(JobState.SUCCESS, data.get(BulkRefreshCompletionListener.EVENT_STATE)); + } + + /** + * Method to test: {@link BulkRefreshCompletionListener#notify} + *

+ * Given scenario: A job from a different queue finishes. + *

+ * Expected result: Ignored entirely. This listener sits on the shared job-completed event, so every + * content import and every other queue's job passes through it. + */ + @Test + public void test_onJobCompleted_ignoresOtherQueues() throws Exception { + final Job job = mock(Job.class); + when(job.queueName()).thenReturn("importContentlets"); + + listener.notify(new JobCompletedEvent(job, LocalDateTime.now())); + + verify(systemEventsAPI, never()).pushAsync(any(), any()); + verify(notificationAPI, never()).generateNotification( + any(I18NMessage.class), any(I18NMessage.class), any(), any(), any(), any(), + any(), any(), any()); + } + + /** + * Method to test: {@link BulkRefreshCompletionListener#notify} + *

+ * Given scenario: A clean run, then a run with failures, then a permanently failed job. + *

+ * Expected result: The notification level tracks what actually happened. The legacy batch reindex + * reported "finished successfully" unconditionally — even when every single item had failed — which + * is the misleading signal this endpoint exists to remove. + */ + @Test + public void test_onJobCompleted_notificationLevelReflectsTheOutcome() throws Exception { + listener.notify(event(JobState.SUCCESS, counters(2, 2, 0, 0, 2))); + assertEquals(NotificationLevel.INFO, capturedLevel()); + + setUpFresh(); + listener.notify(event(JobState.SUCCESS, counters(3, 2, 1, 0, 2))); + assertEquals("A shortfall must not read as a clean run", + NotificationLevel.WARNING, capturedLevel()); + + setUpFresh(); + listener.notify(event(JobState.FAILED_PERMANENTLY, counters(0, 0, 0, 0, 0))); + assertEquals("A dead job must not read as a success", + NotificationLevel.ERROR, capturedLevel()); + } + + /** + * Method to test: {@link BulkRefreshCompletionListener#notify} + *

+ * Given scenario: A job whose parameters carry no submitting user. + *

+ * Expected result: Nothing is pushed. There is nobody to tell, and guessing a recipient would send + * somebody else's reindex outcome to the wrong person. + */ + @Test + public void test_onJobCompleted_withoutASubmitterTellsNobody() throws Exception { + final Job job = mock(Job.class); + when(job.queueName()).thenReturn(BulkRefreshHelper.BULK_REFRESH_QUEUE_NAME); + when(job.parameters()).thenReturn(ImmutableMap.of()); + + listener.notify(new JobCompletedEvent(job, LocalDateTime.now())); + + verify(systemEventsAPI, never()).pushAsync(any(), any()); + } + + /** + * Method to test: {@link BulkRefreshCompletionListener#notify} + *

+ * Given scenario: A terminal job that carried no result metadata at all. + *

+ * Expected result: The event still goes out, carrying the state but no counters, so the client can + * tell "finished but unreportable" apart from a clean run over nothing — which is what all-zero + * counters would look like. + */ + @Test + public void test_onJobCompleted_withoutMetadataStillReportsTheState() throws Exception { + final Job job = mock(Job.class); + when(job.queueName()).thenReturn(BulkRefreshHelper.BULK_REFRESH_QUEUE_NAME); + when(job.state()).thenReturn(JobState.SUCCESS); + when(job.parameters()).thenReturn( + ImmutableMap.of(BulkRefreshContentletsProcessor.PARAM_USER_ID, USER_ID)); + when(job.result()).thenReturn(Optional.empty()); + + listener.notify(new JobCompletedEvent(job, LocalDateTime.now())); + + final ArgumentCaptor payload = ArgumentCaptor.forClass(Payload.class); + verify(systemEventsAPI).pushAsync(any(), payload.capture()); + + @SuppressWarnings("unchecked") + final Map data = (Map) payload.getValue().getData(); + assertEquals(JobState.SUCCESS, data.get(BulkRefreshCompletionListener.EVENT_STATE)); + assertTrue("No counters is different from zero counters", + !data.containsKey(BulkRefreshCompletionListener.EVENT_TOTAL)); + } + + private void setUpFresh() throws Exception { + setUp(); + } + + private NotificationLevel capturedLevel() throws Exception { + final ArgumentCaptor level = + ArgumentCaptor.forClass(NotificationLevel.class); + verify(notificationAPI).generateNotification( + any(I18NMessage.class), any(I18NMessage.class), any(), level.capture(), + any(), any(), any(), any(), any()); + + return level.getValue(); + } + + private static Map counters(final int total, final int success, final int failed, + final int skipped, final int versions) { + final Map metadata = new HashMap<>(); + metadata.put(BulkRefreshCompletionListener.EVENT_TOTAL, total); + metadata.put(BulkRefreshCompletionListener.EVENT_SUCCESS_COUNT, success); + metadata.put(BulkRefreshCompletionListener.EVENT_FAILED_COUNT, failed); + metadata.put(BulkRefreshCompletionListener.EVENT_SKIPPED_COUNT, skipped); + metadata.put(BulkRefreshCompletionListener.EVENT_VERSIONS_INDEXED, versions); + + return metadata; + } + + private static JobCompletedEvent event(final JobState state, + final Map metadata) { + final Job job = mock(Job.class); + when(job.queueName()).thenReturn(BulkRefreshHelper.BULK_REFRESH_QUEUE_NAME); + when(job.state()).thenReturn(state); + when(job.parameters()).thenReturn( + ImmutableMap.of(BulkRefreshContentletsProcessor.PARAM_USER_ID, USER_ID)); + when(job.result()).thenReturn( + Optional.of(JobResult.builder().metadata(metadata).build())); + + return new JobCompletedEvent(job, LocalDateTime.now()); + } +} diff --git a/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshFormTest.java b/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshFormTest.java new file mode 100644 index 000000000000..996c7d960f45 --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshFormTest.java @@ -0,0 +1,111 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import com.dotcms.rest.exception.ValidationException; +import java.util.Collections; +import java.util.List; +import org.junit.Test; + +/** + * Unit tests for {@link BulkRefreshForm}. + */ +public class BulkRefreshFormTest { + + /** + * Method to test: {@link BulkRefreshForm} constructor + *

+ * Given scenario: Only {@code contentletIds} is supplied; both flags are omitted (null). + *

+ * Expected result: Both flags default to false. This matters for {@code includeDependencies} in + * particular — the single-item {@code _refresh} always includes dependencies, so a caller + * migrating to the bulk endpoint must not silently inherit a per-item {@code loadDeps()} fan-out + * across the whole batch. + */ + @Test + public void test_flags_defaultToFalse_whenOmitted() { + final BulkRefreshForm form = new BulkRefreshForm(List.of("inode-1"), null, null); + + assertFalse("includeDependencies must default to false", form.isIncludeDependencies()); + assertFalse("includeItemResults must default to false", form.isIncludeItemResults()); + assertEquals(List.of("inode-1"), form.getContentletIds()); + } + + /** + * Method to test: {@link BulkRefreshForm} constructor + *

+ * Given scenario: Both flags are explicitly true. + *

+ * Expected result: Both are honored. + */ + @Test + public void test_flags_honorExplicitTrue() { + final BulkRefreshForm form = new BulkRefreshForm(List.of("inode-1"), true, true); + + assertTrue(form.isIncludeDependencies()); + assertTrue(form.isIncludeItemResults()); + } + + /** + * Method to test: {@link BulkRefreshForm} constructor + *

+ * Given scenario: {@code contentletIds} is an empty list. + *

+ * Expected result: Rejected at construction. An empty selection is a client bug, not a no-op + * job — enqueuing one would hand back a job id that can only ever report zero work. + */ + @Test(expected = ValidationException.class) + public void test_emptyContentletIds_isRejected() { + new BulkRefreshForm(Collections.emptyList(), null, null); + } + + /** + * Method to test: {@link BulkRefreshForm} constructor + *

+ * Given scenario: {@code contentletIds} is missing from the JSON altogether. + *

+ * Expected result: Rejected at construction, same as an empty list. + */ + @Test(expected = ValidationException.class) + public void test_nullContentletIds_isRejected() { + new BulkRefreshForm(null, null, null); + } + + /** + * Method to test: {@link BulkRefreshForm} constructor + *

+ * Given scenario: The same inode is submitted several times. + *

+ * Expected result: Accepted as-is. Duplicates are a normal consequence of a grid selection and + * are collapsed by identifier server-side, so the form must not reject them — nor de-duplicate + * them here, because the response reports the raw submitted count separately from the + * de-duplicated total. + */ + @Test + public void test_duplicateInodes_areAccepted() { + final BulkRefreshForm form = + new BulkRefreshForm(List.of("inode-1", "inode-1", "inode-2"), null, null); + + assertEquals("Duplicates must survive the form untouched", 3, + form.getContentletIds().size()); + } + + /** + * Method to test: {@link BulkRefreshForm#toString()} + *

+ * Given scenario: A form holding inodes is logged. + *

+ * Expected result: The count is reported rather than the inodes themselves, so a 500-item + * submission does not dump 500 ids into the log on every debug line. + */ + @Test + public void test_toString_reportsCountNotContents() { + final String asString = + new BulkRefreshForm(List.of("inode-1", "inode-2"), null, null).toString(); + + assertTrue("Should report the count", asString.contains("2 item(s)")); + assertFalse("Should not spell out the inodes", asString.contains("inode-1")); + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java b/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java index 2e9375622e65..c118558e378a 100644 --- a/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java +++ b/dotcms-integration/src/test/java/com/dotcms/Junit5Suite1.java @@ -5,6 +5,7 @@ import com.dotcms.jobs.business.api.JobQueueManagerAPIIntegrationTest; import com.dotcms.jobs.business.processor.impl.ImportContentletsProcessorIntegrationTest; import com.dotcms.jobs.business.queue.PostgresJobQueueIntegrationTest; +import com.dotcms.rest.api.v1.content.bulkrefresh.BulkRefreshResourceIntegrationTest; import com.dotcms.rest.api.v1.content.dotimport.ContentImportResourceIntegrationTest; import com.dotcms.rest.api.v1.job.JobQueueHelperIntegrationTest; import org.junit.platform.suite.api.SelectClasses; @@ -18,6 +19,7 @@ JobQueueHelperIntegrationTest.class, ImportContentletsProcessorIntegrationTest.class, ContentImportResourceIntegrationTest.class, + BulkRefreshResourceIntegrationTest.class, JobProcessorDiscoveryTest.class }) public class Junit5Suite1 { diff --git a/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshResourceIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshResourceIntegrationTest.java new file mode 100644 index 000000000000..9aa320d24a4a --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/content/bulkrefresh/BulkRefreshResourceIntegrationTest.java @@ -0,0 +1,534 @@ +package com.dotcms.rest.api.v1.content.bulkrefresh; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.dotcms.Junit5WeldBaseTest; +import com.dotcms.content.elasticsearch.business.ContentletIndexAPI; +import com.dotcms.contenttype.model.type.ContentType; +import com.dotcms.datagen.ContentletDataGen; +import com.dotcms.datagen.RoleDataGen; +import com.dotcms.datagen.TestDataUtils; +import com.dotcms.datagen.TestUserUtils; +import com.dotcms.jobs.business.api.JobQueueManagerAPI; +import com.dotcms.jobs.business.job.Job; +import com.dotcms.jobs.business.job.JobState; +import com.dotcms.jobs.business.processor.impl.BulkRefreshContentletsProcessor; +import com.dotcms.jobs.business.util.JobUtil; +import com.dotcms.mock.response.MockHttpResponse; +import com.dotcms.rest.ResponseEntityBulkRefreshSubmitView; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.beans.Host; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.CacheLocator; +import com.dotmarketing.business.Role; +import com.dotmarketing.exception.DoesNotExistException; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import com.dotmarketing.portlets.languagesmanager.model.Language; +import com.dotmarketing.util.Config; +import com.dotmarketing.util.Logger; +import com.dotmarketing.util.UUIDGenerator; +import com.liferay.portal.model.User; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.core.Response; +import org.awaitility.Awaitility; +import org.jboss.weld.junit5.EnableWeld; +import io.vavr.control.Try; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Integration tests for {@link BulkRefreshResource} — {@code POST /api/v1/content/_bulkrefresh}. + *

+ * The tests that matter most here are the ones that hold the endpoint to its promise rather than to + * its shape: that content genuinely absent from the index is findable once the job reports SUCCESS + * (which only holds if indexing is synchronous), and that every submitted row ends up accounted for in + * the counters however the run ends. + */ +@EnableWeld +public class BulkRefreshResourceIntegrationTest extends Junit5WeldBaseTest { + + private static User adminUser; + private static User powerUser; + private static User plainBackendUser; + private static Host defaultSite; + private static Language defaultLanguage; + private static ContentType contentType; + private static ContentletIndexAPI indexAPI; + private static HttpServletResponse response; + + private BulkRefreshResource resource; + + @Inject + BulkRefreshHelper bulkRefreshHelper; + + @Inject + JobQueueManagerAPI jobQueueManagerAPI; + + /** Static so {@link #stopQueueIfWeStartedIt()} can reach them; only this class writes them. */ + private static boolean queueStartedHere; + private static JobQueueManagerAPI sharedJobQueueManagerAPI; + + @BeforeAll + static void setUp() throws Exception { + IntegrationTestInitService.getInstance().init(); + + adminUser = TestUserUtils.getAdminUser(); + defaultSite = APILocator.getHostAPI().findDefaultHost(adminUser, false); + defaultLanguage = APILocator.getLanguageAPI().getDefaultLanguage(); + contentType = TestDataUtils.getRichTextLikeContentType(); + indexAPI = APILocator.getContentletIndexAPI(); + response = new MockHttpResponse(); + + powerUser = TestUserUtils.getUser(getOrCreatePowerUserRole(), + "bulkrefresh.power@dotcms.com", "Bulk", "Power", "bulkrefreshpower"); + plainBackendUser = TestUserUtils.getBackendUser(defaultSite); + } + + @BeforeEach + void prepare() throws Exception { + resource = new BulkRefreshResource(bulkRefreshHelper); + + // Without this the queue accepts jobs and never runs them: every job stays PENDING and each + // wait below times out. ContentImportResourceIntegrationTest never noticed because it only + // asserts job *creation* — 24 tests in under nine seconds — so this suite had no test that + // actually needed a processor to execute until now. + if (!jobQueueManagerAPI.isStarted()) { + jobQueueManagerAPI.start(); + jobQueueManagerAPI.awaitStart(5, TimeUnit.SECONDS); + queueStartedHere = true; + } + sharedJobQueueManagerAPI = jobQueueManagerAPI; + } + + /** + * Hands the queue back in the state this class found it. + * + * Leaving it running would keep it draining jobs for the rest of the JVM, and this class shares a + * suite with tests that only assert job *creation* — they pass either way, so the difference would + * surface as confusing behaviour elsewhere rather than as a failure here. Declaration order + * happening to put this class last is luck, not isolation. + */ + @AfterAll + static void stopQueueIfWeStartedIt() { + if (queueStartedHere && null != sharedJobQueueManagerAPI) { + Try.run(sharedJobQueueManagerAPI::close) + .onFailure(e -> Logger.warn(BulkRefreshResourceIntegrationTest.class, + "Unable to stop the job queue after the bulk refresh tests", e)); + } + } + + /** + * Method to test: {@link BulkRefreshResource#bulkRefresh} + *

+ * Given scenario: A contentlet is created and then deleted from the index behind dotCMS's back, + * leaving the database correct and the index wrong — the exact situation this endpoint exists for. + * A CMS Administrator submits it. + *

+ * Expected result: The job reaches SUCCESS and the contentlet is findable in the index again. This + * is the test that actually proves indexing is synchronous: under the default DEFER policy the job + * would report SUCCESS having only written a row to {@code dist_reindex_journal}, and this + * assertion would fail. + */ + @Test + void test_bulkRefresh_makesContentMissingFromTheIndexFindableAgain() throws Exception { + final Contentlet contentlet = newContentlet(); + indexAPI.removeContentFromIndex(contentlet); + assertFalse(isInIndex(contentlet.getIdentifier()), + "Precondition: the contentlet must be absent from the index"); + + final String jobId = submit(adminUser, List.of(contentlet.getInode()), false, true); + final Job job = awaitTerminal(jobId); + + assertEquals(JobState.SUCCESS, job.state()); + assertTrue(isInIndex(contentlet.getIdentifier()), + "A job reporting SUCCESS must mean the content is actually searchable"); + } + + /** + * Method to test: {@link BulkRefreshResource#bulkRefresh} + *

+ * Given scenario: Three language versions of one contentlet are submitted by their separate + * inodes. + *

+ * Expected result: {@code total} is 1 and the single record names all three inodes, while + * {@code submitted} still reports 3. Reindexing the same identifier three times would be wasted + * work, but a client that selected three rows needs all three named back so it can settle them. + */ + @Test + void test_bulkRefresh_deduplicatesLanguageVersionsButReportsEveryInode() throws Exception { + final Contentlet english = newContentlet(); + final Language spanish = TestDataUtils.getSpanishLanguage(); + final Contentlet translated = ContentletDataGen.checkout(english); + translated.setLanguageId(spanish.getId()); + final Contentlet spanishVersion = ContentletDataGen.checkin(translated); + + final List inodes = List.of(english.getInode(), spanishVersion.getInode()); + final BulkRefreshSubmitResponse submitted = submitResponse(adminUser, inodes, false, true); + assertEquals(2, submitted.submitted(), "submitted is the raw inode count"); + + final Job job = awaitTerminal(submitted.jobId()); + final Map metadata = metadata(job); + + assertEquals(1, metadata.get("total"), "Two language rows are one identifier"); + assertEquals(1, metadata.get("successCount")); + + final List> results = itemResults(metadata); + assertEquals(1, results.size()); + @SuppressWarnings("unchecked") + final List reported = (List) results.get(0).get("inodes"); + assertTrue(reported.containsAll(inodes), + "Every submitted inode must be named so the client can mark its rows"); + } + + /** + * Method to test: {@link BulkRefreshResource#bulkRefresh} + *

+ * Given scenario: A batch containing one real inode and one that never existed. + *

+ * Expected result: The job still reaches SUCCESS, with one success and one failure. A selection can + * go stale between the click and the submit, and losing the rest of the batch to one dead row would + * make the endpoint unusable on a busy site. + */ + @Test + void test_bulkRefresh_mixedBatchSucceedsAndReportsTheFailure() throws Exception { + final Contentlet contentlet = newContentlet(); + final String ghostInode = UUIDGenerator.generateUuid(); + + final Job job = awaitTerminal( + submit(adminUser, List.of(contentlet.getInode(), ghostInode), false, true)); + final Map metadata = metadata(job); + + assertEquals(JobState.SUCCESS, job.state(), "One bad row must not fail the job"); + assertEquals(1, metadata.get("successCount")); + assertEquals(1, metadata.get("failedCount")); + + final Map failure = itemResults(metadata).stream() + .filter(r -> "FAILED".equals(String.valueOf(r.get("status")))) + .findFirst().orElseThrow(); + assertTrue(String.valueOf(failure.get("errorMessage")).contains(ghostInode), + "The failure must name the inode that could not be resolved"); + } + + /** + * Method to test: {@link BulkRefreshResource#bulkRefresh} + *

+ * Given scenario: However a run ends, the counters are read back. + *

+ * Expected result: {@code success + failed + skipped == total}. Any client rendering per-row state + * relies on this to know it can stop waiting; a run whose counters do not close leaves rows + * spinning forever. + */ + @Test + void test_bulkRefresh_countersAlwaysSumToTotal() throws Exception { + final Contentlet first = newContentlet(); + final Contentlet second = newContentlet(); + + final Job job = awaitTerminal(submit(adminUser, + List.of(first.getInode(), second.getInode(), UUIDGenerator.generateUuid()), + false, true)); + final Map metadata = metadata(job); + + final int total = (int) metadata.get("total"); + final int sum = (int) metadata.get("successCount") + + (int) metadata.get("failedCount") + + (int) metadata.get("skippedCount"); + assertEquals(total, sum, "Every item must be accounted for exactly once"); + assertEquals(total, itemResults(metadata).size(), "One record per item when recorded"); + } + + /** + * Method to test: {@link BulkRefreshResource#bulkRefresh} + *

+ * Given scenario: The same selection is submitted once with {@code includeItemResults} false and + * once with it true. + *

+ * Expected result: Counters both times; the per-item array only when asked for. It is persisted + * with the job, so recording a 500-entry array nobody requested is storage spent on nothing. + */ + @Test + void test_bulkRefresh_itemResultsAreOptIn() throws Exception { + final Contentlet contentlet = newContentlet(); + + final Map without = + metadata(awaitTerminal(submit(adminUser, List.of(contentlet.getInode()), false, false))); + assertEquals(1, without.get("total"), "Counters are always reported"); + assertFalse(without.containsKey("results"), + "The per-item array must be absent when it was not requested"); + + final Map with = + metadata(awaitTerminal(submit(adminUser, List.of(contentlet.getInode()), false, true))); + assertEquals(1, itemResults(with).size()); + } + + /** + * Method to test: {@link BulkRefreshResource#bulkRefresh} + *

+ * Given scenario: A plain backend user with neither the CMS Power User nor the CMS Administrator + * role submits a selection. + *

+ * Expected result: Rejected. The legacy Refresh button was gated the same way in + * {@code view_contentlets.jsp}; reindexing is expensive, and the client hiding the action is not + * authorization. + */ + @Test + void test_bulkRefresh_plainBackendUserIsForbidden() throws Exception { + final Contentlet contentlet = newContentlet(); + + assertThrows(DotSecurityException.class, () -> resource.bulkRefresh( + requestFor(plainBackendUser), response, + new BulkRefreshForm(List.of(contentlet.getInode()), false, false))); + } + + /** + * Method to test: {@link BulkRefreshResource#bulkRefresh} + *

+ * Given scenario: A CMS Power User submits a selection. + *

+ * Expected result: Accepted. Power Users could press the legacy button, so none of them may lose + * the capability in the move to the Action Center. + */ + @Test + void test_bulkRefresh_powerUserIsAllowed() throws Exception { + final Contentlet contentlet = newContentlet(); + + final Job job = awaitTerminal(submit(powerUser, List.of(contentlet.getInode()), false, false)); + assertEquals(JobState.SUCCESS, job.state()); + } + + /** + * Method to test: {@link BulkRefreshResource#bulkRefresh} + *

+ * Given scenario: More inodes than the configured cap are submitted. + *

+ * Expected result: Rejected outright. Synchronous indexing over an unbounded selection is a + * self-inflicted full reindex, which is the one thing this endpoint must never become. + */ + @Test + void test_bulkRefresh_rejectsSelectionsOverTheCap() throws Exception { + final int cap = Config.getIntProperty(BulkRefreshHelper.MAX_ITEMS_CONFIG_PROPERTY, + BulkRefreshHelper.MAX_ITEMS_DEFAULT); + final List tooMany = new ArrayList<>(); + for (int i = 0; i <= cap; i++) { + tooMany.add(UUIDGenerator.generateUuid()); + } + + assertThrows(IllegalArgumentException.class, () -> resource.bulkRefresh( + requestFor(adminUser), response, new BulkRefreshForm(tooMany, false, false))); + } + + /** + * Method to test: {@link com.dotcms.rest.api.v1.content.bulkrefresh.BulkRefreshCompletionListener} + *

+ * Given scenario: A reindex is submitted and allowed to finish. + *

+ * Expected result: The submitter gains a notification, which is only possible if the completion + * listener was actually registered at startup and fired for this job. + *

+ * This is the assertion the suite was missing. Completion is reported by push now, so every other + * test here can pass while the reporting path is entirely dead — and it was: the listener began life + * as a CDI bean nothing injected, so it was never constructed, never subscribed, and never told + * anyone anything, with all 12 tests green. + */ + @Test + void test_bulkRefresh_completionNotifiesTheSubmitter() throws Exception { + final Contentlet contentlet = newContentlet(); + final Long before = APILocator.getNotificationAPI() + .getNotificationsCount(adminUser.getUserId()); + + awaitTerminal(submit(adminUser, List.of(contentlet.getInode()), false, false)); + + // The notification is raised off the job-completed event, so it can land slightly after the job + // itself reaches a terminal state. + Awaitility.await().atMost(30, TimeUnit.SECONDS).pollInterval(500, TimeUnit.MILLISECONDS) + .until(() -> APILocator.getNotificationAPI() + .getNotificationsCount(adminUser.getUserId()) > before); + } + + /** + * Method to test: {@link BulkRefreshResource#bulkRefresh} + *

+ * Given scenario: A submission is accepted. + *

+ * Expected result: HTTP 202 with the job id. 202 rather than 200 because the work is accepted and + * not yet done — that distinction is the whole reason this endpoint is job-backed, and a client + * must not be able to read the response as "reindexed". Completion arrives by push, not by the + * client asking. + */ + @Test + void test_bulkRefresh_respondsAcceptedWithAJobHandle() throws Exception { + final Contentlet contentlet = newContentlet(); + + final Response httpResponse = resource.bulkRefresh(requestFor(adminUser), response, + new BulkRefreshForm(List.of(contentlet.getInode()), false, false)); + + assertEquals(Response.Status.ACCEPTED.getStatusCode(), httpResponse.getStatus()); + + final BulkRefreshSubmitResponse entity = + ((ResponseEntityBulkRefreshSubmitView) httpResponse.getEntity()).getEntity(); + assertNotNull(entity.jobId()); + assertEquals(1, entity.submitted(), "submitted is the raw inode count"); + } + + /** + * Method to test: {@link BulkRefreshContentletsProcessor#cancel} + *

+ * Given scenario: A larger selection is submitted and cancellation is requested straight away. + * There is no reindex-specific cancel endpoint — the UI never called one — so this goes through + * the generic job queue, which is the only way a run gets cancelled in production. + *

+ * Expected result: However the race lands — cancellation reaching the run mid-flight, or the run + * finishing first — the counters still close over {@code total}, and if the job did end up + * CANCELED then some items are reported skipped rather than left pending. The assertion is + * deliberately about the invariant rather than about winning the race, because a test that depends + * on timing here would be flaky rather than informative. + */ + @Test + void test_cancelledRun_leavesNoItemPending() throws Exception { + final List inodes = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + inodes.add(newContentlet().getInode()); + } + + final String jobId = submit(adminUser, inodes, false, true); + try { + jobQueueManagerAPI.cancelJob(jobId); + } catch (final IllegalStateException | DoesNotExistException e) { + // The run may already be terminal by the time cancel lands; that is one of the two + // legitimate outcomes and the invariant below covers both. Narrowed deliberately: catching + // everything would let a genuine NullPointerException from cancelJob pass as "the race + // landed the other way". + Logger.info(BulkRefreshResourceIntegrationTest.class, + "Cancel arrived after the run finished: " + e.getMessage()); + } + + final Job job = awaitTerminal(jobId); + final Map metadata = metadata(job); + final int total = (int) metadata.get("total"); + assertEquals(total, (int) metadata.get("successCount") + + (int) metadata.get("failedCount") + + (int) metadata.get("skippedCount"), + "A cancelled run must still account for every item"); + assertEquals(total, itemResults(metadata).size(), + "Skipped items are recorded, not omitted"); + + if (JobState.CANCELED == job.state()) { + assertTrue((int) metadata.get("skippedCount") > 0, + "A cancelled run must report what it did not attempt"); + } + } + + // --------------------------------------------------------------------------------------------- + // helpers + // --------------------------------------------------------------------------------------------- + + /** + * The CMS Power User role, created if this database does not have it. + * + * {@code loadRoleByKey} answers null for a missing role rather than throwing, and + * {@code doesUserHaveRole(user, null)} is then silently false — so passing the lookup straight + * through produced a "power user" with no such role and a permission check that could only ever + * fail. Mirrors {@code TestUserUtils.getOrCreateAdminRole}. + */ + private static Role getOrCreatePowerUserRole() throws DotDataException { + final Role existing = APILocator.getRoleAPI().loadRoleByKey(Role.CMS_POWER_USER); + + if (null != existing) { + return existing; + } + + // Not a dead branch: this database really does lack the role. Before this helper existed the + // power-user test failed with "must be a CMS Power User or a CMS Administrator" precisely + // because loadRoleByKey answered null here, so failing loudly instead of creating it would + // just reinstate that failure. Logged rather than silent, because persisting a role into a + // shared test database is worth seeing in the output. + Logger.warn(BulkRefreshResourceIntegrationTest.class, String.format( + "Role [%s] not present; creating it for the bulk refresh permission tests", + Role.CMS_POWER_USER)); + + return new RoleDataGen().key(Role.CMS_POWER_USER).nextPersisted(); + } + + private Contentlet newContentlet() { + final Contentlet contentlet = new ContentletDataGen(contentType.id()) + .languageId(defaultLanguage.getId()) + .host(defaultSite) + .nextPersisted(); + Awaitility.await().atMost(30, TimeUnit.SECONDS).pollInterval(500, TimeUnit.MILLISECONDS) + .until(() -> isInIndex(contentlet.getIdentifier())); + return contentlet; + } + + private static boolean isInIndex(final String identifier) throws Exception { + CacheLocator.getESQueryCache().clearCache(); + return !APILocator.getContentletAPI() + .searchIndex("+identifier:" + identifier, 10, 0, "moddate", adminUser, false) + .isEmpty(); + } + + private static HttpServletRequest requestFor(final User user) { + return JobUtil.generateMockRequest(user, defaultSite.getHostname()); + } + + private String submit(final User user, final List inodes, + final boolean includeDependencies, final boolean includeItemResults) throws Exception { + return submitResponse(user, inodes, includeDependencies, includeItemResults).jobId(); + } + + private BulkRefreshSubmitResponse submitResponse(final User user, final List inodes, + final boolean includeDependencies, final boolean includeItemResults) throws Exception { + final Response httpResponse = resource.bulkRefresh(requestFor(user), response, + new BulkRefreshForm(inodes, includeDependencies, includeItemResults)); + return ((ResponseEntityBulkRefreshSubmitView) httpResponse.getEntity()).getEntity(); + } + + private Job awaitTerminal(final String jobId) { + return Awaitility.await().atMost(120, TimeUnit.SECONDS) + .pollInterval(500, TimeUnit.MILLISECONDS) + .until(() -> jobQueueManagerAPI.getJob(jobId), job -> isTerminal(job.state())); + } + + private static boolean isTerminal(final JobState state) { + return state == JobState.SUCCESS || state == JobState.CANCELED + || state == JobState.FAILED_PERMANENTLY + || state == JobState.ABANDONED_PERMANENTLY; + } + + private static Map metadata(final Job job) { + return job.result() + .orElseThrow(() -> new AssertionError("A terminal job must carry its result")) + .metadata() + .orElseThrow(() -> new AssertionError("A terminal job must carry its metadata")); + } + + @SuppressWarnings("unchecked") + private static List> itemResults(final Map metadata) { + final Object results = metadata.get("results"); + assertNotNull(results, "The per-item array was requested and must be present"); + return ((List) results).stream() + .map(BulkRefreshResourceIntegrationTest::asMap) + .collect(Collectors.toList()); + } + + @SuppressWarnings("unchecked") + private static Map asMap(final Object item) { + if (item instanceof Map) { + return (Map) item; + } + return new com.fasterxml.jackson.databind.ObjectMapper().convertValue(item, Map.class); + } +} diff --git a/dotcms-postman/config.json b/dotcms-postman/config.json index dca5b14e7a41..089044e029d9 100644 --- a/dotcms-postman/config.json +++ b/dotcms-postman/config.json @@ -55,6 +55,7 @@ "name": "default-split", "collections": [ "ApiToken_Resource.postman_collection", + "BulkRefreshResource.postman_collection", "ContentImportResource.postman_collection", "Manifest_Download_End_Point.postman_collection", "Osgi.postman_collection", diff --git a/dotcms-postman/src/main/resources/postman/BulkRefreshResource.postman_collection.json b/dotcms-postman/src/main/resources/postman/BulkRefreshResource.postman_collection.json new file mode 100644 index 000000000000..c92d0715e34f --- /dev/null +++ b/dotcms-postman/src/main/resources/postman/BulkRefreshResource.postman_collection.json @@ -0,0 +1,370 @@ +{ + "info": { + "_postman_id": "8f2c1a44-6b3d-4f01-9a5c-2e7d4b9c1f30", + "name": "BulkRefreshResource", + "description": "Exercises POST /api/v1/content/_bulkrefresh over HTTP.\n\nCompletion is reported by push — a BULK_REFRESH_COMPLETED system event over the admin UI's websocket — so there is no status endpoint to poll and no completion assertion possible here. That is covered by BulkRefreshResourceIntegrationTest instead.\n\nThis endpoint is NOT a full index rebuild. POST /api/v1/esindex/reindex is a different operation and is not touched here.", + "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json" + }, + "auth": { + "type": "bearer", + "bearer": [ + { + "key": "token", + "value": "{{jwt}}", + "type": "string" + } + ] + }, + "event": [ + { + "listen": "prerequest", + "script": { + "type": "text/javascript", + "packages": {}, + "exec": [ + " ", + "if(!pm.collectionVariables.get('jwt')){", + " console.log(\"generating....\")", + " const serverURL = pm.environment.get('serverURL') || pm.collectionVariables.get('baseUrl'); // Get the server URL from the environment variable", + " const apiUrl = `${serverURL}/api/v1/apitoken`; // Construct the full API URL", + "", + " const username = pm.environment.get(\"user\") || pm.collectionVariables.get('user'); ", + " const password = pm.environment.get(\"password\") || pm.collectionVariables.get('password');", + " const basicAuth = Buffer.from(`${username}:${password}`).toString('base64');", + "", + " const requestOptions = {", + " url: apiUrl,", + " method: \"POST\",", + " header: {", + " \"accept\": \"*/*\",", + " \"content-type\": \"application/json\",", + " \"Authorization\": `Basic ${basicAuth}`", + " },", + " body: {", + " mode: \"raw\",", + " raw: JSON.stringify({", + " \"expirationSeconds\": 7200,", + " \"userId\": \"dotcms.org.1\",", + " \"network\": \"0.0.0.0/0\",", + " \"claims\": {\"label\": \"postman-tests\"}", + " })", + " }", + " };", + "", + " pm.sendRequest(requestOptions, function (err, response) {", + " if (err) {", + " console.log(err);", + " } else {", + " const jwt = response.json().entity.jwt;", + " pm.collectionVariables.set('jwt', jwt);", + " console.log(\"Successfully got a jwt :\" + jwt);", + " }", + " }); ", + "} " + ] + } + }, + { + "listen": "test", + "script": { + "type": "text/javascript", + "packages": {}, + "exec": [ + "" + ] + } + } + ], + "item": [ + { + "name": "pre-execution-scripts", + "description": "Finds a real contentlet to reindex, so the tests operate on live content.", + "item": [ + { + "name": "Find a Contentlet to Refresh", + "request": { + "method": "POST", + "header": [ + { + "key": "Content-Type", + "value": "application/json" + } + ], + "url": { + "raw": "{{serverURL}}/api/content/_search", + "host": [ + "{{serverURL}}" + ], + "path": [ + "api", + "content", + "_search" + ] + }, + "description": "Any working contentlet will do; the endpoint is not type-specific.", + "body": { + "mode": "raw", + "raw": "{\n \"query\": \"+baseType:1 +working:true +deleted:false\",\n \"limit\": 1,\n \"sort\": \"modDate desc\"\n}", + "options": { + "raw": { + "language": "json" + } + } + } + }, + "response": [], + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test('Search returns a contentlet to work with', function () {", + " pm.response.to.have.status(200);", + " const results = pm.response.json().entity.jsonObjectView.contentlets;", + " pm.expect(results).to.be.an('array').that.is.not.empty;", + " pm.collectionVariables.set('inode', results[0].inode);", + " pm.collectionVariables.set('identifier', results[0].identifier);", + "});" + ] + } + } + ] + } + ] + }, + { + "name": "Test Bulk Refresh Happy Path", + "item": [ + { + "name": "Submit Bulk Refresh Expect Accepted", + "request": { + "method": "POST", + "header": [ + { + "key": "Content-Type", + "value": "application/json" + } + ], + "url": { + "raw": "{{serverURL}}/api/v1/content/_bulkrefresh", + "host": [ + "{{serverURL}}" + ], + "path": [ + "api", + "v1", + "content", + "_bulkrefresh" + ] + }, + "description": "202 rather than 200: the work is accepted, not finished. A client must not be able to read this response as 'reindexed'.", + "body": { + "mode": "raw", + "raw": "{\n \"contentletIds\": [\"{{inode}}\"],\n \"includeItemResults\": true\n}", + "options": { + "raw": { + "language": "json" + } + } + } + }, + "response": [], + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test('Responds 202 Accepted', function () {", + " pm.response.to.have.status(202);", + "});", + "", + "pm.test('Hands back the accepted job id', function () {", + " const entity = pm.response.json().entity;", + " pm.expect(entity.jobId).to.be.a('string').and.not.empty;", + " pm.expect(entity.submitted).to.eql(1);", + "});" + ] + } + } + ] + } + ] + }, + { + "name": "Test Request Validations", + "item": [ + { + "name": "Submit Empty Selection Expect Bad Request", + "request": { + "method": "POST", + "header": [ + { + "key": "Content-Type", + "value": "application/json" + } + ], + "url": { + "raw": "{{serverURL}}/api/v1/content/_bulkrefresh", + "host": [ + "{{serverURL}}" + ], + "path": [ + "api", + "v1", + "content", + "_bulkrefresh" + ] + }, + "description": "An empty selection is a client bug, not a no-op job: enqueuing one would hand back a job id that can only ever report zero work.", + "body": { + "mode": "raw", + "raw": "{\n \"contentletIds\": []\n}", + "options": { + "raw": { + "language": "json" + } + } + } + }, + "response": [], + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test('Rejected with 400', function () {", + " pm.response.to.have.status(400);", + "});" + ] + } + } + ] + }, + { + "name": "Submit Without contentletIds Expect Bad Request", + "request": { + "method": "POST", + "header": [ + { + "key": "Content-Type", + "value": "application/json" + } + ], + "url": { + "raw": "{{serverURL}}/api/v1/content/_bulkrefresh", + "host": [ + "{{serverURL}}" + ], + "path": [ + "api", + "v1", + "content", + "_bulkrefresh" + ] + }, + "body": { + "mode": "raw", + "raw": "{\n \"includeItemResults\": true\n}", + "options": { + "raw": { + "language": "json" + } + } + } + }, + "response": [], + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test('Rejected with 400', function () {", + " pm.response.to.have.status(400);", + "});" + ] + } + } + ] + }, + { + "name": "Submit Over The Item Cap Expect Bad Request", + "request": { + "method": "POST", + "header": [ + { + "key": "Content-Type", + "value": "application/json" + } + ], + "url": { + "raw": "{{serverURL}}/api/v1/content/_bulkrefresh", + "host": [ + "{{serverURL}}" + ], + "path": [ + "api", + "v1", + "content", + "_bulkrefresh" + ] + }, + "description": "Synchronous indexing over an unbounded selection is a self-inflicted full reindex, which is the one thing this endpoint must never become.", + "body": { + "mode": "raw", + "raw": "{{oversizedSelection}}", + "options": { + "raw": { + "language": "json" + } + } + } + }, + "response": [], + "event": [ + { + "listen": "prerequest", + "script": { + "type": "text/javascript", + "exec": [ + "// One past the documented default cap of 500. If CONTENT_BULK_REFRESH_MAX_ITEMS is raised", + "// above this in an environment, this request stops being over the cap - and the test says so", + "// rather than failing mysteriously.", + "const ids = [];", + "for (let i = 0; i <= 500; i++) {", + " ids.push('00000000-0000-0000-0000-' + String(i).padStart(12, '0'));", + "}", + "pm.collectionVariables.set('oversizedSelection', JSON.stringify({contentletIds: ids}));" + ] + } + }, + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test('Rejected with 400', function () {", + " pm.response.to.have.status(400);", + "});" + ] + } + } + ] + } + ] + } + ], + "variable": [ + { + "key": "inode", + "value": "" + }, + { + "key": "oversizedSelection", + "value": "" + } + ] +}