diff --git a/.circleci/config.yml b/.circleci/config.yml index b92b46f..172cc1c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -41,6 +41,9 @@ jobs: default: false docker-image: type: string + run-contract-tests: + type: boolean + default: true docker: - image: <> steps: @@ -56,6 +59,14 @@ jobs: condition: <> steps: - run: npm run lint + - when: + condition: <> + steps: + - run: + command: npm run contract-test-service + background: true + - run: mkdir -p reports/junit + - run: TEST_HARNESS_PARAMS="-junit reports/junit/contract-test-results.xml -skip-from contract-tests/testharness-suppressions.txt" npm run contract-test-harness - run: name: dependency audit command: ./scripts/better-audit.sh diff --git a/.gitignore b/.gitignore index 4c3aeeb..1a6aa78 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,7 @@ package-lock.json /docs/build/ /local/ -/node_modules/ +**/node_modules/ junit.xml npm-debug.log test-types.js diff --git a/configuration.js b/configuration.js index 112fe26..8f0bb3c 100644 --- a/configuration.js +++ b/configuration.js @@ -31,7 +31,7 @@ module.exports = (function () { const typesForPropertiesWithNoDefault = { // Add a value here if we add a configuration property whose type cannot be determined by looking // in baseDefaults (for instance, the default is null but if the value isn't null it should be a - // string). The allowable values are 'boolean', 'string', 'number', 'object', 'function', or + // string). The allowable values are 'boolean', 'string', 'number', 'object', 'function', 'array' or // 'factory' (the last one means it can be either a function or an object). bigSegments: 'object', eventProcessor: 'object', @@ -48,6 +48,42 @@ module.exports = (function () { wrapperVersion: 'string', }; + /** + * Expression to validate characters that are allowed in tag keys and values. + */ + const allowedTagCharacters = /^(\w|\.|-)+$/; + + /** + * Verify that a value meets the requirements for a tag value. + * @param {Object} config + * @param {string} tagValue + */ + function validateTagValue(name, config, tagValue) { + if (typeof tagValue !== 'string' || !tagValue.match(allowedTagCharacters)) { + config.logger.warn(messages.invalidTagValue(name)); + return undefined; + } + return tagValue; + } + + const optionsWithValidatorsOrConversions = { + // Add a value here if we add a configuration property which requires custom validation + // and/or type conversion. + // The validator should log a message for any validation issues encountered. + // The validator should return undefined, or the validated value. + + application: (name, config, value) => { + const validated = {}; + if (value.id) { + validated.id = validateTagValue(`${name}.id`, config, value.id); + } + if (value.version) { + validated.version = validateTagValue(`${name}.version`, config, value.version); + } + return validated; + }, + }; + /* eslint-disable camelcase */ const deprecatedOptions = {}; /* eslint-enable camelcase */ @@ -102,8 +138,16 @@ module.exports = (function () { if (value !== null && value !== undefined) { const defaultValue = defaultConfig[name]; const typeDesc = typesForPropertiesWithNoDefault[name]; - if (defaultValue === undefined && typeDesc === undefined) { + const validator = optionsWithValidatorsOrConversions[name]; + if (defaultValue === undefined && typeDesc === undefined && validator === undefined) { config.logger.warn(messages.unknownOption(name)); + } else if (validator !== undefined) { + const validated = validator(name, config, config[name]); + if (validated !== undefined) { + config[name] = validated; + } else { + delete config[name]; + } } else { const expectedType = typeDesc || typeDescForValue(defaultValue); const actualType = typeDescForValue(value); @@ -156,8 +200,29 @@ module.exports = (function () { return config; } + /** + * Get tags for the specified configuration. + * + * If any additional tags are added to the configuration, then the tags from + * this method should be extended with those. + * @param {Object} config The already valiated configuration. + * @returns {Object} The tag configuration. + */ + function getTags(config) { + const tags = {}; + if (config.application && config.application.id !== undefined && config.application.id !== null) { + tags['application-id'] = [config.application.id]; + } + if (config.application && config.application.version !== undefined && config.application.id !== null) { + tags['application-version'] = [config.application.version]; + } + + return tags; + } + return { - validate: validate, - defaults: defaults, + validate, + defaults, + getTags, }; })(); diff --git a/contract-tests/README.md b/contract-tests/README.md new file mode 100644 index 0000000..70965d7 --- /dev/null +++ b/contract-tests/README.md @@ -0,0 +1,7 @@ +# SDK contract test service + +This directory contains an implementation of the cross-platform SDK testing protocol defined by https://github.com/launchdarkly/sdk-test-harness. See that project's `README` for details of this protocol, and the kinds of SDK capabilities that are relevant to the contract tests. This code should not need to be updated unless the SDK has added or removed such capabilities. + +To run these tests locally, run `npm run contract-tests` from the SDK project root directory. This will start the test service, download the correct version of the test harness tool, and run the tests. + +Or, to test against an in-progress local version of the test harness, run `npm run contract-test-service` from the SDK project root directory; then, in the root directory of the `sdk-test-harness` project, build the test harness and run it from the command line. diff --git a/contract-tests/index.js b/contract-tests/index.js new file mode 100644 index 0000000..77752fb --- /dev/null +++ b/contract-tests/index.js @@ -0,0 +1,108 @@ +const express = require('express'); +const bodyParser = require('body-parser'); + +const { Log } = require('./log'); +const { newSdkClientEntity, badCommandError } = require('./sdkClientEntity'); + +const app = express(); +let server = null; + +const port = 8000; + +let clientCounter = 0; +const clients = {}; + +const mainLog = Log('service'); + +app.use(bodyParser.json()); + +app.get('/', (req, res) => { + res.header('Content-Type', 'application/json'); + res.json({ + capabilities: [ + 'server-side', + 'all-flags-client-side-only', + 'all-flags-details-only-for-tracked-flags', + 'all-flags-with-reasons', + 'tags', + ], + }); +}); + +app.delete('/', (req, res) => { + mainLog.info('Test service has told us to exit'); + res.status(204); + res.send(); + + // Defer the following actions till after the response has been sent + setTimeout(() => { + server.close(() => process.exit()); + // We force-quit with process.exit because, even after closing the server, there could be some + // scheduled tasks lingering if an SDK instance didn't get cleaned up properly, and we don't want + // that to prevent us from quitting. + }, 1); +}); + +app.post('/', async (req, res) => { + const options = req.body; + + clientCounter += 1; + const clientId = clientCounter.toString(); + const resourceUrl = `/clients/${clientId}`; + + try { + const client = await newSdkClientEntity(options); + clients[clientId] = client; + + res.status(201); + res.set('Location', resourceUrl); + } catch (e) { + res.status(500); + const message = e.message || JSON.stringify(e); + mainLog.error('Error creating client: ' + message); + res.write(message); + } + res.send(); +}); + +app.post('/clients/:id', async (req, res) => { + const client = clients[req.params.id]; + if (!client) { + res.status(404); + } else { + try { + const respValue = await client.doCommand(req.body); + if (respValue) { + res.status(200); + res.write(JSON.stringify(respValue)); + } else { + res.status(204); + } + } catch (e) { + const isBadRequest = e === badCommandError; + res.status(isBadRequest ? 400 : 500); + res.write(e.message || JSON.stringify(e)); + if (!isBadRequest && e.stack) { + console.log(e.stack); + } + } + } + res.send(); +}); + +app.delete('/clients/:id', async (req, res) => { + const client = clients[req.params.id]; + if (!client) { + res.status(404); + res.send(); + } else { + client.close(); + delete clients[req.params.id]; + res.status(204); + res.send(); + } +}); + +server = app.listen(port, () => { + console.log('Listening on port %d', port); +}); diff --git a/contract-tests/log.js b/contract-tests/log.js new file mode 100644 index 0000000..72cecff --- /dev/null +++ b/contract-tests/log.js @@ -0,0 +1,23 @@ +const ld = require('launchdarkly-node-server-sdk'); + +function Log(tag) { + function doLog(level, message) { + console.log(new Date().toISOString() + ` [${tag}] ${level}: ${message}`); + } + return { + info: message => doLog('info', message), + error: message => doLog('error', message), + }; +} + +function sdkLogger(tag) { + return ld.basicLogger({ + level: 'debug', + destination: line => { + console.log(new Date().toISOString() + ` [${tag}.sdk] ${line}`); + }, + }); +} + +module.exports.Log = Log; +module.exports.sdkLogger = sdkLogger; diff --git a/contract-tests/package.json b/contract-tests/package.json new file mode 100644 index 0000000..998b865 --- /dev/null +++ b/contract-tests/package.json @@ -0,0 +1,15 @@ +{ + "name": "node-server-sdk-contract-tests", + "version": "0.0.0", + "main": "index.js", + "scripts": { + "start": "node index.js" + }, + "author": "", + "license": "Apache-2.0", + "dependencies": { + "body-parser": "^1.19.0", + "express": "^4.17.1", + "launchdarkly-node-server-sdk": "file:.." + } +} diff --git a/contract-tests/sdkClientEntity.js b/contract-tests/sdkClientEntity.js new file mode 100644 index 0000000..e118fa6 --- /dev/null +++ b/contract-tests/sdkClientEntity.js @@ -0,0 +1,115 @@ +const ld = require('launchdarkly-node-server-sdk'); + +const { Log, sdkLogger } = require('./log'); + +const badCommandError = new Error('unsupported command'); + +function makeSdkConfig(options, tag) { + const cf = { + logger: sdkLogger(tag), + }; + const maybeTime = seconds => (seconds === undefined || seconds === null ? undefined : seconds / 1000); + if (options.streaming) { + cf.streamUri = options.streaming.baseUri; + cf.streamInitialReconnectDelay = maybeTime(options.streaming.initialRetryDelayMs); + } + if (options.events) { + cf.allAttributesPrivate = options.events.allAttributesPrivate; + cf.eventsUri = options.events.baseUri; + cf.capacity = options.events.capacity; + cf.diagnosticOptOut = !options.events.enableDiagnostics; + cf.flushInterval = maybeTime(options.events.flushIntervalMs); + cf.inlineUsersInEvents = options.events.inlineUsers; + cf.privateAttributeNames = options.events.globalPrivateAttributes; + } + if (options.tags) { + cf.application = { + id: options.tags.applicationId, + version: options.tags.applicationVersion, + }; + } + return cf; +} + +async function newSdkClientEntity(options) { + const c = {}; + const log = Log(options.tag); + + log.info('Creating client with configuration: ' + JSON.stringify(options.configuration)); + const timeout = + options.configuration.startWaitTimeMs !== null && options.configuration.startWaitTimeMs !== undefined + ? options.configuration.startWaitTimeMs + : 5000; + const client = ld.init( + options.configuration.credential || 'unknown-sdk-key', + makeSdkConfig(options.configuration, options.tag) + ); + try { + await Promise.race([client.waitForInitialization(), new Promise(resolve => setTimeout(resolve, timeout))]); + } catch (_) { + // if waitForInitialization() rejects, the client failed to initialize, see next line + } + if (!client.initialized() && !options.configuration.initCanFail) { + client.close(); + throw new Error('client initialization failed'); + } + + c.close = () => { + client.close(); + log.info('Test ended'); + }; + + c.doCommand = async params => { + log.info('Received command: ' + params.command); + switch (params.command) { + case 'evaluate': { + const pe = params.evaluate; + if (pe.detail) { + return await client.variationDetail(pe.flagKey, pe.user, pe.defaultValue); + } else { + const value = await client.variation(pe.flagKey, pe.user, pe.defaultValue); + return { value }; + } + } + + case 'evaluateAll': { + const pea = params.evaluateAll; + const eao = { + clientSideOnly: pea.clientSideOnly, + detailsOnlyForTrackedFlags: pea.detailsOnlyForTrackedFlags, + withReasons: pea.withReasons, + }; + return { state: await client.allFlagsState(pea.user, eao) }; + } + + case 'identifyEvent': + client.identify(params.identifyEvent.user); + return undefined; + + case 'customEvent': { + const pce = params.customEvent; + client.track(pce.eventKey, pce.user, pce.data, pce.metricValue); + return undefined; + } + + case 'aliasEvent': + client.alias(params.aliasEvent.user, params.aliasEvent.previousUser); + return undefined; + + case 'flushEvents': + client.flush(); + return undefined; + + case 'getBigSegmentStoreStatus': + return undefined; + + default: + throw badCommandError; + } + }; + + return c; +} + +module.exports.newSdkClientEntity = newSdkClientEntity; +module.exports.badCommandError = badCommandError; diff --git a/contract-tests/testharness-suppressions.txt b/contract-tests/testharness-suppressions.txt new file mode 100644 index 0000000..8a29da9 --- /dev/null +++ b/contract-tests/testharness-suppressions.txt @@ -0,0 +1,2 @@ +streaming/validation/drop and reconnect if stream event has malformed JSON +streaming/validation/drop and reconnect if stream event has well-formed JSON not matching schema diff --git a/event_processor.js b/event_processor.js index 7acc75c..504a465 100644 --- a/event_processor.js +++ b/event_processor.js @@ -76,7 +76,7 @@ function EventProcessor(sdkKey, config, errorReporter, diagnosticsManager) { if (event.variation !== undefined && event.variation !== null) { out.variation = event.variation; } - if (event.version) { + if (event.version !== undefined && event.version !== null) { out.version = event.version; } if (event.reason) { diff --git a/event_summarizer.js b/event_summarizer.js index c395915..113a5e9 100644 --- a/event_summarizer.js +++ b/event_summarizer.js @@ -53,7 +53,7 @@ function EventSummarizer() { if (c.variation !== undefined && c.variation !== null) { counterOut.variation = c.variation; } - if (c.version) { + if (c.version !== undefined && c.version !== null) { counterOut.version = c.version; } else { counterOut.unknown = true; diff --git a/index.d.ts b/index.d.ts index e618e38..d81a24f 100644 --- a/index.d.ts +++ b/index.d.ts @@ -429,6 +429,31 @@ declare module 'launchdarkly-node-server-sdk' { * the User-Agent headers along with the `wrapperName` during requests to the LaunchDarkly servers. */ wrapperVersion?: string; + + /** + * Information about the application where the LaunchDarkly SDK is running. + */ + application?: { + /** + * A unique identifier 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. + * + * Example: `authentication-service` + */ + id?: string; + + /** + * A unique identifier 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. + * + * Example: `1.0.0` (standard version string) or `abcdef` (sha prefix) + */ + version?: string; + } } /** diff --git a/messages.js b/messages.js index ce6e0de..0912a6c 100644 --- a/messages.js +++ b/messages.js @@ -16,7 +16,7 @@ exports.httpErrorMessage = (err, context, retryMessage) => { exports.missingUserKeyNoEvent = () => 'User was unspecified or had no key; event will not be sent'; exports.optionBelowMinimum = (name, value, min) => - `Config option "${name}' had invalid value of ${value}, using minimum of ${min} instead`; + `Config option "${name}" had invalid value of ${value}, using minimum of ${min} instead`; exports.unknownOption = name => `Ignoring unknown config option "${name}"`; @@ -25,3 +25,5 @@ exports.wrongOptionType = (name, expectedType, actualType) => exports.wrongOptionTypeBoolean = (name, actualType) => `Config option "${name}" should be a boolean, got ${actualType}, converting to boolean`; + +exports.invalidTagValue = name => `Config option "${name}" must only contain letters, numbers, ., _ or -.`; diff --git a/package.json b/package.json index 0d29032..b768ccc 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,10 @@ "scripts": { "test": "jest --ci", "check-typescript": "node_modules/typescript/bin/tsc", - "lint": "eslint --format 'node_modules/eslint-formatter-pretty' --ignore-path .eslintignore ." + "lint": "eslint --format 'node_modules/eslint-formatter-pretty' --ignore-path .eslintignore .", + "contract-test-service": "npm --prefix contract-tests install && npm --prefix contract-tests start", + "contract-test-harness": "curl -s https://raw.githubusercontent.com/launchdarkly/sdk-test-harness/master/downloader/run.sh \\ | VERSION=v1 PARAMS=\"-url http://localhost:8000 -debug -stop-service-at-end $TEST_HARNESS_PARAMS\" sh", + "contract-tests": "npm run contract-test-service & npm run contract-test-harness" }, "types": "./index.d.ts", "repository": { @@ -25,7 +28,7 @@ "homepage": "https://github.com/launchdarkly/node-server-sdk", "dependencies": { "async": "^3.1.0", - "launchdarkly-eventsource": "1.4.2", + "launchdarkly-eventsource": "1.4.4", "lru-cache": "^6.0.0", "node-cache": "^5.1.0", "semver": "^7.3.0", diff --git a/test-types.ts b/test-types.ts index feafa11..b457c36 100644 --- a/test-types.ts +++ b/test-types.ts @@ -40,7 +40,11 @@ var allOptions: ld.LDOptions = { diagnosticOptOut: true, diagnosticRecordingInterval: 100, wrapperName: 'x', - wrapperVersion: 'y' + wrapperVersion: 'y', + application: { + id: 'test-id', + version: 'test-version' + } }; var userWithKeyOnly: ld.LDUser = { key: 'user' }; var anonymousUser: ld.LDUser = { key: 'anon-user', anonymous: true }; diff --git a/test/configuration-test.js b/test/configuration-test.js index 97e7803..bd96ce9 100644 --- a/test/configuration-test.js +++ b/test/configuration-test.js @@ -182,4 +182,35 @@ describe('configuration', function() { expect(() => configuration.validate(configIn)).toThrow(/Provided logger instance must support .* method/); }); }); + + it('handles a valid application id', () => { + const configIn = emptyConfigWithMockLogger(); + configIn.application = {id: 'test-application'}; + expect(configuration.validate(configIn).application.id).toEqual('test-application'); + }); + + it('logs a warning with an invalid application id', () => { + const configIn = emptyConfigWithMockLogger(); + configIn.application = {id: 'test #$#$#'}; + expect(configuration.validate(configIn).application.id).toBeUndefined(); + expect(configIn.logger.warn).toHaveBeenCalledTimes(1); + }); + + it('handles a valid application version', () => { + const configIn = emptyConfigWithMockLogger(); + configIn.application = {version: 'test-version'}; + expect(configuration.validate(configIn).application.version).toEqual('test-version'); + }); + + it('logs a warning with an invalid application version', () => { + const configIn = emptyConfigWithMockLogger(); + configIn.application = {version: 'test #$#$#'}; + expect(configuration.validate(configIn).application.version).toBeUndefined(); + expect(configIn.logger.warn).toHaveBeenCalledTimes(1); + }); + + it('includes application id and version in tags when present', () => { + expect(configuration.getTags({application: {id: 'test-id', version: 'test-version'}})) + .toEqual({'application-id': ['test-id'], 'application-version': ['test-version']}); + }); }); diff --git a/test/event_processor-test.js b/test/event_processor-test.js index 59e1c1b..a509652 100644 --- a/test/event_processor-test.js +++ b/test/event_processor-test.js @@ -182,6 +182,21 @@ describe('EventProcessor', () => { }); })); + it('handles the version being 0', eventsServerTest(async s => { + await withEventProcessor(defaultConfig, s, async ep => { + const e = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey', + version: 0, variation: 1, value: 'value', trackEvents: true }; + ep.sendEvent(e); + await ep.flush(); + + const output = await getJsonRequest(s); + expect(output.length).toEqual(3); + checkIndexEvent(output[0], e, user); + checkFeatureEvent(output[1], e, false); + checkSummaryEvent(output[2]); + }); + })); + it('queues individual feature event with index event for anonymous user', eventsServerTest(async s => { await withEventProcessor(defaultConfig, s, async ep => { const e = { kind: 'feature', creationDate: 1000, user: anonUser, key: 'flagkey', diff --git a/test/event_summarizer-test.js b/test/event_summarizer-test.js index fba7e50..79f4895 100644 --- a/test/event_summarizer-test.js +++ b/test/event_summarizer-test.js @@ -44,15 +44,24 @@ describe('EventSummarizer', function() { variation: 1, value: 100, default: 111 }; var event5 = { kind: 'feature', creationDate: 1000, key: 'badkey', user: user, value: 333, default: 333 }; + var event6 = { kind: 'feature', creationDate: 1000, key: 'zero-version', version: 0, user: user, + variation: 1, value: 100, default: 444 }; es.summarizeEvent(event1); es.summarizeEvent(event2); es.summarizeEvent(event3); es.summarizeEvent(event4); es.summarizeEvent(event5); + es.summarizeEvent(event6); var data = es.getSummary(); data.features.key1.counters.sort(function(a, b) { return a.value - b.value; }); var expectedFeatures = { + 'zero-version': { + default: 444, + counters: [ + { variation: 1, value: 100, version: 0, count: 1} + ] + }, key1: { default: 111, counters: [ @@ -67,7 +76,7 @@ describe('EventSummarizer', function() { badkey: { default: 333, counters: [ { value: 333, unknown: true, count: 1 }] - } + }, }; expect(data.features).toEqual(expectedFeatures); }); diff --git a/utils/__tests__/httpUtils-test.js b/utils/__tests__/httpUtils-test.js index df074f5..7361060 100644 --- a/utils/__tests__/httpUtils-test.js +++ b/utils/__tests__/httpUtils-test.js @@ -25,3 +25,15 @@ it('sets wrapper header with name and version', () => { const h = httpUtils.getDefaultHeaders('my-sdk-key', { wrapperName: 'my-wrapper', wrapperVersion: '2.0' }); expect(h).toMatchObject({ 'x-launchdarkly-wrapper': 'my-wrapper/2.0' }); }); + +it('sets the X-LaunchDarkly-Tags header with valid tags.', () => { + const h = httpUtils.getDefaultHeaders('my-sdk-key', { + application: { + id: 'test-application', + version: 'test-version', + }, + }); + expect(h).toMatchObject({ + 'x-launchdarkly-tags': 'application-id/test-application application-version/test-version', + }); +}); diff --git a/utils/httpUtils.js b/utils/httpUtils.js index 20e191b..313658f 100644 --- a/utils/httpUtils.js +++ b/utils/httpUtils.js @@ -1,6 +1,7 @@ const http = require('http'); const https = require('https'); const url = require('url'); +const configuration = require('../configuration'); const packageJson = require('../package.json'); @@ -18,6 +19,16 @@ function getDefaultHeaders(sdkKey, config) { ? config.wrapperName + '/' + config.wrapperVersion : config.wrapperName; } + const tags = configuration.getTags(config); + const tagKeys = Object.keys(tags); + if (tagKeys.length) { + ret['x-launchdarkly-tags'] = tagKeys + .sort() + .flatMap(key => + Array.isArray(tags[key]) ? tags[key].sort().map(value => `${key}/${value}`) : [`${key}/${tags[key]}`] + ) + .join(' '); + } return ret; }