Skip to content

Commit 01f123a

Browse files
milutinGerrit Code Review
authored and
Gerrit Code Review
committed
Merge "Introduce AppContext"
2 parents 3c9eb8f + 849afea commit 01f123a

File tree

10 files changed

+164
-21
lines changed

10 files changed

+164
-21
lines changed

polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js

+8-3
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-l
3030
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
3131
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
3232
import {htmlTemplate} from './gr-change-list_html.js';
33-
import {flags} from '../../../services/flags';
33+
import {appContext} from '../../../services/app-context.js';
3434
import {BaseUrlBehavior} from '../../../behaviors/base-url-behavior/base-url-behavior.js';
3535
import {ChangeTableBehavior} from '../../../behaviors/gr-change-table-behavior/gr-change-table-behavior.js';
3636
import {URLEncodingBehavior} from '../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.js';
@@ -152,6 +152,11 @@ class GrChangeList extends mixinBehaviors( [
152152
};
153153
}
154154

155+
constructor() {
156+
super();
157+
this.flagsService = appContext.flagsService;
158+
}
159+
155160
/** @override */
156161
created() {
157162
super.created();
@@ -204,7 +209,7 @@ class GrChangeList extends mixinBehaviors( [
204209
this.changeTableColumns = this.columnNames;
205210
this.showNumber = false;
206211
this.visibleChangeTableColumns = this.getEnabledColumns(this.columnNames,
207-
config, flags.enabledExperiments);
212+
config, this.flagsService.enabledExperiments);
208213

209214
if (account) {
210215
this.showNumber = !!(preferences &&
@@ -213,7 +218,7 @@ class GrChangeList extends mixinBehaviors( [
213218
preferences.change_table.length > 0) {
214219
const prefColumns = this.getVisibleColumns(preferences.change_table);
215220
this.visibleChangeTableColumns = this.getEnabledColumns(prefColumns,
216-
config, flags.enabledExperiments);
221+
config, this.flagsService.enabledExperiments);
217222
}
218223
}
219224
}

polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import '../../../scripts/bundled-polymer.js';
1818

1919
import {Polymer} from '@polymer/polymer/lib/legacy/polymer-fn.js';
20-
import {flags} from '../../../services/flags';
20+
import {appContext} from '../../../services/app-context.js';
2121

2222
// Latency reporting constants.
2323
const TIMING = {
@@ -279,7 +279,7 @@ let GrReporting = Polymer({
279279
eventInfo.inBackgroundTab = isInBackgroundTab;
280280
}
281281

282-
const enabledExperiments = flags.enabledExperiments;
282+
const enabledExperiments = appContext.flagsService.enabledExperiments;
283283
if (enabledExperiments.length) {
284284
eventInfo.enabledExperiments = JSON.stringify(enabledExperiments);
285285
}

polygerrit-ui/app/elements/gr-app-init.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17+
import {initAppContext} from '../services/app-context-init.js';
1718

1819
if (!window.Polymer) {
1920
window.Polymer = {
2021
passiveTouchGestures: true,
2122
lazyRegister: true,
2223
};
2324
}
24-
window.Gerrit = window.Gerrit || {};
25+
window.Gerrit = window.Gerrit || {};
26+
27+
initAppContext();

polygerrit-ui/app/services/README.md

+29-5
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,34 @@ Services should be stateful, if its just pure functions, consider having them in
99
Regarding all stateful should be considered as services or not, it's still TBD. Will update as soon
1010
as it's finalized.
1111

12-
## Future plans
12+
## How to access service
1313

14-
To make services much easier to use, we may need to adopt a DI (dependency injection) system instead of exporting singleton
15-
from the services directly. And it will help in mocking services in tests as well.
14+
We use AppContext to access instance of service. It helps in mocking service in tests as well.
15+
We prefer setting instance of service in constructor and then accessing it from variable. We also
16+
allow access straight from appContext especially in static methods.
17+
18+
```
19+
import {appContext} from '../../../services/app-context.js';
20+
21+
class T {
22+
constructor() {
23+
super();
24+
this.flagsService = appContext.flagsService;
25+
}
26+
27+
action1() {
28+
if (this.flagsService.isEnabled('test)) {
29+
// do something
30+
}
31+
}
32+
}
33+
34+
staticMethod() {
35+
if (appContext.flagsService.isEnabled('test)) {
36+
// do something
37+
}
38+
}
39+
```
1640

1741
## What services we have
1842

@@ -21,10 +45,10 @@ from the services directly. And it will help in mocking services in tests as wel
2145
'flags' is a service to provide easy access to all enabled experiments.
2246

2347
```
24-
import {flags} from "./flags.js";
48+
import {appContext} from '../../../services/app-context.js';
2549
2650
// check if an experiment is enabled or not
27-
if (flags.isEnabled('test')) {
51+
if (appContext.flagsService.isEnabled('test')) {
2852
// do something
2953
}
3054
```
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* @license
3+
* Copyright (C) 2020 The Android Open Source Project
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
import {appContext} from './app-context.js';
18+
import {FlagsService} from './flags.js';
19+
20+
const initializedServices = new Map();
21+
22+
function getService(serviceName, serviceInit) {
23+
if (!initializedServices[serviceName]) {
24+
initializedServices[serviceName] = serviceInit();
25+
}
26+
return initializedServices[serviceName];
27+
}
28+
29+
/**
30+
* The AppContext lazy initializator for all services
31+
*/
32+
export function initAppContext() {
33+
const registeredServices = {};
34+
function addService(serviceName, serviceCreator) {
35+
if (registeredServices[serviceName]) {
36+
throw new Error(`Service ${serviceName} already registered.`);
37+
}
38+
registeredServices[serviceName] = {
39+
get() {
40+
return getService(serviceName, serviceCreator);
41+
},
42+
};
43+
}
44+
45+
addService('flagsService', () => new FlagsService());
46+
47+
Object.defineProperties(appContext, registeredServices);
48+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<!DOCTYPE html>
2+
<!--
3+
@license
4+
Copyright (C) 2020 The Android Open Source Project
5+
6+
Licensed under the Apache License, Version 2.0 (the "License");
7+
you may not use this file except in compliance with the License.
8+
You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
-->
18+
19+
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
20+
<script src="/node_modules/@webcomponents/webcomponentsjs/custom-elements-es5-adapter.js"></script>
21+
<script src="/node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js"></script>
22+
<script src="/components/wct-browser-legacy/browser.js"></script>
23+
24+
<script type="module">
25+
import '../test/common-test-setup.js';
26+
import {appContext} from './app-context.js';
27+
import {initAppContext} from './app-context-init.js';
28+
suite('app context initializer tests', () => {
29+
setup(() => {
30+
initAppContext();
31+
});
32+
33+
test('all services initialized and are singletons', () => {
34+
Object.keys(appContext).forEach(serviceName => {
35+
const service = appContext[serviceName];
36+
assert.isNotNull(service);
37+
const service2 = appContext[serviceName];
38+
assert.strictEqual(service, service2);
39+
});
40+
});
41+
});
42+
</script>
+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @license
3+
* Copyright (C) 2020 The Android Open Source Project
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
/**
19+
* The AppContext holds immortal singleton instances of services. It's a
20+
* convenient way to provide singletons that can be swapped out for testing.
21+
*
22+
* AppContext is initialized in ./app-context-init.js
23+
*/
24+
export const appContext = {
25+
flagsService: null,
26+
};

polygerrit-ui/app/services/flags.js

+1-4
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
*
2121
* Provides all related methods / properties regarding on feature flags.
2222
*/
23-
class Flags {
23+
export class FlagsService {
2424
constructor() {
2525
// stores all enabled experiments
2626
this._experiments = new Set();
@@ -46,6 +46,3 @@ class Flags {
4646
return [...this._experiments];
4747
}
4848
}
49-
50-
// Export a single instance of Flags to be used across components.
51-
export const flags = new Flags();

polygerrit-ui/app/services/flags_test.html

+2-5
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,9 @@
2727

2828
<script type="module">
2929
import '../test/common-test-setup.js';
30-
import {flags} from './flags.js';
31-
import {flags as flags2} from './flags.js';
30+
import {FlagsService} from './flags.js';
3231
suite('flags tests', () => {
33-
test('singlton', () => {
34-
assert.equal(flags, flags2);
35-
});
32+
const flags = new FlagsService();
3633

3734
test('isEnabled', () => {
3835
assert.equal(flags.isEnabled('a'), true);

polygerrit-ui/app/test/common-test-setup.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import '@polymer/iron-test-helpers/iron-test-helpers.js';
2121
import './test-router.js';
2222
import moment from 'moment/src/moment.js';
2323
import {SafeTypes} from '../behaviors/safe-types-behavior/safe-types-behavior.js';
24-
24+
import {initAppContext} from '../services/app-context-init.js';
2525
self.moment = moment;
2626
security.polymer_resin.install({
2727
allowedIdentifierPrefixes: [''],
@@ -78,6 +78,7 @@ setup(() => {
7878
if (Gerrit._testOnly_resetPlugins) {
7979
Gerrit._testOnly_resetPlugins();
8080
}
81+
initAppContext();
8182
});
8283

8384
if (window.stub) {

0 commit comments

Comments
 (0)