From 4b7459c9364567a36993d2d0df68c791b588705d Mon Sep 17 00:00:00 2001 From: Naveed Jooma Date: Thu, 21 Nov 2024 15:19:49 -0500 Subject: [PATCH] [hotfix] resource names (#302) --- .../ios/Flutter/AppFrameworkInfo.plist | 2 +- .../ios/Runner.xcodeproj/project.pbxproj | 8 +- .../xcshareddata/xcschemes/Runner.xcscheme | 2 +- .../ios/Runner/AppDelegate.swift | 2 +- lib/src/exceptions.dart | 29 +++++++ lib/src/resource/base.dart | 4 +- lib/src/resource/manager.dart | 40 ++++++--- pubspec.yaml | 2 +- test/unit_test/resource/base_test.dart | 30 +++++++ test/unit_test/resource/manager_test.dart | 85 +++++++++++++++++++ 10 files changed, 181 insertions(+), 23 deletions(-) create mode 100644 test/unit_test/resource/base_test.dart create mode 100644 test/unit_test/resource/manager_test.dart diff --git a/example/viam_example_app/ios/Flutter/AppFrameworkInfo.plist b/example/viam_example_app/ios/Flutter/AppFrameworkInfo.plist index 9625e105df3..7c569640062 100644 --- a/example/viam_example_app/ios/Flutter/AppFrameworkInfo.plist +++ b/example/viam_example_app/ios/Flutter/AppFrameworkInfo.plist @@ -21,6 +21,6 @@ CFBundleVersion 1.0 MinimumOSVersion - 11.0 + 12.0 diff --git a/example/viam_example_app/ios/Runner.xcodeproj/project.pbxproj b/example/viam_example_app/ios/Runner.xcodeproj/project.pbxproj index f14dc1888f8..ded87b019f0 100644 --- a/example/viam_example_app/ios/Runner.xcodeproj/project.pbxproj +++ b/example/viam_example_app/ios/Runner.xcodeproj/project.pbxproj @@ -216,7 +216,7 @@ isa = PBXProject; attributes = { BuildIndependentTargetsInParallel = YES; - LastUpgradeCheck = 1430; + LastUpgradeCheck = 1510; ORGANIZATIONNAME = ""; TargetAttributes = { 331C8080294A63A400263BE5 = { @@ -453,7 +453,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; MTL_ENABLE_DEBUG_INFO = NO; SDKROOT = iphoneos; SUPPORTED_PLATFORMS = iphoneos; @@ -580,7 +580,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; MTL_ENABLE_DEBUG_INFO = YES; ONLY_ACTIVE_ARCH = YES; SDKROOT = iphoneos; @@ -629,7 +629,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 11.0; + IPHONEOS_DEPLOYMENT_TARGET = 12.0; MTL_ENABLE_DEBUG_INFO = NO; SDKROOT = iphoneos; SUPPORTED_PLATFORMS = iphoneos; diff --git a/example/viam_example_app/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme b/example/viam_example_app/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme index 87131a09bea..8e3ca5dfe19 100644 --- a/example/viam_example_app/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme +++ b/example/viam_example_app/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme @@ -1,6 +1,6 @@ 'Address not on local network'; } + +class DuplicateResourceException implements Exception { + final ResourceName name; + + const DuplicateResourceException(this.name); + + @override + String toString() => 'Duplicate registration of resource in manager: $name'; +} + +class ResourceNotFoundException implements Exception { + final ResourceName name; + + const ResourceNotFoundException(this.name); + + @override + String toString() => 'Resource not found in manager: $name'; +} + +class MultipleRemoteResourcesSameNameException implements Exception { + final Iterable names; + + const MultipleRemoteResourcesSameNameException(this.names); + + @override + String toString() => 'Multiple remote resources found with the name ${names.first.localName}: $names'; +} diff --git a/lib/src/resource/base.dart b/lib/src/resource/base.dart index f85b13da002..db2345adc71 100644 --- a/lib/src/resource/base.dart +++ b/lib/src/resource/base.dart @@ -51,7 +51,9 @@ class Subtype { ..namespace = namespace ..type = resourceType ..subtype = resourceSubtype - ..name = name; + ..remotePath.addAll(name.split(':')..removeLast()) + ..name = name + ..localName = name.split(':').last; } @override diff --git a/lib/src/resource/manager.dart b/lib/src/resource/manager.dart index ec6a5ff3225..13b9840dfb2 100644 --- a/lib/src/resource/manager.dart +++ b/lib/src/resource/manager.dart @@ -1,3 +1,6 @@ +import 'package:protobuf/protobuf.dart'; + +import '../exceptions.dart'; import '../gen/common/v1/common.pb.dart'; import 'base.dart'; @@ -5,31 +8,40 @@ import 'base.dart'; class ResourceManager { /// The available resources Map resources = {}; - final Map> _shortToLongName = {}; + final Map> _resourceNamesWithoutRemotes = {}; /// Register a new [Resource] with the manager. void register(ResourceName name, Resource resource) { if (resources.containsKey(name)) { - throw Exception('Duplicate registration of resource in manager'); + throw DuplicateResourceException(name); } - final shortName = name.name.split(':').last; - final names = _shortToLongName[shortName] ?? []; + final rnWithoutRemote = name.deepCopy() + ..remotePath.clear() + ..name = name.localName; + final names = _resourceNamesWithoutRemotes[rnWithoutRemote] ?? []; names.add(name); - _shortToLongName[shortName] = names; + _resourceNamesWithoutRemotes[rnWithoutRemote] = names; resources[name] = resource; } /// Get a resource with the given [ResourceName] T getResource(ResourceName name) { - final resource = resources[name]; - if (resource == null) throw Exception('Resource not found in manager'); + Resource? resource; + if (resources.containsKey(name)) { + resource = resources[name]; + } else { + final resourceNames = _resourceNamesWithoutRemotes[name] ?? []; + // If multiple ResourceNames map to this name-without-remotes, + // that means there are multiple remote resources with this same short name. + // Without any means to disambiguate, we should not select any. + if (resourceNames.length > 1) { + throw MultipleRemoteResourcesSameNameException(resourceNames); + } + if (resourceNames.length == 1) { + resource = resources[resourceNames.first]; + } + } + if (resource == null) throw ResourceNotFoundException(name); return resource as T; } - - /// Get a resource by its name only - T getResourceByName(String name) { - final names = _shortToLongName[name] ?? []; - if (names.isEmpty) throw Exception('Resource not found in manager'); - return getResource(names.first); - } } diff --git a/pubspec.yaml b/pubspec.yaml index f65e1acc9de..dab64efa690 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -10,7 +10,7 @@ environment: dependencies: flutter: sdk: flutter - flutter_webrtc: ^0.11.0 + flutter_webrtc: ^0.12.1+hotfix.1 grpc: ^4.0.1 protobuf: ^3.0.0 image: ^4.0.16 diff --git a/test/unit_test/resource/base_test.dart b/test/unit_test/resource/base_test.dart new file mode 100644 index 00000000000..bf50f67903e --- /dev/null +++ b/test/unit_test/resource/base_test.dart @@ -0,0 +1,30 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:viam_sdk/src/components/sensor/sensor.dart'; + +void main() { + group('Subtype Tests', () { + test('getResourceName', () { + // Local + final localName = 'my-sensor'; + final localRN = Sensor.subtype.getResourceName(localName); + expect(localRN.namespace, Sensor.subtype.namespace); + expect(localRN.type, Sensor.subtype.resourceType); + expect(localRN.subtype, Sensor.subtype.resourceSubtype); + expect(localRN.remotePath, []); + expect(localRN.name, localName); + expect(localRN.localName, localName); + + // Remote + final remoteName = 'my-sensor'; + final remotePath = 'remote2:remote1'; + final fullRemoteName = '$remotePath:$remoteName'; + final remoteRN = Sensor.subtype.getResourceName(fullRemoteName); + expect(remoteRN.namespace, Sensor.subtype.namespace); + expect(remoteRN.type, Sensor.subtype.resourceType); + expect(remoteRN.subtype, Sensor.subtype.resourceSubtype); + expect(remoteRN.remotePath, remotePath.split(':')); + expect(remoteRN.name, fullRemoteName); + expect(remoteRN.localName, remoteName); + }); + }); +} diff --git a/test/unit_test/resource/manager_test.dart b/test/unit_test/resource/manager_test.dart new file mode 100644 index 00000000000..b676a1ac8cb --- /dev/null +++ b/test/unit_test/resource/manager_test.dart @@ -0,0 +1,85 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:viam_sdk/src/components/sensor/sensor.dart'; +import 'package:viam_sdk/src/exceptions.dart'; +import 'package:viam_sdk/src/resource/manager.dart'; + +import '../components/sensor_test.dart'; + +void main() { + group('ResourceManager Tests', () { + group('getResource', () { + test('Local', () { + final localName = 'local-sensor'; + final localRN = Sensor.getResourceName(localName); + final localResource = FakeSensor(localName); + final manager = ResourceManager(); + manager.register(localRN, localResource); + + expect(manager.getResource(Sensor.getResourceName(localName)), localResource); + }); + + test('Remote', () { + final remoteName = 'remote-sensor'; + final remotePath = 'remote2:remote1'; + final fullRemoteName = '$remotePath:$remoteName'; + final remoteRN = Sensor.subtype.getResourceName(fullRemoteName); + final remoteResource = FakeSensor(fullRemoteName); + final manager = ResourceManager(); + manager.register(remoteRN, remoteResource); + + // Works with full name + expect(manager.getResource(Sensor.getResourceName(fullRemoteName)), remoteResource); + + // Works with short name -- no collisions + expect(manager.getResource(Sensor.getResourceName(remoteName)), remoteResource); + }); + + test('Local and Remote - Same Names', () { + final manager = ResourceManager(); + + final localName = 'my-sensor'; + final localRN = Sensor.getResourceName(localName); + final localResource = FakeSensor(localName); + + final remoteName = 'my-sensor'; + final remotePath = 'remote2:remote1'; + final fullRemoteName = '$remotePath:$remoteName'; + final remoteRN = Sensor.subtype.getResourceName(fullRemoteName); + final remoteResource = FakeSensor(fullRemoteName); + + manager.register(localRN, localResource); + manager.register(remoteRN, remoteResource); + + // When using fully qualified names, it should return appropriately + expect(manager.getResource(Sensor.getResourceName(localName)), localResource); + expect(manager.getResource(Sensor.getResourceName(fullRemoteName)), remoteResource); + + // When using just `my-sensor`, it should always return the local + expect(manager.getResource(Sensor.getResourceName(localName)), localResource); + expect(manager.getResource(Sensor.getResourceName(remoteName)), localResource); + }); + + test('Multiple Remotes - Same Names', () { + final remoteName = 'my-sensor'; + + final remotePath1 = 'remote2:remote1'; + final fullRemoteName1 = '$remotePath1:$remoteName'; + final remoteRN1 = Sensor.subtype.getResourceName(fullRemoteName1); + final remoteResource1 = FakeSensor(fullRemoteName1); + + final remotePath2 = 'remote4:remote3'; + final fullRemoteName2 = '$remotePath2:$remoteName'; + final remoteRN2 = Sensor.subtype.getResourceName(fullRemoteName2); + final remoteResource2 = FakeSensor(fullRemoteName2); + + final manager = ResourceManager(); + manager.register(remoteRN1, remoteResource1); + manager.register(remoteRN2, remoteResource2); + + // Error when using short name only + expect(() => manager.getResource(Sensor.getResourceName(remoteName)), + throwsA(isA())); + }); + }); + }); +}