Skip to content

Add internal API to collect platform version info #2368

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

Merged
merged 10 commits into from
Dec 3, 2019
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
1 change: 1 addition & 0 deletions config/tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
],
"module": "ES2015",
"moduleResolution": "node",
"resolveJsonModule": true,
"sourceMap": true,
"target": "es5",
"typeRoots": [
Expand Down
4 changes: 4 additions & 0 deletions packages/analytics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import {
} from '@firebase/component';
import { ERROR_FACTORY, AnalyticsError } from './src/errors';

import { name, version } from './package.json';

declare global {
interface Window {
[key: string]: unknown;
Expand Down Expand Up @@ -62,6 +64,8 @@ export function registerAnalytics(instance: _FirebaseNamespace): void {
new Component('analytics-internal', internalFactory, ComponentType.PRIVATE)
);

instance.registerVersion(name, version);

function internalFactory(
container: ComponentContainer
): FirebaseAnalyticsInternal {
Expand Down
1 change: 1 addition & 0 deletions packages/analytics/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"rollup-plugin-commonjs": "10.1.0",
"rollup-plugin-json": "4.0.0",
"rollup-plugin-node-resolve": "5.2.0",
"rollup-plugin-typescript2": "0.25.2",
"rollup-plugin-uglify": "6.0.3",
"typescript": "3.7.2"
},
Expand Down
14 changes: 14 additions & 0 deletions packages/app-types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,26 @@ export interface FirebaseNamespace {
*/
apps: FirebaseApp[];

/**
* Registers a library's name and version for platform logging purposes.
* @param library Name of 1p or 3p library (e.g. firestore, angularfire)
* @param version Current version of that library.
*/
registerVersion(library: string, version: string): void;

// The current SDK version.
SDK_VERSION: string;
}

export interface VersionService {
library: string;
version: string;
}

declare module '@firebase/component' {
interface NameServiceMapping {
'app': FirebaseApp;
'app-version': VersionService;
'platform-identifier': VersionService;
}
}
10 changes: 10 additions & 0 deletions packages/app-types/private.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ export interface FirebaseServiceFactory {
): FirebaseService;
}

export interface PlatformLoggerService {
getPlatformInfoString(): string;
}

/**
* All ServiceNamespaces extend from FirebaseServiceNamespace
*/
Expand Down Expand Up @@ -154,3 +158,9 @@ export interface _FirebaseNamespace extends FirebaseNamespace {
ErrorFactory: typeof ErrorFactory;
};
}

declare module '@firebase/component' {
interface NameServiceMapping {
'platform-logger': PlatformLoggerService;
}
}
3 changes: 3 additions & 0 deletions packages/app/index.lite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@
*/

import { createFirebaseNamespaceLite } from './src/lite/firebaseNamespaceLite';
import { registerCoreComponents } from './src/registerCoreComponents';

export const firebase = createFirebaseNamespaceLite();

registerCoreComponents(firebase);

// eslint-disable-next-line import/no-default-export
export default firebase;
3 changes: 3 additions & 0 deletions packages/app/index.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { firebase as _firebase } from './src/firebaseNamespace';
import Storage from 'dom-storage';
// @ts-ignore
import { XMLHttpRequest } from 'xmlhttprequest';
import { registerCoreComponents } from './src/registerCoreComponents';

(_firebase as _FirebaseNamespace).INTERNAL.extendNamespace({
INTERNAL: {
Expand All @@ -36,5 +37,7 @@ import { XMLHttpRequest } from 'xmlhttprequest';

export const firebase = _firebase as FirebaseNamespace;

registerCoreComponents(firebase);

// eslint-disable-next-line import/no-default-export
export default firebase;
4 changes: 4 additions & 0 deletions packages/app/index.rn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import { FirebaseNamespace } from '@firebase/app-types';
import { _FirebaseNamespace } from '@firebase/app-types/private';
import { firebase as _firebase } from './src/firebaseNamespace';
import { registerCoreComponents } from './src/registerCoreComponents';

/**
* To avoid having to include the @types/react-native package, which breaks
* some of our tests because of duplicate symbols, we are using require syntax
Expand All @@ -36,5 +38,7 @@ const { AsyncStorage } = require('react-native');

export const firebase = _firebase as FirebaseNamespace;

registerCoreComponents(firebase);

// eslint-disable-next-line import/no-default-export
export default firebase;
3 changes: 3 additions & 0 deletions packages/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { FirebaseNamespace } from '@firebase/app-types';
import { firebase as firebaseNamespace } from './src/firebaseNamespace';
import { isNode, isBrowser } from '@firebase/util';
import { logger } from './src/logger';
import { registerCoreComponents } from './src/registerCoreComponents';

// Firebase Lite detection
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -67,5 +68,7 @@ firebaseNamespace.initializeApp = function(...args: any) {

export const firebase = firebaseNamespace;

registerCoreComponents(firebase);

// eslint-disable-next-line import/no-default-export
export default firebase;
30 changes: 30 additions & 0 deletions packages/app/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,33 @@
*/

export const DEFAULT_ENTRY_NAME = '[DEFAULT]';
import { name as appName } from '../package.json';
import { name as analyticsName } from '../../analytics/package.json';
import { name as authName } from '../../auth/package.json';
import { name as databaseName } from '../../database/package.json';
import { name as functionsName } from '../../functions/package.json';
import { name as messagingName } from '../../messaging/package.json';
import { name as performanceName } from '../../performance/package.json';
import { name as remoteConfigName } from '../../remote-config/package.json';
import { name as storageName } from '../../storage/package.json';
import { name as firestoreName } from '../../firestore/package.json';

export const PLATFORM_LOG_STRING = {
[appName]: 'fire-core',
[analyticsName]: 'fire-analytics',
[authName]: 'fire-auth',
[databaseName]: 'fire-rtdb',
[functionsName]: 'fire-fn',
[messagingName]: 'fire-fcm',
[performanceName]: 'fire-perf',
[remoteConfigName]: 'fire-rc',
[storageName]: 'fire-gcs',
[firestoreName]: 'fire-fst',
'fire-js': 'fire-js', // Platform identifier for JS SDK.
'fire-js-all-app': 'fire-js-all-app', // firebase/app import
'fire-js-all': 'fire-js-all', // 'firebase' import
'fire-js-all-node': 'fire-js-all-node',
'fire-js-all-rn': 'fire-js-all-rn',
'fire-js-all-lite': 'fire-js-all-lite',
'fire-js-all-cdn': 'fire-js-all-cdn'
} as const;
40 changes: 38 additions & 2 deletions packages/app/src/firebaseNamespaceCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ import { deepExtend, contains } from '@firebase/util';
import { FirebaseAppImpl } from './firebaseApp';
import { ERROR_FACTORY, AppError } from './errors';
import { FirebaseAppLiteImpl } from './lite/firebaseAppLite';
import { DEFAULT_ENTRY_NAME } from './constants';
import { DEFAULT_ENTRY_NAME, PLATFORM_LOG_STRING } from './constants';
import { version } from '../../firebase/package.json';
import { logger } from './logger';
import { Component, ComponentType } from '@firebase/component';
import { Component, ComponentType, Name } from '@firebase/component';

/**
* Because auth can't share code with other components, we attach the utility functions
Expand All @@ -59,6 +59,7 @@ export function createFirebaseNamespaceCore(
initializeApp,
// @ts-ignore
app,
registerVersion,
// @ts-ignore
apps: null,
SDK_VERSION: version,
Expand Down Expand Up @@ -234,6 +235,41 @@ export function createFirebaseNamespaceCore(
: null;
}

function registerVersion(libraryKeyOrName: string, version: string): void {
// TODO: We can use this check to whitelist strings when/if we set up
// a good whitelist system.
const library = PLATFORM_LOG_STRING[libraryKeyOrName] ?? libraryKeyOrName;
const libraryMismatch = library.match(/\s|\//);
const versionMismatch = version.match(/\s|\//);
if (libraryMismatch || versionMismatch) {
const warning = [
`Unable to register library "${library}" with version "${version}":`
];
if (libraryMismatch) {
warning.push(
`library name "${library}" contains illegal characters (whitespace or "/")`
);
}
if (libraryMismatch && versionMismatch) {
warning.push('and');
}
if (versionMismatch) {
warning.push(
`version name "${version}" contains illegal characters (whitespace or "/")`
);
}
logger.warn(warning.join(' '));
return;
}
registerComponent(
new Component(
`${library}-version` as Name,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of this cast TBH, since it defeats the purpose of the Name type. Maybe we shouldn't consider versions actual Components and instead create a different type for them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that it might be a good idea in the long term to have separate Map in ComponentContainer in addition to the Providers Map, and this new Map just stores key/value pairs. Can be used for versions now, and other possible key/value pair data going forward. But that requires a bit of modification to ComponentContainer that might not be a good idea right now as we're planning to do our first release with the migration to the component model next week right before a holiday. Since the actual registerVersion API wouldn't change we can change the implementation later when the component model is in use and stable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, sounds good.

() => ({ library, version }),
ComponentType.VERSION
)
);
}

// Map the requested service to a registered service name
// (used to map auth to serverAuth service when needed).
function useAsService(app: FirebaseApp, name: string): string | null {
Expand Down
59 changes: 59 additions & 0 deletions packages/app/src/platformLoggerService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* @license
* Copyright 2019 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import {
ComponentContainer,
ComponentType,
Provider,
Name
} from '@firebase/component';

export class PlatformLoggerService {
constructor(private readonly container: ComponentContainer) {}
// In initial implementation, this will be called by installations on
// auth token refresh, and installations will send this string.
getPlatformInfoString(): string {
const providers = this.container.getProviders();
// Loop through providers and get library/version pairs from any that are
// version components.
return providers
.map(provider => {
if (isVersionServiceProvider(provider)) {
const service = provider.getImmediate();
return `${service.library}/${service.version}`;
} else {
return null;
}
})
.filter(logString => logString)
.join(' ');
}
}
/**
*
* @param provider check if this provider provides a VersionService
*
* NOTE: Using Provider<'app-version'> is a hack to indicate that the provider
* provides VersionService. The provider is not necessarily a 'app-version'
* provider.
*/
function isVersionServiceProvider(
provider: Provider<Name>
): provider is Provider<'app-version'> {
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add a comment that Provider<'app-version'> is a hack to indicate that it's a version component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

const component = provider.getComponent();
return component?.type === ComponentType.VERSION;
}
36 changes: 36 additions & 0 deletions packages/app/src/registerCoreComponents.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* @license
* Copyright 2019 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { FirebaseNamespace } from '@firebase/app-types';
import { _FirebaseNamespace } from '@firebase/app-types/private';
import { Component, ComponentType } from '@firebase/component';
import { PlatformLoggerService } from './platformLoggerService';
import { name, version } from '../package.json';

export function registerCoreComponents(firebase: FirebaseNamespace): void {
(firebase as _FirebaseNamespace).INTERNAL.registerComponent(
new Component(
'platform-logger',
container => new PlatformLoggerService(container),
ComponentType.PRIVATE
)
);
// Register `app` package.
firebase.registerVersion(name, version);
// Register platform SDK identifier (no version).
firebase.registerVersion('fire-js', '');
}
46 changes: 46 additions & 0 deletions packages/app/test/firebaseApp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,52 @@ function executeFirebaseTests(): void {
);
});
});

describe('Firebase Version Registration', () => {
let firebase: FirebaseNamespace;

beforeEach(() => {
firebase = createFirebaseNamespace();
});

it('will register an official version component without warnings', () => {
const warnStub = stub(console, 'warn');
const { components } = (firebase as _FirebaseNamespace).INTERNAL;
const initialSize = components.size;

firebase.registerVersion('@firebase/analytics', '1.2.3');
expect(components.get('fire-analytics-version')).to.exist;
expect(components.size).to.equal(initialSize + 1);

expect(warnStub.called).to.be.false;
});

it('will register an arbitrary version component without warnings', () => {
const warnStub = stub(console, 'warn');
const { components } = (firebase as _FirebaseNamespace).INTERNAL;
const initialSize = components.size;

firebase.registerVersion('angularfire', '1.2.3');
expect(components.get('angularfire-version')).to.exist;
expect(components.size).to.equal(initialSize + 1);

expect(warnStub.called).to.be.false;
});

it('will do nothing if registerVersion() is given illegal characters', () => {
const warnStub = stub(console, 'warn');
const { components } = (firebase as _FirebaseNamespace).INTERNAL;
const initialSize = components.size;

firebase.registerVersion('remote config', '1.2.3');
expect(warnStub.args[0][1]).to.include('library name "remote config"');
expect(components.size).to.equal(initialSize);

firebase.registerVersion('remote-config', '1.2/3');
expect(warnStub.args[1][1]).to.include('version name "1.2/3"');
expect(components.size).to.equal(initialSize);
});
});
}

function executeFirebaseLiteTests(): void {
Expand Down
Loading