Merge pubspec.yaml and sentry.properties values (#295)

Author: denrase

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Enhancements
+
+- Merge `pubspec.yaml` and `sentry.properties` values ([#295](https://github.com/getsentry/sentry-dart-plugin/pull/295))
+
 ### Dependencies
 
 - Bump CLI from v2.39.1 to v2.41.1 ([#290](https://github.com/getsentry/sentry-dart-plugin/pull/290))

README.md

@@ -30,9 +30,10 @@ dart run sentry_dart_plugin
 ## Configuration (Optional)
 
 This tool comes with a default configuration. You can configure it to suit your needs.
-By default the plugin will look for the Sentry configuration in the `pubspec.yaml` file.
-If the configuration doesn't exist, the plugin will look for a `sentry.properties` file.
-If the `sentry.properties` file doesn't exist, the plugin will look for environment variables.
+By default the plugin will look for the Sentry configuration in the `pubspec.yaml` and `sentry.properties` file.
+If a Sentry value does not exist in `pubspec.yaml`, the plugin will fallback to `sentry.properties` file.
+If a value exists in both, the `pubspec.yaml` takes precedence over the `sentry.properties` values.
+Environment and argument variables will take precedence over the file based ones.
 
 ### pubspec.yaml
 

lib/src/utils/config-reader/config_reader.dart

@@ -1,4 +1,5 @@
 import 'package:properties/properties.dart';
+import 'package:sentry_dart_plugin/src/utils/config-reader/fallback_config_reader.dart';
 import 'package:yaml/yaml.dart';
 import 'package:file/file.dart';
 
@@ -13,31 +14,39 @@ abstract class ConfigReader {
   bool? getBool(String key, {String? deprecatedKey});
   bool contains(String key);
 
-  /// By default this ConfigReader factory will try to load pubspec.yaml first.
-  /// If the sentry config doesn't exist on pubspec.yaml it will use sentry.properties as fallback.
+  /// This factory will try to load both pubspec.yaml and sentry.properties.
+  /// If a sentry config key doesn't exist on pubspec.yaml it will use sentry.properties as fallback.
   factory ConfigReader() {
-    // Attempt to retrieve the config from pubspec.yaml first
+    YamlConfigReader? pubspecReader;
+
+    Log.info('Searching for pubspec.yaml or sentry.properties config...');
+
     final pubspec = getPubspec();
     final sentryConfig = pubspec['sentry'] as YamlMap?;
     if (sentryConfig != null) {
-      Log.info('retrieving config from pubspec.yaml');
-      return YamlConfigReader(sentryConfig);
-    } else {
-      Log.info('sentry config not found in pubspec.yaml');
+      Log.info('Found config from pubspec.yaml');
+      pubspecReader = YamlConfigReader(sentryConfig);
     }
 
-    // If sentry config is not found in pubspec.yaml, try loading from sentry.properties
+    PropertiesConfigReader? propertiesReader;
+
     final propertiesFile = injector.get<FileSystem>().file("sentry.properties");
     if (propertiesFile.existsSync()) {
-      Log.info('retrieving config from sentry.properties');
+      Log.info('Found config from sentry.properties');
       // Loads properties class via string as there are issues loading the file
       // from path if run in the test suite
       final properties =
           Properties.fromString(propertiesFile.readAsStringSync());
-      return PropertiesConfigReader(properties);
+      propertiesReader = PropertiesConfigReader(properties);
+    }
+
+    if (pubspecReader == null && propertiesReader == null) {
+      Log.info(
+          'No file config found. Reading values from arguments or environment.');
+      return NoOpConfigReader();
+    } else {
+      return FallbackConfigReader(pubspecReader, propertiesReader);
     }
-    Log.error('no config found, please use sentry.properties or pubspec.yaml.');
-    return NoOpConfigReader();
   }
 
   static dynamic getPubspec() {

lib/src/utils/config-reader/fallback_config_reader.dart

@@ -0,0 +1,27 @@
+import 'config_reader.dart';
+
+class FallbackConfigReader implements ConfigReader {
+  FallbackConfigReader(this._configReader, this._fallbackConfigReader);
+
+  final ConfigReader? _configReader;
+  final ConfigReader? _fallbackConfigReader;
+
+  @override
+  bool? getBool(String key, {String? deprecatedKey}) {
+    return _configReader?.getBool(key, deprecatedKey: deprecatedKey) ??
+        _fallbackConfigReader?.getBool(key, deprecatedKey: deprecatedKey);
+  }
+
+  @override
+  String? getString(String key, {String? deprecatedKey}) {
+    return _configReader?.getString(key, deprecatedKey: deprecatedKey) ??
+        _fallbackConfigReader?.getString(key, deprecatedKey: deprecatedKey);
+  }
+
+  @override
+  bool contains(String key) {
+    return _configReader?.contains(key) ??
+        _fallbackConfigReader?.contains(key) ??
+        false;
+  }
+}

test/configureation_values_test.dart renamed to test/configuration_values_test.dart

@@ -207,6 +207,65 @@ void main() {
       expect(sut.sentryCliVersion, '1.0.0');
     });
 
+    test('from config reader pubspec & properties', () {
+      final sentryPubspec = '''
+      version: pubspec-version
+      name: pubspec-name
+      upload_debug_symbols: true
+      upload_source_maps: true
+      ''';
+
+      final sentryProperties = '''
+      version=properties-version
+      url=properties-url
+      upload_debug_symbols=false
+      upload_sources=true
+      ''';
+
+      FileSystem fs = MemoryFileSystem.test();
+      fs.currentDirectory = fs.directory('/subdir')..createSync();
+      injector.registerSingleton<FileSystem>(() => fs, override: true);
+
+      final propertiesConfig = ConfigFormatter.formatConfig(
+        sentryProperties,
+        ConfigFileType.sentryProperties,
+        null,
+      );
+      final propertiesWriter = ConfigWriter(fs, 'fixture-name');
+      propertiesWriter.write(
+        'fixture-version',
+        ConfigFileType.sentryProperties,
+        propertiesConfig,
+      );
+
+      final pubspecConfig = ConfigFormatter.formatConfig(
+        sentryPubspec,
+        ConfigFileType.pubspecYaml,
+        null,
+      );
+      final pubspecWriter = ConfigWriter(fs, 'fixture-name');
+      pubspecWriter.write(
+        'fixture-version',
+        ConfigFileType.pubspecYaml,
+        pubspecConfig,
+      );
+
+      final reader = ConfigReader();
+      final sut = ConfigurationValues.fromReader(reader);
+
+      // string
+
+      expect(sut.version, 'pubspec-version'); // pubspec before properties
+      expect(sut.name, 'pubspec-name'); // pubspec only
+      expect(sut.url, 'properties-url'); // properties only
+
+      // bool
+
+      expect(sut.uploadDebugSymbols, true); // pubspec before properties
+      expect(sut.uploadSourceMaps, true); // pubspec only
+      expect(sut.uploadSources, true); // properties only
+    });
+
     test("fromPlatformEnvironment", () {
       final arguments = {
         'SENTRY_RELEASE': 'fixture-release',