Merge pull request #10 from johngodley/fix/2.0.1

Fix 2.0.1 regex performance

api/api-search.php

@@ -290,6 +290,11 @@ private function get_paging_params() {
 					'backward',
 				],
 			],
+			'limit' => [
+				'description' => 'Maximum number of results to return',
+				'type' => 'integer',
+				'default' => 0,
+			],
 		];
 	}
 
@@ -399,7 +404,7 @@ public function search( WP_REST_Request $request ) {
 
 		list( $search, $replacer ) = $this->get_search_replace( $params, $params['replacement'] );
 
-		$results = $search->get_results( $replacer, $params['page'], $params['perPage'] );
+		$results = $search->get_results( $replacer, $params['page'], $params['perPage'], $params['limit'] );
 		if ( ! is_wp_error( $results ) ) {
 			$results['results'] = $search->results_to_json( $results['results'] );
 		}
@@ -602,14 +607,19 @@ public function validate_source_flags( $value, WP_REST_Request $request, $param
 			// Get the sanitized flags from all the sources
 			$allowed = [];
 			foreach ( $sources as $source ) {
-				$allowed = array_merge( $allowed, $source->get_source_flags()->get_flags() );
+				$allowed = array_merge( $allowed, array_keys( $source->get_supported_flags() ) );
 			}
 
 			// Make it unique, as some sources can use the same flag
-			$allowed = array_unique( $allowed );
+			$allowed = array_values( array_unique( $allowed ) );
+
+			// Filter the value by this allowed list
+			$filtered_value = array_filter( $value, function( $item ) use ( $allowed ) {
+				return in_array( $item, $allowed, true );
+			} );
 
 			// Any flags missing?
-			if ( count( $allowed ) === count( $value ) ) {
+			if ( count( $filtered_value ) === count( $value ) ) {
 				return true;
 			}
 		}

client/lib/api/index.js

@@ -148,7 +148,8 @@ const getErrorMessage = json => {
 		return json.message;
 	}
 
-	return 'Unknown error ' + json;
+	console.error( json );
+	return 'Unknown error ' + ( typeof json === 'object' ? Object.keys( json ) : json );
 };
 
 const getErrorCode = json => {

client/page/search-replace/pagination/simple-pagination.js

@@ -16,8 +16,8 @@ import { search } from 'state/search/action';
 function SimplePagination( props ) {
 	const { progress, onChangePage, isLoading, matchedPhrases, matchedRows, perPage, search, noTotal = false } = props;
 	const { current, previous, next } = progress;
-	const totalPages = Math.round( matchedRows / perPage );
-	const currentPage = Math.round( current / perPage );
+	const totalPages = Math.ceil( matchedRows / perPage );
+	const currentPage = Math.ceil( current / perPage ) + 1;
 	const hasNext = next && currentPage < totalPages;
 
 	return (
@@ -40,8 +40,8 @@ function SimplePagination( props ) {
 				<span className="tablenav-paging-text">
 					{ __( 'Page %(current)s of %(total)s', {
 						args: {
-							current: numberFormat( currentPage + 1 ),
-							total: numberFormat( Math.max( 1, totalPages + 1 ) ),
+							current: numberFormat( currentPage ),
+							total: numberFormat( totalPages ),
 						},
 					} ) }
 				</span>

client/page/search-replace/search-results/index.js

@@ -16,26 +16,31 @@ import TableLoading from './table-loading';
 import EmptyResults from './empty-results';
 import Pagination from '../pagination';
 import { STATUS_IN_PROGRESS } from 'state/settings/type';
-import { searchMore, setError } from 'state/search/action';
+import { searchMore, setError, cancel } from 'state/search/action';
 import { SEARCH_FORWARD, SEARCH_BACKWARD } from 'state/search/type';
 import './style.scss';
 
 const hasMoreResults = ( searchDirection, progress ) => ( searchDirection === SEARCH_FORWARD && progress.next !== false ) || ( searchDirection === SEARCH_BACKWARD && progress.previous !== false );
-const shouldLoadMore = ( status, requestCount, results, perPage ) => status === STATUS_IN_PROGRESS && requestCount > 0 && results.length !== perPage;
+const shouldLoadMore = ( status, requestCount, results, perPage ) => status === STATUS_IN_PROGRESS && requestCount > 0 && results.length < perPage;
 
-const MAX_REQUESTS = 100;
+const MAX_REQUESTS = 500;
+const SEARCH_MORE_DELAY = 450;
 
 function SearchResults( props ) {
-	const { results, totals, progress, status, requestCount, search, searchDirection, showLoading } = props;
-	const { searchFlags, perPage } = search;
+	const { results, totals, progress, status, requestCount, search, searchDirection, showLoading, onCancel } = props;
+	const { perPage, searchFlags } = search;
 	const { onSearchMore, onChangePage, onSetError } = props;
 	const isLoading = status === STATUS_IN_PROGRESS;
 
 	useEffect( () => {
 		if ( requestCount > MAX_REQUESTS ) {
 			onSetError( __( 'Maximum number of page requests has been exceeded and the search stopped. Try to be more specific with your search term.' ) );
-		} else if ( Object.keys( searchFlags ).length > 0 && shouldLoadMore( status, requestCount, results, perPage ) && hasMoreResults( searchDirection, progress ) ) {
-			onSearchMore( search, searchDirection === SEARCH_FORWARD ? progress.next : progress.previous );
+		} else if ( searchFlags.regex && shouldLoadMore( status, requestCount, results, perPage ) && hasMoreResults( searchDirection, progress ) ) {
+			setTimeout( () => {
+				onSearchMore( search, searchDirection === SEARCH_FORWARD ? progress.next : progress.previous, perPage - results.length );
+			}, SEARCH_MORE_DELAY );
+		} else if ( searchFlags.regex && ! hasMoreResults( searchDirection, progress ) ) {
+			onCancel();
 		}
 	}, [ requestCount ] );
 
@@ -102,12 +107,15 @@ function mapStateToProps( state ) {
 
 function mapDispatchToProps( dispatch ) {
 	return {
-		onSearchMore: ( searchValue, page ) => {
-			dispatch( searchMore( searchValue, page ) );
+		onSearchMore: ( searchValue, page, limit ) => {
+			dispatch( searchMore( searchValue, page, limit ) );
 		},
 		onSetError: ( error ) => {
 			dispatch( setError( error ) );
 		},
+		onCancel: () => {
+			dispatch( cancel() );
+		},
 	};
 }
 

client/state/search/action.js

@@ -21,7 +21,7 @@ import {
 
 import { getApi, SearchRegexApi } from 'lib/api';
 
-function getSearchValues( values ) {
+function getSearchValues( values, sources ) {
 	return {
 		...values,
 		searchFlags: Object.keys( values.searchFlags ),
@@ -31,8 +31,8 @@ function getSearchValues( values ) {
 
 export const setSearch = ( searchValue ) => ( { type: SEARCH_VALUES, searchValue } );
 
-export const search = ( search, page, searchDirection = SEARCH_FORWARD ) => ( dispatch ) => {
-	const searchValues = { ...getSearchValues( search ), page, searchDirection };
+export const search = ( search, page, searchDirection = SEARCH_FORWARD ) => ( dispatch, getState ) => {
+	const searchValues = { ...getSearchValues( search, getState().search.sources ), page, searchDirection };
 
 	getApi( SearchRegexApi.search.get( searchValues ) )
 		.then( json => {
@@ -45,8 +45,13 @@ export const search = ( search, page, searchDirection = SEARCH_FORWARD ) => ( di
 	return dispatch( { type: SEARCH_START_FRESH, ...searchValues } );
 };
 
-export const searchMore = ( search, page ) => ( dispatch, getState ) => {
-	const searchValues = { ...getSearchValues( search ), page, searchDirection: getState().search.searchDirection };
+export const searchMore = ( search, page, limit ) => ( dispatch, getState ) => {
+	const searchValues = {
+		...getSearchValues( search, getState().search.sources ),
+		page,
+		searchDirection: getState().search.searchDirection,
+		limit,
+	};
 
 	getApi( SearchRegexApi.search.get( searchValues ) )
 		.then( json => {
@@ -59,13 +64,17 @@ export const searchMore = ( search, page ) => ( dispatch, getState ) => {
 	return dispatch( { type: SEARCH_START_MORE, ...searchValues } );
 }
 
-export const setError = ( error ) => ( { type: SEARCH_FAIL, error } );
+export const setError = ( error ) => ( { type: SEARCH_FAIL, error: { message: error } } );
 export const cancel = () => ( { type: SEARCH_CANCEL, clearAll: false } );
 export const clear = () => ( { type: SEARCH_CANCEL, clearAll: true } );
 
 export const replaceRow = ( replacement, rowId, columnId = null, posId = null ) => ( dispatch, getState ) => {
 	const { search } = getState().search;
-	const replace = { ...search, rowId, replacePhrase: replacement, searchFlags: Object.keys( search.searchFlags ), sourceFlags: Object.keys( search.sourceFlags ) };
+	const replace = {
+		...getSearchValues( search, getState().search.sources ),
+		rowId,
+		replacePhrase: replacement,
+	};
 
 	if ( columnId ) {
 		replace.columnId = columnId;
@@ -87,7 +96,14 @@ export const replaceRow = ( replacement, rowId, columnId = null, posId = null )
 };
 
 export const replaceAll = ( search, page, perPage ) => ( dispatch, getState ) => {
-	getApi( SearchRegexApi.search.replace( { ...search, replacePhrase: search.replacement, page, perPage, searchFlags: Object.keys( search.searchFlags ) } ) )
+	const replace = {
+		...getSearchValues( search, getState().search.sources ),
+		replacePhrase: search.replacement,
+		page,
+		perPage,
+	};
+
+	getApi( SearchRegexApi.search.replace( replace ) )
 		.then( json => {
 			dispatch( { type: SEARCH_REPLACE_ALL_COMPLETE, ...json } );
 		} )
@@ -99,7 +115,12 @@ export const replaceAll = ( search, page, perPage ) => ( dispatch, getState ) =>
 };
 
 export const replaceNext = ( page ) => ( dispatch, getState ) => {
-	const searchValues = { ...getSearchValues( getState().search.search ), page };
+	const { search } = getState().search;
+	const searchValues = {
+		...getSearchValues( getState().search.search, getState().search.sources ),
+		replacePhrase: search.replacement,
+		page,
+	};
 
 	getApi( SearchRegexApi.search.replace( searchValues ) )
 		.then( json => {

client/state/search/reducer.js

@@ -31,7 +31,7 @@ function mergeProgress( existing, progress, direction, firstInSet ) {
 	};
 }
 
-const isComplete = ( action, results, direction ) => ( direction === SEARCH_FORWARD && action.progress.next === false ) || ( direction === SEARCH_BACKWARD && action.progress.previous === false ) || results.length === action.perPage;
+const isComplete = ( action, results, direction ) => ( direction === SEARCH_FORWARD && action.progress.next === false ) || ( direction === SEARCH_BACKWARD && action.progress.previous === false ) || results.length >= action.perPage;
 const isAdvancedSearch = ( search ) => search.searchFlags.regex;
 
 function getSimpleState( state, action ) {
@@ -48,16 +48,17 @@ function getSimpleState( state, action ) {
 
 function getAdvancedState( state, action ) {
 	const results = state.searchDirection === SEARCH_FORWARD ? state.results.concat( action.results ) : action.results.concat( state.results );
+	const status = isComplete( action, results, state.searchDirection ) ? STATUS_COMPLETE : state.status;
 
 	return {
 		...state,
-		status: isComplete( action, results, state.searchDirection ) ? STATUS_COMPLETE : state.status,
+		status,
 		results,
 		requestCount: state.requestCount + 1,
 		progress: mergeProgress( state.progress, action.progress, state.searchDirection, state.requestCount === 0 ),
 		totals: { ...state.totals, ...action.totals },
-		canCancel: false,
-		showLoading: false,
+		canCancel: status !== STATUS_COMPLETE,
+		showLoading: status !== STATUS_COMPLETE,
 	};
 }
 
@@ -76,7 +77,7 @@ const reset = () => ( {
 	searchedPhrase: '',
 	replaceCount: 0,
 	phraseCount: 0,
-	status: null,
+	status: STATUS_COMPLETE,
 	replacing: [],
 	replaceAll: false,
 	row: null,
@@ -132,6 +133,10 @@ export default function redirects( state = {}, action ) {
 			};
 
 		case SEARCH_START_MORE:
+			if ( isAlreadyFinished( state ) ) {
+				return state;
+			}
+
 			return {
 				...state,
 				canCancel: true,

gulpfile.js

@@ -212,7 +212,7 @@ function potGenerate() {
 		bugReport: 'https://wordpress.org/plugins/search-regex/',
 	};
 
-	return src( [ '**/*.php' ] )
+	return src( [ './*.php', 'models/*.php', 'source/*.php', 'api/*.php' ] )
 		.pipe( sort() )
 		.pipe( wpPot( pot ) )
 		.pipe( dest( 'locale/search-regex.pot' ) );