Skip to content

Commit

Permalink
Move logic to de-duplicate extensions into a helper file for shared u…
Browse files Browse the repository at this point in the history
…se (#7959)
  • Loading branch information
kenzieschmoll authored Jun 21, 2024
1 parent 712504f commit adf025b
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 168 deletions.
1 change: 1 addition & 0 deletions packages/devtools_app/lib/devtools_app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

export 'src/app.dart';
export 'src/extensions/extension_service.dart';
export 'src/extensions/extension_service_helpers.dart';
export 'src/framework/app_bar.dart';
export 'src/framework/home_screen.dart';
export 'src/framework/notifications_view.dart';
Expand Down
85 changes: 13 additions & 72 deletions packages/devtools_app/lib/src/extensions/extension_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@

import 'package:devtools_app_shared/utils.dart';
import 'package:devtools_shared/devtools_extensions.dart';
import 'package:devtools_shared/devtools_shared.dart';
import 'package:flutter/foundation.dart';
import 'package:logging/logging.dart';

import '../shared/globals.dart';
import '../shared/primitives/utils.dart';
import '../shared/server/server.dart' as server;
import 'extension_service_helpers.dart';

final _log = Logger('ExtensionService');

Expand Down Expand Up @@ -221,47 +221,20 @@ class ExtensionService extends DisposableController
'ignoring static extension ${ext.identifier} at '
'${ext.devtoolsOptionsUri} because it requires a connected app.',
);
setExtensionIgnored(ext);
setExtensionIgnored(ext, ignore: true);
}
}
}

/// De-duplicates static extensions from other static extensions by ignoring
/// all that are not the latest version when there are duplicates.
void _deduplicateStaticExtensions() {
final deduped = <String>{};
for (final staticExtension in staticExtensions) {
if (deduped.contains(staticExtension.name)) continue;
deduped.add(staticExtension.name);

// This includes [staticExtension] itself.
final matchingExtensions =
staticExtensions.where((e) => e.name == staticExtension.name);
if (matchingExtensions.length > 1) {
_log.fine(
'detected duplicate static extensions for ${staticExtension.name}',
);

// Ignore all matching extensions and then mark the [latest] as
// unignored after the loop is finished.
var latest = staticExtension;
for (final ext in matchingExtensions) {
setExtensionIgnored(ext);
latest = takeLatestExtension(latest, ext);
}
setExtensionIgnored(latest, ignore: false);

_log.fine(
'ignored ${matchingExtensions.length - 1} duplicate static '
'${pluralize('extension', matchingExtensions.length - 1)} in favor of '
'${latest.identifier} at ${latest.devtoolsOptionsUri}',
);
} else {
_log.fine(
'no duplicates found for static extension ${staticExtension.name}',
);
}
}
deduplicateExtensionsAndTakeLatest(
staticExtensions,
onSetIgnored: setExtensionIgnored,
logger: _log,
extensionType: 'static',
);
}

// De-duplicates unignored static extensions from runtime extensions by
Expand All @@ -282,7 +255,7 @@ class ExtensionService extends DisposableController
'at ${staticExtension.devtoolsOptionsUri} in favor of a matching '
'runtime extension.',
);
setExtensionIgnored(staticExtension);
setExtensionIgnored(staticExtension, ignore: true);
}
}
}
Expand Down Expand Up @@ -357,7 +330,10 @@ class ExtensionService extends DisposableController
/// An extension may be ignored if it is a duplicate or if it is an older
/// version of an existing extension, for example.
@visibleForTesting
void setExtensionIgnored(DevToolsExtensionConfig ext, {bool ignore = true}) {
void setExtensionIgnored(
DevToolsExtensionConfig ext, {
required bool ignore,
}) {
ignore
? _ignoredStaticExtensionsByHashCode.add(identityHashCode(ext))
: _ignoredStaticExtensionsByHashCode.remove(identityHashCode(ext));
Expand Down Expand Up @@ -395,38 +371,3 @@ class ExtensionService extends DisposableController
super.dispose();
}
}

/// Compares the versions of extension configurations [a] and [b] and returns
/// the extension configuration with the latest version, following semantic
/// versioning rules.
@visibleForTesting
DevToolsExtensionConfig takeLatestExtension(
DevToolsExtensionConfig a,
DevToolsExtensionConfig b,
) {
bool exceptionParsingA = false;
bool exceptionParsingB = false;
SemanticVersion? versionA;
SemanticVersion? versionB;
try {
versionA = SemanticVersion.parse(a.version);
} catch (_) {
exceptionParsingA = true;
}

try {
versionB = SemanticVersion.parse(b.version);
} catch (_) {
exceptionParsingB = true;
}

if (exceptionParsingA || exceptionParsingB) {
if (exceptionParsingA) {
return b;
}
return a;
}

final versionCompare = versionA!.compareTo(versionB!);
return versionCompare >= 0 ? a : b;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Copyright 2024 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:devtools_app_shared/utils.dart';
import 'package:devtools_shared/devtools_extensions.dart';
import 'package:devtools_shared/devtools_shared.dart';
import 'package:flutter/foundation.dart';
import 'package:logging/logging.dart';

/// De-duplicates extensions by ignoring all that are not the latest version
/// when there are duplicates.
void deduplicateExtensionsAndTakeLatest(
List<DevToolsExtensionConfig> extensions, {
required void Function(DevToolsExtensionConfig ext, {required bool ignore})
onSetIgnored,
Logger? logger,
String extensionType = '',
}) {
final deduped = <String>{};
for (final ext in extensions) {
if (deduped.contains(ext.name)) continue;
deduped.add(ext.name);

// This includes [ext] itself.
final matchingExtensions = extensions.where((e) => e.name == ext.name);
if (matchingExtensions.length > 1) {
logger?.fine(
'detected duplicate $extensionType extensions for ${ext.name}',
);

// Ignore all matching extensions and then mark the [latest] as
// unignored after the loop is finished.
var latest = ext;
for (final ext in matchingExtensions) {
onSetIgnored(ext, ignore: true);
latest = takeLatestExtension(latest, ext);
}
onSetIgnored(latest, ignore: false);

logger?.fine(
'ignored ${matchingExtensions.length - 1} duplicate $extensionType '
'${pluralize('extension', matchingExtensions.length - 1)} in favor of '
'${latest.identifier} at ${latest.devtoolsOptionsUri}',
);
} else {
logger?.fine(
'no duplicates found for $extensionType extension ${ext.name}',
);
}
}
}

/// Compares the versions of extension configurations [a] and [b] and returns
/// the extension configuration with the latest version, following semantic
/// versioning rules.
@visibleForTesting
DevToolsExtensionConfig takeLatestExtension(
DevToolsExtensionConfig a,
DevToolsExtensionConfig b,
) {
bool exceptionParsingA = false;
bool exceptionParsingB = false;
SemanticVersion? versionA;
SemanticVersion? versionB;
try {
versionA = SemanticVersion.parse(a.version);
} catch (_) {
exceptionParsingA = true;
}

try {
versionB = SemanticVersion.parse(b.version);
} catch (_) {
exceptionParsingB = true;
}

if (exceptionParsingA || exceptionParsingB) {
if (exceptionParsingA) {
return b;
}
return a;
}

final versionCompare = versionA!.compareTo(versionB!);
return versionCompare >= 0 ? a : b;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// Copyright 2024 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:devtools_app/devtools_app.dart';
import 'package:devtools_app/src/shared/development_helpers.dart';
import 'package:devtools_shared/devtools_extensions.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
group('takeLatestExtension', () {
test('returns newer extension', () {
expect(
takeLatestExtension(
StubDevToolsExtensions.barExtension,
StubDevToolsExtensions.newerBarExtension,
),
StubDevToolsExtensions.newerBarExtension,
);
});

test('handles parsing errors', () {
// Returns 'b' when 'a' has parsing errors.
var a = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'bar',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: 'this-will-not-parse',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/absolute/path/to/bar',
DevToolsExtensionConfig.devtoolsOptionsUriKey:
'file:///path/to/options/file',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});
var b = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'bar',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '2.1.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/absolute/path/to/bar',
DevToolsExtensionConfig.devtoolsOptionsUriKey:
'file:///path/to/options/file',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});
expect(takeLatestExtension(a, b), b);

// Returns 'a' when 'b' has parsing errors.
a = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'bar',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: '2.1.0',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/absolute/path/to/bar',
DevToolsExtensionConfig.devtoolsOptionsUriKey:
'file:///path/to/options/file',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});
b = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'bar',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: 'this-will-not-parse',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/path/to/bar',
DevToolsExtensionConfig.devtoolsOptionsUriKey: '/path/to/options/file',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});
expect(takeLatestExtension(a, b), a);

// Returns 'a' when both 'a' and 'b' have parsing errors.
a = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'bar',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: 'this-will-not-parse',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/absolute/path/to/bar',
DevToolsExtensionConfig.devtoolsOptionsUriKey:
'file:///path/to/options/file',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});
b = DevToolsExtensionConfig.parse({
DevToolsExtensionConfig.nameKey: 'bar',
DevToolsExtensionConfig.issueTrackerKey: 'www.google.com',
DevToolsExtensionConfig.versionKey: 'this-will-not-parse',
DevToolsExtensionConfig.materialIconCodePointKey: 0xe638,
DevToolsExtensionConfig.requiresConnectionKey: 'false',
DevToolsExtensionConfig.extensionAssetsPathKey: '/absolute/path/to/bar',
DevToolsExtensionConfig.devtoolsOptionsUriKey:
'file:///path/to/options/file',
DevToolsExtensionConfig.isPubliclyHostedKey: 'false',
DevToolsExtensionConfig.detectedFromStaticContextKey: 'true',
});
expect(takeLatestExtension(a, b), a);
});
});
}
Loading

0 comments on commit adf025b

Please sign in to comment.