Skip to content

Commit 885bf35

Browse files
dpaloudpalou
authored
Merge pull request #4710 from crazyserver/MOBILE-4948
Mobile 4948 Test on non pure pipes
2 parents a6522f1 + f8e3845 commit 885bf35

File tree

19 files changed

+707
-162
lines changed

19 files changed

+707
-162
lines changed

‎.github/workflows/testing.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ jobs:
1919
run: |
2020
npm ci --no-audit
2121
npm ci --no-audit --prefix cordova-plugin-moodleapp
22+
- name: Install languages
23+
run: |
24+
npx gulp lang
25+
npm run lang:update-langpacks
2226
- name: Check langindex
2327
run: |
2428
result=$(cat scripts/langindex.json | grep \"TBD\" | wc -l); test $result -eq 0

‎src/core/classes/result-memoiser.ts‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// (C) Copyright 2015 Moodle Pty Ltd.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import { CoreObject } from '@static/object';
16+
17+
/**
18+
* Class to memoise the result of some calculation to avoid unnecessary recalculations.
19+
* It's particularly useful for impure pipes.
20+
* It's recommended to use basic types as parameters to optimise performance, but objects can be used as well.
21+
*/
22+
export class CoreResultMemoiser<T> {
23+
24+
protected cachedResult?: T;
25+
protected lastParams: unknown[] = [];
26+
27+
/**
28+
* Get the cached result if valid.
29+
*
30+
* @param params Parameters used to calculate the cached result.
31+
* @returns The cached result, or undefined if not set.
32+
*/
33+
get(...params: unknown[]): T | undefined {
34+
return this.isValid(...params) ? this.cachedResult : undefined;
35+
}
36+
37+
/**
38+
* Check if the provided keys match the last cached keys.
39+
*
40+
* @param params Parameters used to calculate the cached result.
41+
* @returns Whether the cached result is valid.
42+
*/
43+
isValid(...params: unknown[]): boolean {
44+
if (this.cachedResult === undefined || params.length !== this.lastParams.length) {
45+
return false;
46+
}
47+
48+
return params.every((param, index) => typeof param === 'object' ?
49+
CoreObject.deepEquals(param, this.lastParams[index]) :
50+
param === this.lastParams[index]);
51+
}
52+
53+
/**
54+
* Clear the cache.
55+
*/
56+
invalidate(): void {
57+
this.cachedResult = undefined;
58+
this.lastParams = [];
59+
}
60+
61+
/**
62+
* Get a memoised result. If the cached result is valid, it returns it.
63+
* Otherwise, it executes the provided function, caches the result, and returns it.
64+
*
65+
* @param fn The function to calculate the result.
66+
* @param params Parameters that identify the result.
67+
* @returns The cached or newly calculated result.
68+
*/
69+
memoise(fn: () => T, ...params: unknown[]): T {
70+
const cachedResult = this.get(...params);
71+
if (cachedResult !== undefined) {
72+
return cachedResult;
73+
}
74+
75+
const result = fn();
76+
this.set(result, ...params);
77+
78+
return result;
79+
}
80+
81+
/**
82+
* Update the cache.
83+
*
84+
* @param result The result to cache.
85+
* @param params Parameters used to calculate the cached result.
86+
*/
87+
set(result: T, ...params: unknown[]): void {
88+
this.cachedResult = result;
89+
this.lastParams = params;
90+
}
91+
92+
}

‎src/core/classes/tests/result-memoiser.test.ts‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// (C) Copyright 2015 Moodle Pty Ltd.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import { CoreResultMemoiser } from '@classes/result-memoiser';
16+
17+
describe('CoreResultMemoiser', () => {
18+
19+
it('should cache and retrieve results with set, get, and isValid', () => {
20+
const memoiser = new CoreResultMemoiser<string>();
21+
22+
// Initially, no result is cached.
23+
expect(memoiser.get()).toBeUndefined();
24+
expect(memoiser.get('param1')).toBeUndefined();
25+
expect(memoiser.isValid()).toBe(false);
26+
expect(memoiser.isValid('param1')).toBe(false);
27+
28+
// Set a result with parameters.
29+
memoiser.set('result', 'param1', 'param2');
30+
expect(memoiser.get('param1', 'param2')).toBe('result');
31+
expect(memoiser.isValid('param1', 'param2')).toBe(true);
32+
33+
// Wrong params return undefined and are invalid.
34+
expect(memoiser.get('param1', 'different')).toBeUndefined();
35+
expect(memoiser.get('param1')).toBeUndefined();
36+
expect(memoiser.get('param1', 'param2', 'extra')).toBeUndefined();
37+
expect(memoiser.isValid('param1')).toBe(false);
38+
expect(memoiser.isValid('param1', 'param2', 'extra')).toBe(false);
39+
40+
// Overwrite with new params.
41+
memoiser.set('second', 'param3');
42+
expect(memoiser.get('param1', 'param2')).toBeUndefined();
43+
expect(memoiser.get('param3')).toBe('second');
44+
45+
// Handle no parameters.
46+
memoiser.set('42');
47+
expect(memoiser.get()).toBe('42');
48+
49+
// Handle falsy values.
50+
const memoiserFalsy = new CoreResultMemoiser<string | number | boolean | null>();
51+
memoiserFalsy.set('', 'empty');
52+
expect(memoiserFalsy.get('empty')).toBe('');
53+
54+
memoiserFalsy.set(0, 'zero');
55+
expect(memoiserFalsy.get('zero')).toBe(0);
56+
57+
memoiserFalsy.set(false, 'false');
58+
expect(memoiserFalsy.get('false')).toBe(false);
59+
60+
memoiserFalsy.set(null, 'null');
61+
expect(memoiserFalsy.get('null')).toBe(null);
62+
});
63+
64+
it('should validate params with primitive types', () => {
65+
const memoiser = new CoreResultMemoiser<string>();
66+
memoiser.set('result', 'string', 123, true, null, undefined);
67+
68+
expect(memoiser.isValid('string', 123, true, null, undefined)).toBe(true);
69+
expect(memoiser.isValid('different', 123, true, null, undefined)).toBe(false);
70+
expect(memoiser.isValid('string', 456, true, null, undefined)).toBe(false);
71+
expect(memoiser.isValid('string', 123, false, null, undefined)).toBe(false);
72+
});
73+
74+
it('should validate params with objects and arrays using deep equality', () => {
75+
const memoiser = new CoreResultMemoiser<string>();
76+
const obj = { a: 1, b: { c: 2 } };
77+
memoiser.set('result', obj);
78+
79+
// Object deep equality.
80+
expect(memoiser.isValid({ a: 1, b: { c: 2 } })).toBe(true);
81+
expect(memoiser.isValid({ a: 1, b: { c: 3 } })).toBe(false);
82+
expect(memoiser.isValid({ a: 1 })).toBe(false);
83+
84+
// Array deep equality.
85+
memoiser.set('array-result', [1, 2, 3]);
86+
expect(memoiser.isValid([1, 2, 3])).toBe(true);
87+
expect(memoiser.isValid([1, 2])).toBe(false);
88+
expect(memoiser.isValid([1, 2, 4])).toBe(false);
89+
expect(memoiser.isValid([1, 2, 3, 4])).toBe(false);
90+
91+
// Mixed primitives and objects.
92+
memoiser.set('mixed', 'string', 123, { key: 'value' }, true);
93+
expect(memoiser.isValid('string', 123, { key: 'value' }, true)).toBe(true);
94+
expect(memoiser.isValid('string', 123, { key: 'different' }, true)).toBe(false);
95+
});
96+
97+
it('should clear cache with invalidate', () => {
98+
const memoiser = new CoreResultMemoiser<string>();
99+
memoiser.set('result', 'param1', 'param2', 'param3');
100+
101+
expect(memoiser.get('param1', 'param2', 'param3')).toBe('result');
102+
expect(memoiser.isValid('param1', 'param2', 'param3')).toBe(true);
103+
104+
memoiser.invalidate();
105+
106+
expect(memoiser.get('param1', 'param2', 'param3')).toBeUndefined();
107+
expect(memoiser.isValid('param1', 'param2', 'param3')).toBe(false);
108+
});
109+
110+
it('should memoise function results', () => {
111+
const memoiser = new CoreResultMemoiser<string>();
112+
const fn = jest.fn(() => 'result');
113+
114+
// First call executes function and caches.
115+
let result = memoiser.memoise(fn, 'param1');
116+
expect(fn).toHaveBeenCalledTimes(1);
117+
expect(result).toBe('result');
118+
expect(memoiser.get('param1')).toBe('result');
119+
120+
// Second call with same params uses cache.
121+
result = memoiser.memoise(fn, 'param1');
122+
expect(fn).toHaveBeenCalledTimes(1);
123+
expect(result).toBe('result');
124+
125+
// Different params re-execute.
126+
memoiser.invalidate();
127+
const fn2 = jest.fn((param: string) => `result-${param}`);
128+
memoiser.memoise(() => fn2('param1'), 'param1');
129+
memoiser.memoise(() => fn2('param2'), 'param2');
130+
expect(fn2).toHaveBeenCalledTimes(2);
131+
expect(memoiser.get('param1')).toBe(undefined);
132+
expect(memoiser.get('param2')).toBe('result-param2');
133+
134+
// Object parameters.
135+
const memoiserObj = new CoreResultMemoiser<string>();
136+
const fnObj = jest.fn(() => 'result');
137+
memoiserObj.memoise(fnObj, { a: 1 });
138+
memoiserObj.memoise(fnObj, { a: 1 }); // Same object structure
139+
memoiserObj.memoise(fnObj, { a: 2 }); // Different object
140+
expect(fnObj).toHaveBeenCalledTimes(2);
141+
expect(memoiserObj.get({ a: 1 })).toBe(undefined);
142+
expect(memoiserObj.get({ a: 2 })).toBe('result');
143+
144+
// Complex return types
145+
const memoiserComplex = new CoreResultMemoiser<{ value: string; count: number }>();
146+
const fnComplex = jest.fn((param: string) => ({ value: `result-${param}`, count: 42 }));
147+
const result1 = memoiserComplex.memoise(() => fnComplex('first'), 'param');
148+
const result2 = memoiserComplex.memoise(() => fnComplex('second'), 'param');
149+
expect(fnComplex).toHaveBeenCalledTimes(1);
150+
expect(result1).toEqual({ value: 'result-first', count: 42 });
151+
expect(result2).toBe(result1); // Same reference
152+
});
153+
154+
});

‎src/core/features/login/tests/credentials.test.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,9 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
import { findElement, mockSingleton, renderPageComponent, requireElement } from '@/testing/utils';
15+
import { findElement, mockSingleton, renderPageComponent, requireElement, overrideTranslations } from '@/testing/utils';
1616
import { CoreLoginError } from '@classes/errors/loginerror';
1717
import CoreLoginCredentialsPage from '@features/login/pages/credentials/credentials';
18-
import { CoreLang } from '@services/lang';
1918
import { CoreSites } from '@services/sites';
2019
import { Http } from '@singletons';
2120
import { of } from 'rxjs';
@@ -29,7 +28,6 @@ describe('Credentials page', () => {
2928
beforeEach(() => {
3029
// eslint-disable-next-line @typescript-eslint/no-explicit-any
3130
mockSingleton(Http, { get: () => of(null as any) });
32-
mockSingleton(CoreLang, { getCurrentLanguage: async () => 'en' });
3331
});
3432

3533
it('renders', async () => {
@@ -129,6 +127,12 @@ describe('Credentials page', () => {
129127

130128
mockSingleton(CoreLoginHelper, { getAvailableSites: async () => [] });
131129

130+
// Override translation before rendering the component to make sure it's used in the page.
131+
const translationText = 'You have exceeded the maximum number of login attempts. Please contact support.';
132+
overrideTranslations({
133+
'core.login.exceededloginattempts': translationText,
134+
});
135+
132136
const fixture = await renderPageComponent(CoreLoginCredentialsPage, {
133137
routeParams: { siteUrl, siteCheck },
134138
});
@@ -146,7 +150,7 @@ describe('Credentials page', () => {
146150
}
147151

148152
// Assert.
149-
expect(findElement(fixture, 'ion-label', 'core.login.exceededloginattempts')).not.toBeNull();
153+
expect(findElement(fixture, 'ion-label', translationText)).not.toBeNull();
150154
});
151155

152156
});

‎src/core/pipes/bytes-to-size.ts‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { Translate } from '@singletons';
1818
import { CoreLogger } from '@static/logger';
1919
import { CoreText } from '@static/text';
2020
import { Subscription } from 'rxjs';
21+
import { CoreResultMemoiser } from '@classes/result-memoiser';
2122

2223
/**
2324
* Pipe to turn a number in bytes to a human readable size (e.g. 5,25 MB).
@@ -29,16 +30,14 @@ import { Subscription } from 'rxjs';
2930
export class CoreBytesToSizePipe implements PipeTransform, OnDestroy {
3031

3132
protected logger: CoreLogger;
32-
protected cachedResult?: string;
33+
protected memoiser = new CoreResultMemoiser<string>();
3334
protected subscription: Subscription;
3435

35-
protected lastValue?: number | string;
36-
3736
constructor() {
3837
this.logger = CoreLogger.getInstance('CoreBytesToSizePipe');
3938

4039
this.subscription = Translate.onLangChange.subscribe(() => {
41-
this.cachedResult = undefined;
40+
this.memoiser.invalidate();
4241
});
4342
}
4443

@@ -49,16 +48,10 @@ export class CoreBytesToSizePipe implements PipeTransform, OnDestroy {
4948
* @returns Readable bytes.
5049
*/
5150
transform(value: number | string): string {
52-
if (this.lastValue !== value) {
53-
this.lastValue = value;
54-
this.cachedResult = undefined;
55-
}
56-
57-
if (this.cachedResult === undefined) {
58-
this.cachedResult = this.formatBytes(value);
59-
}
60-
61-
return this.cachedResult;
51+
return this.memoiser.memoise(
52+
() => this.formatBytes(value),
53+
value,
54+
);
6255
}
6356

6457
/**

‎src/core/pipes/date-day-or-time.ts‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { CoreTime } from '@static/time';
1919
import { Translate } from '@singletons';
2020
import { CoreLogger } from '@static/logger';
2121
import { Subscription } from 'rxjs';
22+
import { CoreResultMemoiser } from '@classes/result-memoiser';
2223

2324
/**
2425
* Filter to display a date using the day, or the time.
@@ -40,16 +41,14 @@ import { Subscription } from 'rxjs';
4041
export class CoreDateDayOrTimePipe implements PipeTransform, OnDestroy {
4142

4243
protected logger: CoreLogger;
43-
protected cachedResult?: string;
44+
protected memoiser = new CoreResultMemoiser<string>();
4445
protected subscription: Subscription;
4546

46-
protected lastTimestamp?: number | string;
47-
4847
constructor() {
4948
this.logger = CoreLogger.getInstance('CoreDateDayOrTimePipe');
5049

5150
this.subscription = Translate.onLangChange.subscribe(() => {
52-
this.cachedResult = undefined;
51+
this.memoiser.invalidate();
5352
});
5453
}
5554

@@ -60,16 +59,10 @@ export class CoreDateDayOrTimePipe implements PipeTransform, OnDestroy {
6059
* @returns Formatted time.
6160
*/
6261
transform(timestamp: string | number): string {
63-
if (this.lastTimestamp !== timestamp) {
64-
this.lastTimestamp = timestamp;
65-
this.cachedResult = undefined;
66-
}
67-
68-
if (this.cachedResult === undefined) {
69-
this.cachedResult = this.formatTimestamp(timestamp);
70-
}
71-
72-
return this.cachedResult;
62+
return this.memoiser.memoise(
63+
() => this.formatTimestamp(timestamp),
64+
timestamp,
65+
);
7366
}
7467

7568
/**

0 commit comments

Comments
 (0)