Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

[Full changelog](https://github.com/mozilla/glean.js/compare/v0.10.0...main)

* [#254](https://github.com/mozilla/glean.js/pull/254): BUGFIX: Allow the usage of the Glean specific metrics API before Glean is initialized.

# v0.10.0 (2021-04-20)

[Full changelog](https://github.com/mozilla/glean.js/compare/v0.9.2...v0.10.0)
Expand Down
35 changes: 30 additions & 5 deletions glean/src/core/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import type { DebugOptions } from "./debug_options";
import type Dispatcher from "./dispatcher";
import Dispatcher from "./dispatcher";
import type MetricsDatabase from "./metrics/database";
import type EventsDatabase from "./metrics/events_database";
import type PingsDatabase from "./pings/database";
Expand All @@ -23,21 +23,20 @@ import type PingsDatabase from "./pings/database";
export class Context {
private static _instance: Context;

private _dispatcher!: Dispatcher;
private _dispatcher!: Dispatcher | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this need to be null? Can't we initialize the dispatcher on the Context's constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs to be null because we can't really tear down the whole Context when we reset Glean, because we need to retain the storage. So we do a "soft reset" by nulling out the dispatcher in testReset

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would prefer private _dispatcher?: Dispatcher;, but no strong requirement for this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes it undefined though,

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but this a very philosophical thing (null vs undefined) and makes no real difference here. Feel free to ignore the request ;)


private _uploadEnabled!: boolean;
private _metricsDatabase!: MetricsDatabase;
private _eventsDatabase!: EventsDatabase;
private _pingsDatabase!: PingsDatabase;

private _applicationId!: string;
private _initialized!: boolean;
private _initialized: boolean;

private _debugOptions!: DebugOptions;

private constructor() {
// Intentionally empty, exclusively defined to mark the
// constructor as private.
this._initialized = false;
}

static get instance(): Context {
Expand All @@ -48,7 +47,33 @@ export class Context {
return Context._instance;
}

/**
* **Test-only API**
*
* Resets the Context to an uninitialized state.
*/
static async testUninitialize(): Promise<void> {
// Clear the dispatcher queue and return the dispatcher back to an uninitialized state.
if (Context.instance._dispatcher) {
await Context.instance._dispatcher.testUninitialize();
}

// Due to the test requirement of keeping storage in place,
// we can't simply wipe out the full `Context` instance.
// The closest thing we can do is making the dispatcher `null`.
Context.instance._dispatcher = null;
Context.initialized = false;
}

static get dispatcher(): Dispatcher {
// Create a dispatcher if one isn't available already.
// This is required since the dispatcher may be used
// earlier than Glean initialization, so we can't rely
// on `Glean.initialize` to set it.
if (!Context.instance._dispatcher) {
Context.instance._dispatcher = new Dispatcher();
}

return Context.instance._dispatcher;
}

Expand Down
10 changes: 1 addition & 9 deletions glean/src/core/glean.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import EventsDatabase from "./metrics/events_database.js";
import UUIDMetricType from "./metrics/types/uuid.js";
import DatetimeMetricType from "./metrics/types/datetime.js";
import { DatetimeMetric } from "./metrics/types/datetime_metric.js";
import Dispatcher from "./dispatcher.js";
import CorePings from "./internal_pings.js";
import { registerPluginToEvent, testResetEvents } from "./events/utils.js";

Expand Down Expand Up @@ -51,10 +50,8 @@ class Glean {
Use Glean.instance instead to access the Glean singleton.`);
}

Context.dispatcher = new Dispatcher();
this._coreMetrics = new CoreMetrics();
this._corePings = new CorePings();
Context.initialized = false;
}

private static get instance(): Glean {
Expand Down Expand Up @@ -501,16 +498,11 @@ class Glean {
*/
static async testUninitialize(): Promise<void> {
// Get back to an uninitialized state.
Context.initialized = false;
await Context.testUninitialize();

// Deregister all plugins
testResetEvents();

// Clear the dispatcher queue and return the dispatcher back to an uninitialized state.
if (Context.dispatcher) {
await Context.dispatcher.testUninitialize();
}

// Stop ongoing jobs and clear pending pings queue.
if (Glean.pingUploader) {
// The first time tests run, before Glean is initialized, we are
Expand Down
16 changes: 16 additions & 0 deletions glean/tests/core/context.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,20 @@ describe("Context", function() {
assert.notStrictEqual(Context.pingsDatabase, undefined);
assert.ok(Context.pingsDatabase instanceof PingsDatabase);
});

it("the dispatcher is always available", async function () {
const originalDispatcher = Context.dispatcher;
assert.notStrictEqual(originalDispatcher, null);

await Context.testUninitialize();

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
assert.strictEqual(Context.instance._dispatcher, null);

// Trying to access the dispatcher will instantiate a new one.
const newDispatcher = Context.instance;
assert.notStrictEqual(newDispatcher, null);
assert.notStrictEqual(newDispatcher, originalDispatcher);
});
});