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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ export abstract class UmbContentDetailWorkspaceContextBase<

public async loadLanguages() {
// TODO: If we don't end up having a Global Context for languages, then we should at least change this into using a asObservable which should be returned from the repository. [Nl]
const { data } = await this.#languageRepository.requestCollection({});
const { data } = await this.#languageRepository.requestAllItems();
this.#languages.setValue(data?.items ?? []);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import { expect } from '@open-wc/testing';
import { fetchAllPages } from './fetch-all-pages.function.js';
import type { UmbDataSourceResponse, UmbPagedModel } from '@umbraco-cms/backoffice/repository';

interface TestItem {
id: number;
}

const buildFakeFetcher = (allItems: Array<TestItem>) => {
const calls: Array<{ skip: number; take: number }> = [];
const fetchPage = async (skip: number, take: number) => {
calls.push({ skip, take });
return { data: { items: allItems.slice(skip, skip + take), total: allItems.length } };
};
return { fetchPage, calls };
};

describe('fetchAllPages', () => {
it('returns all items in a single call when total <= take', async () => {
const all: Array<TestItem> = [{ id: 1 }, { id: 2 }];
const { fetchPage, calls } = buildFakeFetcher(all);

const { data } = await fetchAllPages(fetchPage, 10);

expect(data?.items).to.eql(all);
expect(data?.total).to.equal(2);
expect(calls).to.have.lengthOf(1);
expect(calls[0]).to.eql({ skip: 0, take: 10 });
});

it('pages through and returns all items when total > take', async () => {
const all: Array<TestItem> = [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }, { id: 5 }];
const { fetchPage, calls } = buildFakeFetcher(all);

const { data } = await fetchAllPages(fetchPage, 2);

expect(data?.items).to.eql(all);
expect(data?.total).to.equal(5);
expect(calls.map((c) => c.skip)).to.eql([0, 2, 4]);
});

it('makes no extra call when the last page exactly fills `take`', async () => {
const all: Array<TestItem> = [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }];
const { fetchPage, calls } = buildFakeFetcher(all);

const { data } = await fetchAllPages(fetchPage, 2);

expect(data?.items).to.eql(all);
expect(data?.total).to.equal(4);
expect(calls).to.have.lengthOf(2);
});

it('returns an empty result when there are no items', async () => {
const { fetchPage, calls } = buildFakeFetcher([]);

const { data } = await fetchAllPages(fetchPage, 100);

expect(data?.items).to.eql([]);
expect(data?.total).to.equal(0);
expect(calls).to.have.lengthOf(1);
});

it('returns the error and stops paging when a page fetch fails', async () => {
let callCount = 0;
const fetchPage = async (skip: number, take: number) => {
callCount++;
if (callCount === 1) {
return { data: { items: [{ id: 1 }, { id: 2 }] as Array<TestItem>, total: 100 } };
}
return { error: new Error('boom') };
};

const { data, error } = await fetchAllPages<TestItem>(fetchPage, 2);

expect(data).to.be.undefined;
expect(error).to.exist;
expect(callCount).to.equal(2);
});

it('returns a synthesised error when the fetcher returns neither data nor error', async () => {
const fetchPage = async () => ({}) as UmbDataSourceResponse<UmbPagedModel<TestItem>>;

const { data, error } = await fetchAllPages<TestItem>(fetchPage, 2);

expect(data).to.be.undefined;
expect(error).to.be.an.instanceOf(Error);
});

it('rejects when `take` is not a positive finite number', async () => {
const { fetchPage } = buildFakeFetcher([{ id: 1 }, { id: 2 }]);

for (const invalid of [0, -1, NaN, Number.POSITIVE_INFINITY]) {
let thrown: unknown;
try {
await fetchAllPages(fetchPage, invalid);
} catch (e) {
thrown = e;
}
expect(thrown, `take=${invalid}`).to.be.an.instanceOf(RangeError);
}
});

it('returns an error if the server delivers an empty page before reaching the reported total', async () => {
// Surfaces (rather than masks) a server that reports more items than it returns. Also guards against
// an infinite loop if `total` and the actual items disagree.
let callCount = 0;
const fetchPage = async (skip: number, take: number) => {
callCount++;
if (callCount === 1) {
return { data: { items: [{ id: 1 }, { id: 2 }] as Array<TestItem>, total: 10 } };
}
return { data: { items: [] as Array<TestItem>, total: 10 } };
};

const { data, error } = await fetchAllPages<TestItem>(fetchPage, 2);

expect(data).to.be.undefined;
expect(error).to.be.an.instanceOf(Error);
expect(callCount).to.equal(2);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import type { UmbDataSourceResponse, UmbPagedModel } from '@umbraco-cms/backoffice/repository';

/**
* A function that returns a single page of an offset-paginated collection.
* @template T - The type of items in the page.
*/
export type UmbOffsetPageFetcher<T> = (
skip: number,
take: number,
) => Promise<UmbDataSourceResponse<UmbPagedModel<T>>>;

/**
* Pages through an offset-paginated data source, accumulating every item until `total` has been reached.
* Use when a caller genuinely needs the full set rather than a single page — for example, populating a
* dropdown of every configured language. Returns the same `{ data: { items, total } }` shape as a
* single-page fetch, or `{ error }` if any page fails.
*
* If the server reports a higher `total` than it actually delivers — i.e. an empty page is returned
* before `allItems.length` reaches `total` — the function fails with an error rather than silently
* truncating, since a partial "fetch all" result would mislead the caller.
* @param {UmbOffsetPageFetcher} fetchPage - Called once per page with the current `skip` and `take`.
* @param {number} take - Page size used for every request. Must be a positive finite number.
* @returns {Promise} A promise resolving to all items, or the first error encountered.
* @throws {RangeError} If `take` is not a positive finite number.
*/
export async function fetchAllPages<T>(
fetchPage: UmbOffsetPageFetcher<T>,
take: number,
): Promise<UmbDataSourceResponse<UmbPagedModel<T>>> {
if (!Number.isFinite(take) || take <= 0) {
throw new RangeError(`fetchAllPages: \`take\` must be a positive finite number, got ${take}.`);
}

const allItems: Array<T> = [];
let skip = 0;
let total = Number.POSITIVE_INFINITY;

while (allItems.length < total) {
const { data, error } = await fetchPage(skip, take);
if (error) return { error };
if (!data) return { error: new Error('fetchAllPages: page fetcher returned neither data nor error.') };

// If the server reports more items than it delivers, fail rather than silently truncating —
// also guards against an infinite loop on a misbehaving source.
if (data.items.length === 0 && allItems.length < data.total) {
return {
error: new Error(
`fetchAllPages: page fetcher returned an empty page after ${allItems.length} items but reported a total of ${data.total}.`,
),
};
}

allItems.push(...data.items);
total = data.total;
skip += data.items.length;
}

return { data: { items: allItems, total: allItems.length } };
}

Check warning on line 59 in src/Umbraco.Web.UI.Client/src/packages/core/utils/pagination/offset/fetch-all-pages.function.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Complex Method

fetchAllPages has a cyclomatic complexity of 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
Comment thread
AndyButland marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './fetch-all-pages.function.js';
export * from './is-offset-request.guard.js';
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class UmbDictionaryTableCollectionViewElement extends UmbLitElement {
async #observeCollectionItems() {
if (!this.#collectionContext) return;

const { data: languageData } = await this.#languageCollectionRepository.requestCollection({});
const { data: languageData } = await this.#languageCollectionRepository.requestAllItems();
if (!languageData) return;

this.observe(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class UmbWorkspaceViewDictionaryEditorElement extends UmbLitElement {
}

override async firstUpdated() {
const { data } = await this.#languageCollectionRepository.requestCollection({});
const { data } = await this.#languageCollectionRepository.requestAllItems();
if (data) {
this._languages = data.items;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class UmbCultureAndHostnamesModalElement extends UmbModalBaseElement<
}

async #requestLanguages() {
const { data } = await this.#languageCollectionRepository.requestCollection({ take: 999 });
const { data } = await this.#languageCollectionRepository.requestAllItems();
// Set to empty array if no data, to indicate loading is complete
this._languageModel = data?.items ?? [];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class UmbPublishDocumentEntityAction extends UmbEntityActionBase<never> {
const localize = new UmbLocalizationController(this);

const languageRepository = new UmbLanguageCollectionRepository(this._host);
const { data: languageData } = await languageRepository.requestCollection({});
const { data: languageData } = await languageRepository.requestAllItems();

const documentRepository = new UmbDocumentDetailRepository(this._host);
const { data: documentData } = await documentRepository.requestByUnique(this.args.unique);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export class UmbDocumentPublishEntityBulkAction extends UmbEntityBulkActionBase<

const [{ data: documentItems }, { data: languageData }] = await Promise.all([
itemRepository.requestItems(this.selection),
languageRepository.requestCollection({}),
languageRepository.requestAllItems(),
]);

if (!documentItems?.length) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class UmbUnpublishDocumentEntityAction extends UmbEntityActionBase<never>
const localize = new UmbLocalizationController(this);

const languageRepository = new UmbLanguageCollectionRepository(this._host);
const { data: languageData } = await languageRepository.requestCollection({});
const { data: languageData } = await languageRepository.requestAllItems();

const documentRepository = new UmbDocumentDetailRepository(this._host);
const { data: documentData } = await documentRepository.requestByUnique(this.args.unique);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class UmbDocumentUnpublishEntityBulkAction extends UmbEntityBulkActionBas

const [{ data: documentItems }, { data: languageData }] = await Promise.all([
itemRepository.requestItems(this.selection),
languageRepository.requestCollection({}),
languageRepository.requestAllItems(),
]);

if (!documentItems?.length) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class UmbAppLanguageSelectElement extends UmbLitElement {
}

async #observeLanguages() {
const { data } = await this.#collectionRepository.requestCollection({});
const { data } = await this.#collectionRepository.requestAllItems();

// TODO: listen to changes
if (data) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import type { UmbLanguageCollectionFilterModel } from '../types.js';
import type { UmbLanguageDetailModel } from '../../types.js';
import { UmbLanguageCollectionServerDataSource } from './language-collection.server.data-source.js';
import type { UmbLanguageCollectionDataSource } from './types.js';
import { UmbRepositoryBase } from '@umbraco-cms/backoffice/repository';
import type { UmbCollectionRepository } from '@umbraco-cms/backoffice/collection';
import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api';
import { fetchAllPages } from '@umbraco-cms/backoffice/utils';

// Mirrors the server's default page size for `GET /language` — chosen so the underlying request matches
// the unconfigured server contract.
const LANGUAGE_PAGE_SIZE = 100;

export class UmbLanguageCollectionRepository extends UmbRepositoryBase implements UmbCollectionRepository {
#collectionSource: UmbLanguageCollectionDataSource;
Expand All @@ -16,6 +22,19 @@ export class UmbLanguageCollectionRepository extends UmbRepositoryBase implement
async requestCollection(filter: UmbLanguageCollectionFilterModel) {
return this.#collectionSource.getCollection(filter);
}

/**
* Requests all languages by paging through the collection until every item has been retrieved.
* Use this in preference to `requestCollection` when callers need the full set — the server defaults
* `take` to 100, so a single un-paged request would silently truncate installations with more languages.
* @returns {Promise} A promise resolving to `{ data: { items, total } }` containing every language, or `{ error }`.
*/
async requestAllItems() {
return fetchAllPages<UmbLanguageDetailModel>(
(skip, take) => this.#collectionSource.getCollection({ skip, take }),
LANGUAGE_PAGE_SIZE,
);
}
}

export default UmbLanguageCollectionRepository;
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export class UmbAppLanguageContext extends UmbContextBase implements UmbApi {
}

async #requestLanguages() {
const { data } = await this.#languageCollectionRepository.requestCollection({});
const { data } = await this.#languageCollectionRepository.requestAllItems();

// TODO: make this observable / update when languages are added/removed/updated
if (data) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class UmbLanguagePickerModalElement extends UmbModalBaseElement<
}

override async firstUpdated() {
const { data } = await this.#collectionRepository.requestCollection({});
const { data } = await this.#collectionRepository.requestAllItems();
this._languages = data?.items ?? [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class UmbDocumentLinkPickerContext extends UmbPickerContext {
}

async #loadLanguages() {
const { data } = await this.#languageCollectionRepository.requestCollection({ skip: 0, take: 1000 });
const { data } = await this.#languageCollectionRepository.requestAllItems();
const languages = data?.items || [];

this.#languages.setValue(languages);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export class UmbPreviewCultureElement extends UmbLitElement {
}

async #loadCultures() {
const { data: langauges } = await this.#languageRepository.requestCollection({ skip: 0, take: 100 });
this._cultures = langauges?.items ?? [];
const { data: languages } = await this.#languageRepository.requestAllItems();
this._cultures = languages?.items ?? [];

const searchParams = new URLSearchParams(window.location.search);
const culture = searchParams.get('culture');
Expand Down
Loading