fix: Add LDOptions.application name and versionName. (#358)

yusinto · web-flow · commit cd75210d20e3 · 2024-02-01T10:36:22.000-08:00
Adds support for name and versionName in addition to id and version. I
have also renamed the client sdk config option from `application` to
`applicationInfo`. I have kept the `ApplicationTags` type though,
because that's a shared type with the server sdks and it's a much bigger
change to rename that.
diff --git a/packages/shared/common/__tests__/options/ApplicationTags.test.ts b/packages/shared/common/__tests__/options/ApplicationTags.test.ts
@@ -1,82 +1,90 @@
-import ApplicationTags from '../../src/options/ApplicationTags';
+import { logger } from '@launchdarkly/private-js-mocks';
 
-function makeLogger() {
-  return {
-    error: jest.fn(),
-    warn: jest.fn(),
-    info: jest.fn(),
-    debug: jest.fn(),
-  };
-}
+import ApplicationTags from '../../src/options/ApplicationTags';
 
 describe.each([
   [
-    { application: { id: 'is-valid', version: 'also-valid' }, logger: makeLogger() },
+    { application: { id: 'is-valid', version: 'also-valid' }, logger },
     'application-id/is-valid application-version/also-valid',
     [],
   ],
-  [{ application: { id: 'is-valid' }, logger: makeLogger() }, 'application-id/is-valid', []],
   [
-    { application: { version: 'also-valid' }, logger: makeLogger() },
-    'application-version/also-valid',
+    {
+      application: {
+        id: 'is-valid',
+        version: 'also-valid',
+        name: 'test-app-1',
+        versionName: 'test-version-1',
+      },
+      logger,
+    },
+    'application-id/is-valid application-name/test-app-1 application-version/also-valid application-version-name/test-version-1',
     [],
   ],
-  [{ application: {}, logger: makeLogger() }, undefined, []],
-  [{ logger: makeLogger() }, undefined, []],
-  [undefined, undefined, undefined],
+  [{ application: { id: 'is-valid' }, logger }, 'application-id/is-valid', []],
+  [{ application: { version: 'also-valid' }, logger }, 'application-version/also-valid', []],
+  [{ application: {}, logger }, undefined, []],
+  [{ logger }, undefined, []],
+  [undefined, undefined, []],
 
   // Above ones are 'valid' cases. Below are invalid.
+  [{ application: { id: 'bad tag' }, logger }, undefined, [/Config option "application.id" must/]],
   [
-    { application: { id: 'bad tag' }, logger: makeLogger() },
-    undefined,
-    [{ level: 'warn', matches: /Config option "application.id" must/ }],
+    { application: { id: 'bad tag', version: 'good-tag' }, logger },
+    'application-version/good-tag',
+    [/Config option "application.id" must/],
   ],
   [
-    { application: { id: 'bad tag', version: 'good-tag' }, logger: makeLogger() },
-    'application-version/good-tag',
-    [{ level: 'warn', matches: /Config option "application.id" must/ }],
+    {
+      application: { id: 'bad tag', version: 'good-tag', name: '', versionName: 'test-version-1' },
+      logger,
+    },
+    'application-version/good-tag application-version-name/test-version-1',
+    [/Config option "application.id" must/, /Config option "application.name" must/],
   ],
   [
-    { application: { id: 'bad tag', version: 'also bad' }, logger: makeLogger() },
+    {
+      application: {
+        id: 'bad tag',
+        version: 'also bad',
+        name: 'invalid name',
+        versionName: 'invalid version name',
+      },
+      logger,
+    },
     undefined,
     [
-      { level: 'warn', matches: /Config option "application.id" must/ },
-      { level: 'warn', matches: /Config option "application.version" must/ },
+      /Config option "application.id" must/,
+      /Config option "application.version" must/,
+      /Config option "application.name" must/,
+      /Config option "application.versionName" must/,
     ],
   ],
   // Bad tags and no logger.
-  [
-    { application: { id: 'bad tag', version: 'also bad' }, logger: undefined },
-    undefined,
-    undefined,
-  ],
-])('given application tags configurations %p', (config, result, logs) => {
+  [{ application: { id: 'bad tag', version: 'also bad' }, logger: undefined }, undefined, []],
+])('given application tags configurations %j', (config, result, warnings: RegExp[]) => {
   describe('when getting tag values', () => {
-    // @ts-ignore
-    const tags = new ApplicationTags(config);
+    let tags: ApplicationTags;
+
+    beforeEach(() => {
+      // @ts-ignore
+      tags = new ApplicationTags(config);
+    });
+
+    afterEach(() => {
+      jest.resetAllMocks();
+    });
 
     it('produces the correct tag values', () => {
       expect(tags.value).toEqual(result);
     });
 
-    it('logs issues it encounters', () => {
-      expect(config?.logger?.warn.mock.calls.length).toEqual(logs?.length);
+    it(`logs issues it encounters for ${JSON.stringify(config)}`, () => {
+      expect(logger.warn).toHaveBeenCalledTimes(warnings.length);
 
-      if (logs) {
-        const expected = [...logs];
-        config!.logger!.warn.mock.calls.forEach((call) => {
-          const index = expected.findIndex((expectedLog) => call[0].match(expectedLog.matches));
-          if (index < 0) {
-            throw new Error(`Did not find expectation for ${call[0]}`);
-          }
-          expected.splice(index, 1);
-        });
-        if (expected.length) {
-          throw new Error(
-            `Did not find expected messages: ${expected.map((item) => item.matches.toString())}`,
-          );
-        }
-      }
+      warnings.forEach((regExp) => {
+        expect(logger.warn).toHaveBeenCalledWith(expect.stringMatching(regExp));
+      });
     });
   });
 });
diff --git a/packages/shared/common/src/options/ApplicationTags.ts b/packages/shared/common/src/options/ApplicationTags.ts
@@ -27,27 +27,28 @@ const tagValidator = {
 export default class ApplicationTags {
   public readonly value?: string;
 
-  constructor(options: { application?: { id?: string; version?: string }; logger?: LDLogger }) {
+  constructor(options: {
+    application?: { id?: string; version?: string; name?: string; versionName?: string };
+    logger?: LDLogger;
+  }) {
     const tags: Record<string, string[]> = {};
     const application = options?.application;
-
-    if (application?.id !== null && application?.id !== undefined) {
-      const { valid, message } = tagValidator.is(application.id, 'application.id');
-
-      if (!valid) {
-        options.logger?.warn(message);
-      } else {
-        tags['application-id'] = [application.id];
-      }
-    }
-
-    if (application?.version !== null && application?.version !== undefined) {
-      const { valid, message } = tagValidator.is(application.version, 'application.version');
-      if (!valid) {
-        options.logger?.warn(message);
-      } else {
-        tags['application-version'] = [application.version];
-      }
+    const logger = options?.logger;
+
+    if (application) {
+      Object.entries(application).forEach(([key, value]) => {
+        if (value !== null && value !== undefined) {
+          const { valid, message } = tagValidator.is(value, `application.${key}`);
+
+          if (!valid) {
+            logger?.warn(message);
+          } else if (key === 'versionName') {
+            tags[`application-version-name`] = [value];
+          } else {
+            tags[`application-${key}`] = [value];
+          }
+        }
+      });
     }
 
     const tagKeys = Object.keys(tags);
diff --git a/packages/shared/sdk-client/src/api/LDOptions.ts b/packages/shared/sdk-client/src/api/LDOptions.ts
@@ -14,7 +14,7 @@ export interface LDOptions {
   /**
    * Information about the application where the LaunchDarkly SDK is running.
    */
-  application?: {
+  applicationInfo?: {
     /**
      * A unique identifier representing the application where the LaunchDarkly SDK is running.
      *
@@ -33,6 +33,22 @@ export interface LDOptions {
      * Example: `1.0.0` (standard version string) or `abcdef` (sha prefix)
      */
     version?: string;
+
+    /**
+     * A human-friendly application name representing the application where the LaunchDarkly SDK is running.
+     *
+     * This can be specified as any string value as long as it only uses the following characters: ASCII letters,
+     * ASCII digits, period, hyphen, underscore. A string containing any other characters will be ignored.
+     */
+    name?: string;
+
+    /**
+     * A human-friendly name representing the version of the application where the LaunchDarkly SDK is running.
+     *
+     * This can be specified as any string value as long as it only uses the following characters: ASCII letters,
+     * ASCII digits, period, hyphen, underscore. A string containing any other characters will be ignored.
+     */
+    versionName?: string;
   };
 
   /**
diff --git a/packages/shared/sdk-client/src/configuration/Configuration.ts b/packages/shared/sdk-client/src/configuration/Configuration.ts
@@ -39,7 +39,12 @@ export default class Configuration {
   public readonly privateAttributes: string[] = [];
 
   public readonly tags: ApplicationTags;
-  public readonly application?: { id?: string; version?: string };
+  public readonly applicationInfo?: {
+    id?: string;
+    version?: string;
+    name?: string;
+    versionName?: string;
+  };
   public readonly bootstrap?: 'localStorage' | LDFlagSet;
 
   // TODO: implement requestHeaderTransform
@@ -66,7 +71,7 @@ export default class Configuration {
       internalOptions.diagnosticEventPath,
       internalOptions.includeAuthorizationHeader,
     );
-    this.tags = new ApplicationTags({ application: this.application, logger: this.logger });
+    this.tags = new ApplicationTags({ application: this.applicationInfo, logger: this.logger });
   }
 
   validateTypesAndNames(pristineOptions: LDOptions): string[] {
diff --git a/packages/shared/sdk-client/src/configuration/validators.ts b/packages/shared/sdk-client/src/configuration/validators.ts
@@ -37,7 +37,7 @@ const validators: Record<keyof LDOptions, TypeValidator> = {
   }),
   privateAttributes: TypeValidators.StringArray,
 
-  application: TypeValidators.Object,
+  applicationInfo: TypeValidators.Object,
   bootstrap: new BootStrapValidator(),
   wrapperName: TypeValidators.String,
   wrapperVersion: TypeValidators.String,
diff --git a/packages/shared/sdk-client/src/utils/addAutoEnv.test.ts b/packages/shared/sdk-client/src/utils/addAutoEnv.test.ts
@@ -318,18 +318,24 @@ describe('automatic environment attributes', () => {
   });
 
   describe('addApplicationInfo', () => {
-    test('add application tags id, version', () => {
+    test('add id, version, name, versionName', () => {
       config = new Configuration({
-        application: { id: 'com.from-config.ld', version: '2.2.2' },
+        applicationInfo: {
+          id: 'com.from-config.ld',
+          version: '2.2.2',
+          name: 'test-ld-app-name',
+          versionName: 'test-ld-version-name',
+        },
       });
       const ldApplication = addApplicationInfo(basicPlatform, config);
 
       expect(ldApplication).toEqual({
         envAttributesVersion: '1.0',
         id: 'com.from-config.ld',
         key: '1234567890123456',
-        name: 'LDApplication.TestApp',
+        name: 'test-ld-app-name',
         version: '2.2.2',
+        versionName: 'test-ld-version-name',
       });
     });
 
@@ -368,15 +374,15 @@ describe('automatic environment attributes', () => {
       });
     });
 
-    test('omit if both tags and auto generated data are unavailable', () => {
+    test('omit if customer and auto env data are unavailable', () => {
       info.platformData = jest.fn().mockReturnValueOnce({});
 
       const ldApplication = addApplicationInfo(basicPlatform, config);
 
       expect(ldApplication).toBeUndefined();
     });
 
-    test('omit if tags unavailable and auto generated data are falsy', () => {
+    test('omit if customer unavailable and auto env data are falsy', () => {
       const mockData = info.platformData();
       info.platformData = jest.fn().mockReturnValueOnce({
         ld_application: {
@@ -392,7 +398,7 @@ describe('automatic environment attributes', () => {
       expect(ldApplication).toBeUndefined();
     });
 
-    test('omit if tags unavailable and auto generated data only contains key and attributesVersion', () => {
+    test('omit if customer data is unavailable and auto env data only contains key and attributesVersion', () => {
       info.platformData = jest.fn().mockReturnValueOnce({
         ld_application: { key: 'key-from-sdk', envAttributesVersion: '0.0.1' },
       });
@@ -406,7 +412,7 @@ describe('automatic environment attributes', () => {
       info.platformData = jest
         .fn()
         .mockReturnValueOnce({ ld_application: { version: null, locale: '' } });
-      config = new Configuration({ application: { version: '1.2.3' } });
+      config = new Configuration({ applicationInfo: { version: '1.2.3' } });
       const ldApplication = addApplicationInfo(basicPlatform, config);
 
       expect(ldApplication).toBeUndefined();
diff --git a/packages/shared/sdk-client/src/utils/addAutoEnv.ts b/packages/shared/sdk-client/src/utils/addAutoEnv.ts
@@ -31,28 +31,34 @@ export const toMulti = (c: LDSingleKindContext) => {
  *
  * @param crypto
  * @param info
- * @param applicationTags
+ * @param applicationInfo
  * @param config
  * @return An LDApplication object with populated key, envAttributesVersion, id and version.
  */
 export const addApplicationInfo = (
   { crypto, info }: Platform,
-  { application: applicationTags }: Configuration,
+  { applicationInfo }: Configuration,
 ): LDApplication | undefined => {
   const { ld_application } = info.platformData();
-  const app = deepCompact<LDApplication>(ld_application) ?? ({} as LDApplication);
-  const id = applicationTags?.id || app?.id;
+  let app = deepCompact<LDApplication>(ld_application) ?? ({} as LDApplication);
+  const id = applicationInfo?.id || app?.id;
 
   if (id) {
-    app.id = id;
+    const version = applicationInfo?.version || app?.version;
+    const name = applicationInfo?.name || app?.name;
+    const versionName = applicationInfo?.versionName || app?.versionName;
 
-    const version = applicationTags?.version || app?.version;
-    if (version) {
-      app.version = version;
-    }
+    app = {
+      ...app,
+      id,
+      // only add props if they are defined
+      ...(version ? { version } : {}),
+      ...(name ? { name } : {}),
+      ...(versionName ? { versionName } : {}),
+    };
 
     const hasher = crypto.createHash('sha256');
-    hasher.update(app.id);
+    hasher.update(id);
     app.key = hasher.digest('base64');
     app.envAttributesVersion = app.envAttributesVersion || defaultAutoEnvSchemaVersion;
 
diff --git a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts
@@ -347,8 +347,19 @@ describe('when setting different options', () => {
   // This is more thoroughly tested in the application tags test.
   it.each([
     [{ application: { id: 'valid-id', version: 'valid-version' } }, 0],
+    [
+      {
+        application: {
+          id: 'valid-id',
+          version: 'valid-version',
+          name: 'valid-name',
+          versionName: 'valid-versionName',
+        },
+      },
+      0,
+    ],
     [{ application: 'tomato' }, 1],
-  ])('handles application tag settings', (values, warnings) => {
+  ])('handles application tag settings %j', (values, warnings) => {
     // @ts-ignore
     const config = new Configuration(withLogger({ ...values }));
     expect(logger(config).getCount()).toEqual(warnings);
diff --git a/packages/shared/sdk-server/src/api/options/LDOptions.ts b/packages/shared/sdk-server/src/api/options/LDOptions.ts
diff --git a/packages/shared/sdk-server/src/options/ValidatedOptions.ts b/packages/shared/sdk-server/src/options/ValidatedOptions.ts
