[hotfix] resource names (#302)

Author: njooma

example/viam_example_app/ios/Flutter/AppFrameworkInfo.plist

@@ -21,6 +21,6 @@
   <key>CFBundleVersion</key>
   <string>1.0</string>
   <key>MinimumOSVersion</key>
-  <string>11.0</string>
+  <string>12.0</string>
 </dict>
 </plist>

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;

example/viam_example_app/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
-   LastUpgradeVersion = "1430"
+   LastUpgradeVersion = "1510"
    version = "1.3">
    <BuildAction
       parallelizeBuildables = "YES"

example/viam_example_app/ios/Runner/AppDelegate.swift

@@ -1,7 +1,7 @@
 import UIKit
 import Flutter
 
-@UIApplicationMain
+@main
 @objc class AppDelegate: FlutterAppDelegate {
   override func application(
     _ application: UIApplication,

lib/src/exceptions.dart

@@ -1,3 +1,5 @@
+import '../protos/common/common.dart';
+
 /// Error thrown with connection is lost
 class ConnectionLostException implements Exception {
   final String? message;
@@ -17,3 +19,30 @@ class NotLocalAddressException implements Exception {
   @override
   String toString() => '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<ResourceName> names;
+
+  const MultipleRemoteResourcesSameNameException(this.names);
+
+  @override
+  String toString() => 'Multiple remote resources found with the name ${names.first.localName}: $names';
+}

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

lib/src/resource/manager.dart

@@ -1,35 +1,47 @@
+import 'package:protobuf/protobuf.dart';
+
+import '../exceptions.dart';
 import '../gen/common/v1/common.pb.dart';
 import 'base.dart';
 
 /// [ResourceManager] manages the state of all currently available resources of a robot
 class ResourceManager {
   /// The available resources
   Map<ResourceName, Resource> resources = {};
-  final Map<String, List<ResourceName>> _shortToLongName = {};
+  final Map<ResourceName, List<ResourceName>> _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<T>(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<T>(String name) {
-    final names = _shortToLongName[name] ?? [];
-    if (names.isEmpty) throw Exception('Resource not found in manager');
-    return getResource(names.first);
-  }
 }

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

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);
+    });
+  });
+}

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>(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>(Sensor.getResourceName(fullRemoteName)), remoteResource);
+
+        // Works with short name -- no collisions
+        expect(manager.getResource<Sensor>(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>(Sensor.getResourceName(localName)), localResource);
+        expect(manager.getResource<Sensor>(Sensor.getResourceName(fullRemoteName)), remoteResource);
+
+        // When using just `my-sensor`, it should always return the local
+        expect(manager.getResource<Sensor>(Sensor.getResourceName(localName)), localResource);
+        expect(manager.getResource<Sensor>(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>(Sensor.getResourceName(remoteName)),
+            throwsA(isA<MultipleRemoteResourcesSameNameException>()));
+      });
+    });
+  });
+}