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

[RNMobile] Add enableCaching param to fetch request on Android #31186

Merged
merged 7 commits into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -280,8 +280,9 @@ public void getOtherMediaOptions(ReadableArray filter, final Callback jsCallback
}

@ReactMethod
public void fetchRequest(String path, Promise promise) {
public void fetchRequest(String path, boolean enableCaching, Promise promise) {
mGutenbergBridgeJS2Parent.performRequest(path,
enableCaching,
promise::resolve,
errorBundle -> {
WritableMap writableMap = Arguments.makeNativeMap(errorBundle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
import androidx.core.util.Consumer;

public interface RequestExecutor {
void performRequest(String path, Consumer<String> onSuccess, Consumer<Bundle> onError);
void performRequest(String path, boolean enableCaching, Consumer<String> onSuccess, Consumer<Bundle> onError);
}
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,8 @@ public void requestMediaPickFrom(String mediaSource,
}

@Override
public void performRequest(String pathFromJS, Consumer<String> onSuccess, Consumer<Bundle> onError) {
mRequestExecutor.performRequest(pathFromJS, onSuccess, onError);
public void performRequest(String pathFromJS, boolean enableCaching, Consumer<String> onSuccess, Consumer<Bundle> onError) {
mRequestExecutor.performRequest(pathFromJS, enableCaching, onSuccess, onError);
}

@Override
Expand Down
5 changes: 4 additions & 1 deletion packages/react-native-bridge/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,10 @@ export function requestMediaEditor( mediaUrl, callback ) {
);
}

export function fetchRequest( path ) {
export function fetchRequest( path, enableCaching = true ) {
if ( isAndroid ) {
return RNReactNativeGutenbergBridge.fetchRequest( path, enableCaching );
}
Comment on lines +279 to +281
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request caching is only available on Android.

Copy link
Contributor

@jd-alexander jd-alexander May 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good approach since the bridge code on iOS does not contain enableCaching in the attached bridge code snippet and in the line below.

func fetchRequest(_ path: String, resolver: @escaping RCTPromiseResolveBlock, rejecter: @escaping RCTPromiseRejectBlock) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, actually, I was surprised that we have the caching feature only on Android 🤔 .

return RNReactNativeGutenbergBridge.fetchRequest( path );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public void editorDidEmitLog(String message, LogLevel logLevel) {
}

@Override
public void performRequest(String path, Consumer<String> onSuccess, Consumer<Bundle> onError) {}
public void performRequest(String path, boolean enableCaching, Consumer<String> onSuccess, Consumer<Bundle> onError) {}

@Override
public void gutenbergDidRequestUnsupportedBlockFallback(ReplaceUnsupportedBlockCallback replaceUnsupportedBlockCallback,
Expand Down
8 changes: 7 additions & 1 deletion packages/react-native-editor/src/api-fetch-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ const SUPPORTED_ENDPOINTS = [
/wp\/v2\/search\?.*/i,
];

// [ONLY ON ANDROID] The requests made to these endpoints won't be cached.
const DISABLED_CACHING_ENDPOINTS = [ /wp\/v2\/(blocks)\/?\d*?.*/i ];

const setTimeoutPromise = ( delay ) =>
new Promise( ( resolve ) => setTimeout( resolve, delay ) );

Expand All @@ -18,7 +21,7 @@ const fetchHandler = ( { path }, retries = 20, retryCount = 1 ) => {
return Promise.reject( `Unsupported path: ${ path }` );
}

const responsePromise = fetchRequest( path );
const responsePromise = fetchRequest( path, shouldEnableCaching( path ) );

const parseResponse = ( response ) => {
if ( typeof response === 'string' ) {
Expand All @@ -44,6 +47,9 @@ const fetchHandler = ( { path }, retries = 20, retryCount = 1 ) => {
export const isPathSupported = ( path ) =>
SUPPORTED_ENDPOINTS.some( ( pattern ) => pattern.test( path ) );

export const shouldEnableCaching = ( path ) =>
! DISABLED_CACHING_ENDPOINTS.some( ( pattern ) => pattern.test( path ) );

export default () => {
apiFetch.setFetchHandler( ( options ) => fetchHandler( options ) );
};
31 changes: 30 additions & 1 deletion packages/react-native-editor/src/test/api-fetch-setup.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Internal dependencies
*/
import { isPathSupported } from '../api-fetch-setup';
import { isPathSupported, shouldEnableCaching } from '../api-fetch-setup';

const supportedPaths = [
'wp/v2/media/54?context=edit&_locale=user',
Expand All @@ -11,12 +11,27 @@ const supportedPaths = [
'wp/v2/media?context=edit&_locale=user',
'wp/v2/categories/',
'wp/v2/blocks/28?_locale=user',
'/wp/v2/blocks?per_page=100&context=edit&_locale=user',
];

const unsupportedPaths = [
'wp/v1/media/', // made up example
];

const enabledCachingPaths = [
'wp/v2/media/54?context=edit&_locale=user',
'wp/v2/media/5?context=edit',
'wp/v2/media/54/',
'wp/v2/media/',
'wp/v2/media?context=edit&_locale=user',
'wp/v2/categories/',
];

const disabledCachingPaths = [
'wp/v2/blocks/28?_locale=user',
'/wp/v2/blocks?per_page=100&context=edit&_locale=user',
];

describe( 'isPathSupported', () => {
supportedPaths.forEach( ( path ) => {
it( `supports ${ path }`, () => {
Expand All @@ -30,3 +45,17 @@ describe( 'isPathSupported', () => {
} );
} );
} );

describe( 'shouldEnableCaching', () => {
enabledCachingPaths.forEach( ( path ) => {
it( `enables caching for ${ path }`, () => {
expect( shouldEnableCaching( path ) ).toBe( true );
} );
} );

disabledCachingPaths.forEach( ( path ) => {
it( `does not enable caching for ${ path }`, () => {
expect( shouldEnableCaching( path ) ).toBe( false );
} );
} );
} );