Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PCH: Enhance Request Handling in Provider Classes #2401

Merged
merged 21 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
a77b2c7
Create BaseProvider class and apply it to SmartLinkingProvider
vaurdan Apr 18, 2024
2411c8f
Apply new BaseProvider to PerformanceStats provider
vaurdan Apr 18, 2024
2a4c932
Apply new BaseProvider to RelatedPostsProvider
vaurdan Apr 18, 2024
de917f4
Apply new BaseProvider to TitleSuggestionsProvider
vaurdan Apr 18, 2024
b0cf576
Apply new BaseProvider to DashboardWidgetProvider
vaurdan Apr 18, 2024
a6b8289
Refactor BaseProvider to use non-static methods instead.
vaurdan Apr 18, 2024
c12338a
Clean-up abort controller after each request.
vaurdan Apr 18, 2024
e3ff446
Remove testing code
vaurdan Apr 18, 2024
6b756a8
Merge branch 'refs/heads/develop' into update/ch-provider-refactor
vaurdan Apr 18, 2024
69d6433
Rebuild assets
vaurdan Apr 18, 2024
e9d8c5f
Adjust some whitespace/comments
acicovic Apr 19, 2024
b0f7efc
Merge branch 'develop' into update/ch-provider-refactor
acicovic Apr 22, 2024
077d0a2
build(deps-dev): bump @testing-library/react from 15.0.2 to 15.0.4
dependabot[bot] Apr 24, 2024
4f11587
PCH Editor Sidebar: Update isDismissible in Notices
acicovic Apr 24, 2024
b5df10a
TypeScript DocBlocks: Remove spaces in braces
acicovic Apr 19, 2024
8a27528
Remove unnecessary empty line
acicovic Apr 24, 2024
9396e93
Add JSDoc comment to `GetAbortControllerResult`
vaurdan Apr 24, 2024
141dbe6
Merge branch 'develop' into update/ch-provider-refactor
vaurdan Apr 24, 2024
8a79fa6
Adjust some whitespace/comments
acicovic Apr 19, 2024
8a9266e
Clean-up docblock merge mess
vaurdan Apr 24, 2024
facd64b
Remove extra spaces.
vaurdan Apr 24, 2024
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
2 changes: 1 addition & 1 deletion build/content-helper/dashboard-widget.asset.php
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<?php return array('dependencies' => array('react', 'wp-api-fetch', 'wp-components', 'wp-data', 'wp-element', 'wp-i18n', 'wp-url'), 'version' => '58c71232f1e12c0b0925');
<?php return array('dependencies' => array('react', 'wp-api-fetch', 'wp-components', 'wp-data', 'wp-element', 'wp-i18n', 'wp-url'), 'version' => 'a9ac73db3b76299e492d');
2 changes: 1 addition & 1 deletion build/content-helper/dashboard-widget.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion build/content-helper/editor-sidebar.asset.php
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<?php return array('dependencies' => array('react', 'wp-api-fetch', 'wp-block-editor', 'wp-components', 'wp-compose', 'wp-core-data', 'wp-data', 'wp-edit-post', 'wp-editor', 'wp-element', 'wp-hooks', 'wp-i18n', 'wp-plugins', 'wp-primitives', 'wp-url'), 'version' => '7cda9ce01084868d6502');
<?php return array('dependencies' => array('react', 'wp-api-fetch', 'wp-block-editor', 'wp-components', 'wp-compose', 'wp-core-data', 'wp-data', 'wp-edit-post', 'wp-editor', 'wp-element', 'wp-hooks', 'wp-i18n', 'wp-plugins', 'wp-primitives', 'wp-url'), 'version' => 'a522b73f43053e03c73b');
34 changes: 17 additions & 17 deletions build/content-helper/editor-sidebar.js

Large diffs are not rendered by default.

168 changes: 168 additions & 0 deletions src/content-helper/common/base-provider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
/**
* WordPress dependencies
*/
// eslint-disable-next-line import/named
import apiFetch, { APIFetchOptions } from '@wordpress/api-fetch';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import { ContentHelperError, ContentHelperErrorCode } from './content-helper-error';

/**
* The response structure of the API.
*
* @since 3.15.0
*/
export interface ContentHelperAPIResponse<T> {
error?: Error;
data: T;
}

/**
* The result of the getOrCreateController method.
*
* @since 3.15.0
*/
type GetAbortControllerResult = {
acicovic marked this conversation as resolved.
Show resolved Hide resolved
abortController: AbortController;
abortId: string;
};

/**
* Base class for all providers.
*
* Provides a common interface for fetching data from the API, with support
* for cancelling requests.
*
* @since 3.15.0
*/
export abstract class BaseProvider {
/**
* A map of AbortControllers used to cancel fetch requests.
*
* @since 3.15.0
*/
private abortControllers: Map<string, AbortController> = new Map();

/**
* Protected empty constructor to prevent instantiation.
*
* @since 3.15.0
*/
protected constructor() {} // eslint-disable-line no-useless-constructor

/**
* Cancels the fetch request.
*
* If an ID is provided, it cancels the request with that ID.
* If no ID is provided, it cancels the most recent request.
*
* @since 3.15.0
*
* @param {string?} id The (optional) ID of the request to cancel.
*/
public cancelRequest( id?: string ): void {
// If an ID is provided, cancel the request with that ID.
if ( id ) {
const controller = this.abortControllers.get( id );
if ( controller ) {
controller.abort();
this.abortControllers.delete( id );
}
return;
}

// Otherwise, cancel the most recent request.
const lastKey = Array.from( this.abortControllers.keys() ).pop();
if ( lastKey ) {
const controller = this.abortControllers.get( lastKey );
if ( controller ) {
controller.abort();
this.abortControllers.delete( lastKey );
}
}
}

/**
* Cancels all fetch requests for the provider.
*
* @since 3.15.0
*/
public cancelAll(): void {
this.abortControllers.forEach( ( controller ) => controller.abort() );
this.abortControllers.clear();
}

/**
* Private method to manage creating and storing AbortControllers.
*
* @param {string?} id The (optional) ID of the request.
*
* @return {GetAbortControllerResult} The AbortController and its ID.
*/
private getOrCreateController( id?: string ): GetAbortControllerResult {
if ( id && this.abortControllers.has( id ) ) {
return {
abortController: this.abortControllers.get( id )!,
abortId: id,
};
}

// If no ID is provided, generate one.
const abortId = id ?? 'auto-' + Date.now();
// Create a new AbortController.
const controller = new AbortController();
// Store the AbortController.
this.abortControllers.set( abortId, controller );

return {
abortController: controller,
abortId,
};
}

/**
* Fetches data from the API. Either resolves with the data or rejects with an error.
*
* This method is a wrapper around apiFetch() that automatically adds the AbortController signal.
*
* @param {APIFetchOptions} options The options to pass to apiFetch
* @param {string?} id The (optional) ID of the request
*
* @return {Promise<ContentHelperAPIResponse<any>>} The fetched data
*/
protected async fetch<T>( options: APIFetchOptions, id?: string ): Promise<T> {
const { abortController, abortId } = this.getOrCreateController( id );
options.signal = abortController.signal;
try {
const response = await apiFetch<ContentHelperAPIResponse<T>>( options );

// Validate API side errors.
if ( response.error ) {
return Promise.reject(
new ContentHelperError(
response.error.message,
ContentHelperErrorCode.ParselyApiResponseContainsError,
),
);
}

return response.data;
} catch ( wpError: any ) { // eslint-disable-line @typescript-eslint/no-explicit-any
if ( wpError.name === 'AbortError' ) {
return Promise.reject(
new ContentHelperError(
__( 'The operation was aborted.', 'wp-parsely' ),
ContentHelperErrorCode.ParselyAborted,
),
);
}
return Promise.reject( new ContentHelperError( wpError.message, wpError.code ) );
} finally {
// Clean-up the AbortController after a successful request.
this.abortControllers.delete( abortId );
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function TopPosts(): JSX.Element {
* @since 3.7.0
*/
useEffect( () => {
const provider = new DashboardWidgetProvider();
const provider = DashboardWidgetProvider.getInstance();

const fetchPosts = async ( retries: number ) => {
provider.getTopPosts( settings, page )
Expand Down
70 changes: 34 additions & 36 deletions src/content-helper/dashboard-widget/provider.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/**
* WordPress dependencies
*/
import apiFetch from '@wordpress/api-fetch';
import { __ } from '@wordpress/i18n';
import { addQueryArgs } from '@wordpress/url';

/**
* Internal dependencies
*/
import { BaseProvider } from '../common/base-provider';
import {
ContentHelperError,
ContentHelperErrorCode,
Expand All @@ -16,18 +16,31 @@ import { getApiPeriodParams } from '../common/utils/api';
import { PostData } from '../common/utils/post';
import { TopPostsSettings } from '../common/settings';

/**
* The form of the response returned by the /stats/posts WordPress REST API
* endpoint.
*/
interface TopPostsApiResponse {
error?: Error;
data?: PostData[];
}

export const TOP_POSTS_DEFAULT_LIMIT = 5;

export class DashboardWidgetProvider {
export class DashboardWidgetProvider extends BaseProvider {
/**
* The singleton instance of the DashboardWidgetProvider.
*
* @since 3.15.0
*/
private static instance: DashboardWidgetProvider;

/**
* Returns the singleton instance of the DashboardWidgetProvider.
*
* @since 3.15.0
*
* @return {DashboardWidgetProvider} The singleton instance.
*/
public static getInstance(): DashboardWidgetProvider {
if ( ! this.instance ) {
this.instance = new DashboardWidgetProvider();
}

return this.instance;
}

/**
* Returns the site's top posts.
*
Expand Down Expand Up @@ -69,31 +82,16 @@ export class DashboardWidgetProvider {
private async fetchTopPostsFromWpEndpoint(
settings: TopPostsSettings, page: number
): Promise<PostData[]> {
let response;

try {
response = await apiFetch( {
path: addQueryArgs( '/wp-parsely/v1/stats/posts/', {
limit: TOP_POSTS_DEFAULT_LIMIT,
...getApiPeriodParams( settings.Period ),
sort: settings.Metric,
page,
itm_source: 'wp-parsely-content-helper',
} ),
} ) as TopPostsApiResponse;
} catch ( wpError: any ) { // eslint-disable-line @typescript-eslint/no-explicit-any
return Promise.reject( new ContentHelperError(
wpError.message, wpError.code
) );
}

if ( response?.error ) {
return Promise.reject( new ContentHelperError(
response.error.message,
ContentHelperErrorCode.ParselyApiResponseContainsError
) );
}
const response = this.fetch<PostData[]>( {
path: addQueryArgs( '/wp-parsely/v1/stats/posts/', {
limit: TOP_POSTS_DEFAULT_LIMIT,
...getApiPeriodParams( settings.Period ),
sort: settings.Metric,
page,
itm_source: 'wp-parsely-content-helper',
} ),
} );

return response?.data ?? [];
return response ?? [];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export const PerformanceStats = (
const { settings, setSettings } = useSettings<SidebarSettings>();

useEffect( () => {
const provider = new PerformanceStatsProvider();
const provider = PerformanceStatsProvider.getInstance();

const fetchPosts = async ( retries: number ) => {
provider.getPerformanceStats( period )
Expand Down
Loading