From b5dce9896898b9c1d94359562bb57ef8f936fa03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Mon, 15 Apr 2024 11:20:27 +0000 Subject: [PATCH] Support configuration arguments via `--sentry-define` (#198) --- CHANGELOG.md | 1 + README.md | 8 + lib/src/configuration.dart | 80 +++++----- lib/src/configuration_values.dart | 161 +++++++++++++++++++ test/configuration_test.dart | 225 +++++++++++++++++++++++++++ test/configureation_values_test.dart | 141 +++++++++++++++++ 6 files changed, 575 insertions(+), 41 deletions(-) create mode 100644 lib/src/configuration_values.dart create mode 100644 test/configuration_test.dart create mode 100644 test/configureation_values_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index b03c6dc..872b5ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Update env/config `release` and `dist` behaviour ([#217](https://github.com/getsentry/sentry-dart-plugin/pull/217)) - This is a breaking change, please check if the `release` and `dist` values are as you expect them. - Add option to provide alternative binary directory ([#214](https://github.com/getsentry/sentry-dart-plugin/pull/214)) +- Support configuration arguments via `--sentry-define` ([#198](https://github.com/getsentry/sentry-dart-plugin/pull/198)) ### Dependencies diff --git a/README.md b/README.md index b41c608..e539bb9 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,14 @@ sentry: ignore_missing: true ``` +You can also override or extend your file based configuration by passing the parameters as arguments +in the format `--sentry-define==`. They take precedence over your file based parameters, +but not over the alternative environment variables. + +```bash +flutter packages pub run sentry_dart_plugin --sentry-define=release=app-internal-test@0.0.1 +``` + ### sentry.properties Create a `sentry.properties` file at the root of your project: diff --git a/lib/src/configuration.dart b/lib/src/configuration.dart index cf7f475..cb3cad8 100644 --- a/lib/src/configuration.dart +++ b/lib/src/configuration.dart @@ -6,6 +6,7 @@ import 'package:system_info2/system_info2.dart'; import 'cli/host_platform.dart'; import 'cli/setup.dart'; +import 'configuration_values.dart'; import 'utils/config-reader/config_reader.dart'; import 'utils/extensions.dart'; import 'utils/injector.dart'; @@ -48,13 +49,13 @@ class Configuration { // the Sentry CLI path, defaults to the assets folder late String? cliPath; - /// The Apps release name, defaults to 'name@version+buildNumber' from pubspec or set via env. var. SENTRY_RELEASE + /// The Apps release name, defaults to 'name@version+buildNumber' from SENTRY_RELEASE env. variable, arguments or pubspec. /// Example, name: 'my_app', version: 2.0.0+1, in this case the release is my_app@2.0.0+1 /// This field has precedence over the [name] from pubspec /// If this field has a build number, it has precedence over the [version]'s build number from pubspec late String? release; - /// The Apps dist/build number, taken from pubspec dist or SENTRY_DIST env. variable + /// The Apps dist/build number, taken from SENTRY_DIST env. variable, arguments or pubspec. /// If provided, it will override the build number from [version] late String? dist; @@ -85,63 +86,60 @@ class Configuration { late String binDir; /// Loads the configuration values - Future getConfigValues(List arguments) async { + Future getConfigValues(List cliArguments) async { const taskName = 'reading config values'; Log.startingTask(taskName); - final reader = ConfigReader(); - loadConfig(reader); + loadConfig( + argsConfig: ConfigurationValues.fromArguments(cliArguments), + fileConfig: ConfigurationValues.fromReader(ConfigReader()), + platformEnvConfig: ConfigurationValues.fromPlatformEnvironment( + Platform.environment, + ), + ); await _findAndSetCliPath(); Log.taskCompleted(taskName); } - void loadConfig(ConfigReader reader) { - final environments = Platform.environment; + void loadConfig({ + required ConfigurationValues platformEnvConfig, + required ConfigurationValues argsConfig, + required ConfigurationValues fileConfig, + }) { final pubspec = ConfigReader.getPubspec(); - String? envRelease = environments['SENTRY_RELEASE']; - if (envRelease?.isEmpty ?? false) { - envRelease = null; - } - - String? envDist = environments['SENTRY_DIST']; - if (envDist?.isEmpty ?? false) { - envDist = null; - } - - release = envRelease ?? reader.getString('release'); - dist = envDist ?? reader.getString('dist'); - version = pubspec['version'].toString(); - name = pubspec['name'].toString(); - - uploadDebugSymbols = reader.getBool('upload_debug_symbols', - deprecatedKey: 'upload_native_symbols') ?? - true; - uploadSourceMaps = reader.getBool('upload_source_maps') ?? false; - uploadSources = reader.getBool('upload_sources', - deprecatedKey: 'include_native_sources') ?? - false; - commits = (reader.getString('commits') ?? 'auto').toString(); - ignoreMissing = reader.getBool('ignore_missing') ?? false; + final configValues = ConfigurationValues.merged( + args: argsConfig, + file: fileConfig, + platformEnv: platformEnvConfig, + ); + + release = configValues.release; + dist = configValues.dist; + version = configValues.version ?? pubspec['version'].toString(); + name = configValues.name ?? pubspec['name'].toString(); + uploadDebugSymbols = configValues.uploadDebugSymbols ?? true; + uploadSourceMaps = configValues.uploadSourceMaps ?? false; + uploadSources = configValues.uploadSources ?? false; + commits = configValues.commits ?? 'auto'; + ignoreMissing = configValues.ignoreMissing ?? false; // uploading JS and Map files need to have the correct folder structure // otherwise symbolication fails, the default path for the web build folder is build/web // but can be customized so making it flexible. final webBuildPath = - reader.getString('web_build_path') ?? _fs.path.join('build', 'web'); + configValues.webBuildPath ?? _fs.path.join('build', 'web'); webBuildFilesFolder = _fs.path.join(buildFilesFolder, webBuildPath); - project = reader.getString('project'); // or env. var. SENTRY_PROJECT - org = reader.getString('org'); // or env. var. SENTRY_ORG - waitForProcessing = reader.getBool('wait_for_processing') ?? false; - authToken = - reader.getString('auth_token'); // or env. var. SENTRY_AUTH_TOKEN - url = reader.getString('url'); // or env. var. SENTRY_URL - logLevel = reader.getString('log_level'); // or env. var. SENTRY_LOG_LEVEL - binDir = - reader.getString('bin_dir') ?? '.dart_tool/pub/bin/sentry_dart_plugin'; + project = configValues.project; // or env. var. SENTRY_PROJECT + org = configValues.org; // or env. var. SENTRY_ORG + waitForProcessing = configValues.waitForProcessing ?? false; + authToken = configValues.authToken; // or env. var. SENTRY_AUTH_TOKEN + url = configValues.url; // or env. var. SENTRY_URL + logLevel = configValues.logLevel; // or env. var. SENTRY_LOG_LEVEL + binDir = configValues.binDir ?? '.dart_tool/pub/bin/sentry_dart_plugin'; } /// Validates the configuration values and log an error if required fields diff --git a/lib/src/configuration_values.dart b/lib/src/configuration_values.dart new file mode 100644 index 0000000..bc7c0d7 --- /dev/null +++ b/lib/src/configuration_values.dart @@ -0,0 +1,161 @@ +// https://stackoverflow.com/a/73564526 +import 'package:sentry_dart_plugin/src/utils/config-reader/config_reader.dart'; + +class ConfigurationValues { + final String? version; + final String? name; + + final bool? uploadDebugSymbols; + final bool? uploadSourceMaps; + final bool? uploadSources; + final String? project; + final String? org; + final String? authToken; + final String? url; + final bool? waitForProcessing; + final String? logLevel; + final String? release; + final String? dist; + final String? webBuildPath; + final String? commits; + final bool? ignoreMissing; + final String? binDir; + + ConfigurationValues({ + this.version, + this.name, + this.uploadDebugSymbols, + this.uploadSourceMaps, + this.uploadSources, + this.project, + this.org, + this.authToken, + this.url, + this.waitForProcessing, + this.logLevel, + this.release, + this.dist, + this.webBuildPath, + this.commits, + this.ignoreMissing, + this.binDir, + }); + + factory ConfigurationValues.fromArguments(List arguments) { + Map sentryArguments = {}; + for (final arg in arguments) { + final components = arg.split("="); + if (components.length < 3) { + continue; + } + if (components[0] != "--sentry-define") { + continue; + } + sentryArguments[components[1]] = components.sublist(2).join('='); + } + boolFromString(String? value) { + return value == "true" + ? true + : value == "false" + ? false + : null; + } + + return ConfigurationValues( + version: sentryArguments['version'], + name: sentryArguments['name'], + uploadDebugSymbols: boolFromString( + sentryArguments['upload_debug_symbols'] ?? + sentryArguments['upload_native_symbols'], + ), + uploadSourceMaps: boolFromString(sentryArguments['upload_source_maps']), + uploadSources: boolFromString( + sentryArguments['upload_sources'] ?? + sentryArguments['include_native_sources'], + ), + project: sentryArguments['project'], + org: sentryArguments['org'], + authToken: sentryArguments['auth_token'], + url: sentryArguments['url'], + waitForProcessing: + boolFromString(sentryArguments['wait_for_processing']), + logLevel: sentryArguments['log_level'], + release: sentryArguments['release'], + dist: sentryArguments['dist'], + webBuildPath: sentryArguments['web_build_path'], + commits: sentryArguments['commits'], + ignoreMissing: boolFromString(sentryArguments['ignore_missing']), + binDir: sentryArguments['bin_dir']); + } + + factory ConfigurationValues.fromReader(ConfigReader configReader) { + return ConfigurationValues( + version: configReader.getString('version'), + name: configReader.getString('name'), + uploadDebugSymbols: configReader.getBool( + 'upload_debug_symbols', + deprecatedKey: 'upload_native_symbols', + ), + uploadSourceMaps: configReader.getBool('upload_source_maps'), + uploadSources: configReader.getBool( + 'upload_sources', + deprecatedKey: 'include_native_sources', + ), + project: configReader.getString('project'), + org: configReader.getString('org'), + authToken: configReader.getString('auth_token'), + url: configReader.getString('url'), + waitForProcessing: configReader.getBool('wait_for_processing'), + logLevel: configReader.getString('log_level'), + release: configReader.getString('release'), + dist: configReader.getString('dist'), + webBuildPath: configReader.getString('web_build_path'), + commits: configReader.getString('commits'), + ignoreMissing: configReader.getBool('ignore_missing'), + binDir: configReader.getString('bin_dir'), + ); + } + + factory ConfigurationValues.fromPlatformEnvironment( + Map environment, + ) { + String? envRelease = environment['SENTRY_RELEASE']; + if (envRelease?.isEmpty ?? false) { + envRelease = null; + } + String? envDist = environment['SENTRY_DIST']; + if (envDist?.isEmpty ?? false) { + envDist = null; + } + return ConfigurationValues( + release: envRelease, + dist: envDist, + ); + } + + factory ConfigurationValues.merged({ + required ConfigurationValues platformEnv, + required ConfigurationValues args, + required ConfigurationValues file, + }) { + return ConfigurationValues( + version: args.version ?? file.version, + name: args.name ?? file.name, + uploadDebugSymbols: args.uploadDebugSymbols ?? file.uploadDebugSymbols, + uploadSourceMaps: args.uploadSourceMaps ?? file.uploadSourceMaps, + uploadSources: args.uploadSources ?? file.uploadSources, + project: args.project ?? file.project, + org: args.org ?? file.org, + authToken: args.authToken ?? file.authToken, + url: args.url ?? file.url, + waitForProcessing: args.waitForProcessing ?? file.waitForProcessing, + logLevel: args.logLevel ?? file.logLevel, + release: platformEnv.release ?? args.release ?? file.release, + dist: platformEnv.dist ?? args.dist ?? file.dist, + webBuildPath: args.webBuildPath ?? file.webBuildPath, + commits: args.commits ?? file.commits, + ignoreMissing: args.ignoreMissing ?? file.ignoreMissing, + binDir: args.binDir ?? file.binDir, + ); + } +} diff --git a/test/configuration_test.dart b/test/configuration_test.dart new file mode 100644 index 0000000..684f390 --- /dev/null +++ b/test/configuration_test.dart @@ -0,0 +1,225 @@ +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:sentry_dart_plugin/src/configuration.dart'; +import 'package:sentry_dart_plugin/src/configuration_values.dart'; +import 'package:sentry_dart_plugin/src/utils/injector.dart'; +import 'package:test/test.dart'; + +import 'utils/config_file_type.dart'; +import 'utils/config_formatter.dart'; +import 'utils/config_writer.dart'; + +void main() { + group('loadConfiguration', () { + late Fixture fixture; + + setUp(() { + final fs = MemoryFileSystem.test(); + fs.currentDirectory = fs.directory('/subdir')..createSync(); + injector.registerSingleton(() => fs, override: true); + fixture = Fixture(fs); + }); + + test("takes values from platform env config", () { + final platformEnvConfig = ConfigurationValues( + release: 'release-platformEnv-config', + dist: 'dist-platformEnv-config', + ); + final argsConfig = ConfigurationValues( + release: 'release-args-config', + dist: 'dist-args-config', + ); + final fileConfig = ConfigurationValues( + release: 'release-file-config', + dist: 'dist-file-config', + ); + + final sut = fixture.getSut( + platformEnvConfig: platformEnvConfig, + argsConfig: argsConfig, + fileConfig: fileConfig, + ); + + expect(sut.release, 'release-platformEnv-config'); + expect(sut.dist, 'dist-platformEnv-config'); + }); + + // env config + + test("takes values from args config", () { + final platformEnvConfig = ConfigurationValues(); + final argsConfig = ConfigurationValues( + version: 'version-args-config', + name: 'name-args-config', + uploadDebugSymbols: true, + uploadSourceMaps: true, + uploadSources: true, + project: 'project-args-config', + org: 'org-args-config', + authToken: 'auth_token-args-config', + url: 'url-args-config', + waitForProcessing: true, + logLevel: 'warning', + release: 'release-args-config', + dist: 'dist-args-config', + webBuildPath: 'web_build_path-args-config', + commits: 'commits-args-config', + ignoreMissing: true, + ); + final fileConfig = ConfigurationValues( + version: 'version-file-config', + name: 'name-file-config', + uploadDebugSymbols: false, + uploadSourceMaps: false, + uploadSources: false, + project: 'project-file-config', + org: 'org-file-config', + authToken: 'auth_token-file-config', + url: 'url-file-config', + waitForProcessing: false, + logLevel: 'debug', + release: 'release-file-config', + dist: 'dist-file-config', + webBuildPath: 'web_build_path-file-config', + commits: 'commits-file-config', + ignoreMissing: false, + ); + + final sut = fixture.getSut( + platformEnvConfig: platformEnvConfig, + argsConfig: argsConfig, + fileConfig: fileConfig, + ); + + expect(sut.name, 'name-args-config'); + expect(sut.version, 'version-args-config'); + expect(sut.uploadDebugSymbols, true); + expect(sut.uploadSourceMaps, true); + expect(sut.uploadSources, true); + expect(sut.project, 'project-args-config'); + expect(sut.org, 'org-args-config'); + expect(sut.authToken, 'auth_token-args-config'); + expect(sut.url, 'url-args-config'); + expect(sut.waitForProcessing, true); + expect(sut.logLevel, 'warning'); + expect(sut.release, 'release-args-config'); + expect(sut.dist, 'dist-args-config'); + expect( + sut.webBuildFilesFolder, + fixture.fs.path.join( + sut.buildFilesFolder, + 'web_build_path-args-config', + ), + ); + expect(sut.commits, 'commits-args-config'); + expect(sut.ignoreMissing, true); + }); + + test("takes values from file config", () { + final platformEnvConfig = ConfigurationValues(); + final argsConfig = ConfigurationValues(); + final fileConfig = ConfigurationValues( + version: 'version-file-config', + name: 'name-file-config', + uploadDebugSymbols: false, + uploadSourceMaps: true, + uploadSources: true, + project: 'project-file-config', + org: 'org-file-config', + authToken: 'auth_token-file-config', + url: 'url-file-config', + waitForProcessing: true, + logLevel: 'debug', + release: 'release-file-config', + dist: 'dist-file-config', + webBuildPath: 'web_build_path-file-config', + commits: 'commits-file-config', + ignoreMissing: true, + ); + + final sut = fixture.getSut( + argsConfig: argsConfig, + fileConfig: fileConfig, + platformEnvConfig: platformEnvConfig, + ); + + expect(sut.name, 'name-file-config'); + expect(sut.version, 'version-file-config'); + expect(sut.uploadDebugSymbols, false); + expect(sut.uploadSourceMaps, true); + expect(sut.uploadSources, true); + expect(sut.project, 'project-file-config'); + expect(sut.org, 'org-file-config'); + expect(sut.authToken, 'auth_token-file-config'); + expect(sut.url, 'url-file-config'); + expect(sut.waitForProcessing, true); + expect(sut.logLevel, 'debug'); + expect(sut.release, 'release-file-config'); + expect(sut.dist, 'dist-file-config'); + expect( + sut.webBuildFilesFolder, + fixture.fs.path + .join(sut.buildFilesFolder, 'web_build_path-file-config'), + ); + expect(sut.commits, 'commits-file-config'); + expect(sut.ignoreMissing, true); + }); + + test("falls back to default values", () { + final envConfig = ConfigurationValues(); + final fileConfig = ConfigurationValues(); + final platformEnvConfig = ConfigurationValues(); + + final sut = fixture.getSut( + argsConfig: envConfig, + fileConfig: fileConfig, + platformEnvConfig: platformEnvConfig, + ); + + expect(sut.name, 'name-pubspec-config'); + expect(sut.version, 'version-pubspec-config'); + expect(sut.uploadDebugSymbols, true); + expect(sut.uploadSourceMaps, false); + expect(sut.uploadSources, false); + expect(sut.commits, 'auto'); + expect(sut.ignoreMissing, false); + expect( + sut.webBuildFilesFolder, + fixture.fs.path.join(sut.buildFilesFolder, 'build/web'), + ); + expect(sut.waitForProcessing, false); + }); + }); +} + +class Fixture { + Fixture(this.fs); + + FileSystem fs; + + Configuration getSut({ + required ConfigurationValues platformEnvConfig, + required ConfigurationValues argsConfig, + required ConfigurationValues fileConfig, + }) { + final pubspecConfig = ConfigFormatter.formatConfig( + '', + ConfigFileType.pubspecYaml, + null, + ); + final writer = ConfigWriter( + fs, + 'name-pubspec-config', + ); + writer.write( + 'version-pubspec-config', ConfigFileType.pubspecYaml, pubspecConfig); + + final configuration = Configuration(); + configuration.loadConfig( + platformEnvConfig: platformEnvConfig, + argsConfig: argsConfig, + fileConfig: fileConfig, + ); + return configuration; + } +} diff --git a/test/configureation_values_test.dart b/test/configureation_values_test.dart new file mode 100644 index 0000000..59e24a7 --- /dev/null +++ b/test/configureation_values_test.dart @@ -0,0 +1,141 @@ +import 'package:file/file.dart'; +import 'package:file/memory.dart'; + +import 'package:sentry_dart_plugin/src/configuration_values.dart'; +import 'package:sentry_dart_plugin/src/utils/config-reader/config_reader.dart'; +import 'package:sentry_dart_plugin/src/utils/injector.dart'; +import 'package:test/test.dart'; + +import 'utils/config_file_type.dart'; +import 'utils/config_formatter.dart'; +import 'utils/config_writer.dart'; + +void main() { + group('ctor', () { + test("fromArguments", () { + final arguments = [ + "--sentry-define=version=fixture-version", + "--sentry-define=name=fixture-name", + "--sentry-define=upload_debug_symbols=true", + "--sentry-define=upload_source_maps=true", + "--sentry-define=upload_sources=true", + "--sentry-define=project=fixture-project", + "--sentry-define=org=fixture-org", + "--sentry-define=auth_token=fixture-auth_token", + "--sentry-define=url=fixture-url", + "--sentry-define=wait_for_processing=true", + "--sentry-define=log_level=fixture-log_level", + "--sentry-define=release=fixture-release", + "--sentry-define=dist=fixture-dist", + "--sentry-define=web_build_path=fixture-web_build_path", + "--sentry-define=commits=fixture-commits", + "--sentry-define=ignore_missing=true", + "--sentry-define=bin_dir=fixture-bin_dir", + ]; + final sut = ConfigurationValues.fromArguments(arguments); + expect(sut.name, 'fixture-name'); + expect(sut.version, 'fixture-version'); + expect(sut.uploadDebugSymbols, true); + expect(sut.uploadSourceMaps, true); + expect(sut.uploadSources, true); + expect(sut.project, 'fixture-project'); + expect(sut.org, 'fixture-org'); + expect(sut.authToken, 'fixture-auth_token'); + expect(sut.url, 'fixture-url'); + expect(sut.waitForProcessing, true); + expect(sut.logLevel, 'fixture-log_level'); + expect(sut.release, 'fixture-release'); + expect(sut.dist, 'fixture-dist'); + expect(sut.webBuildPath, 'fixture-web_build_path'); + expect(sut.commits, 'fixture-commits'); + expect(sut.ignoreMissing, true); + expect(sut.binDir, 'fixture-bin_dir'); + }); + + test("fromArguments supports deprecated fields", () { + final arguments = [ + "--sentry-define=upload_native_symbols=true", + "--sentry-define=include_native_sources=true", + ]; + final sut = ConfigurationValues.fromArguments(arguments); + expect(sut.uploadDebugSymbols, true); + expect(sut.uploadSources, true); + }); + + test("fromArguments correctly reads values containing '=' delimiter", () { + final arguments = [ + "--sentry-define=version=fixture=version", + "--sentry-define=name=fixture=name", + ]; + final sut = ConfigurationValues.fromArguments(arguments); + expect(sut.version, 'fixture=version'); + expect(sut.name, 'fixture=name'); + }); + + test('from config reader', () { + final config = ''' + name: fixture-name + version: fixture-version + upload_debug_symbols: true + upload_source_maps: true + upload_sources: true + url: fixture-url + wait_for_processing: true + log_level: fixture-log_level + release: fixture-release + dist: fixture-dist + web_build_path: fixture-web_build_path + commits: fixture-commits + ignore_missing: true + bin_dir: fixture-bin_dir + '''; + + FileSystem fs = MemoryFileSystem.test(); + fs.currentDirectory = fs.directory('/subdir')..createSync(); + injector.registerSingleton(() => fs, override: true); + + final pubspecConfig = ConfigFormatter.formatConfig( + config, + ConfigFileType.pubspecYaml, + null, + ); + final writer = ConfigWriter( + fs, + 'fixture-name', + ); + writer.write( + 'fixture-version', ConfigFileType.pubspecYaml, pubspecConfig); + + final reader = ConfigReader(); + final sut = ConfigurationValues.fromReader(reader); + expect(sut.name, 'fixture-name'); + expect(sut.version, 'fixture-version'); + expect(sut.uploadDebugSymbols, true); + expect(sut.uploadSourceMaps, true); + expect(sut.uploadSources, true); + expect(sut.project, 'p'); + expect(sut.org, 'o'); + expect(sut.authToken, 't'); + expect(sut.url, 'fixture-url'); + expect(sut.waitForProcessing, true); + expect(sut.logLevel, 'fixture-log_level'); + expect(sut.release, 'fixture-release'); + expect(sut.dist, 'fixture-dist'); + expect(sut.webBuildPath, 'fixture-web_build_path'); + expect(sut.commits, 'fixture-commits'); + expect(sut.ignoreMissing, true); + expect(sut.binDir, 'fixture-bin_dir'); + }); + + test("fromPlatformEnvironment", () { + final arguments = { + 'SENTRY_RELEASE': 'fixture-release', + 'SENTRY_DIST': 'fixture-dist', + }; + + final sut = ConfigurationValues.fromPlatformEnvironment(arguments); + expect(sut.release, 'fixture-release'); + expect(sut.dist, 'fixture-dist'); + }); + }); +}