From 6a8f833d8643855f5a21cd6f8efbd70eabb07b1a Mon Sep 17 00:00:00 2001
From: Jon Skeet <jonskeet@google.com>
Date: Wed, 6 Sep 2023 12:25:59 +0100
Subject: [PATCH] feat: Validate resource name uniqueness to provide readable
 errors

Fixes #672.
---
 Google.Api.Generator.Tests/ProtoTest.cs       |  8 +++++
 .../DuplicateResourceDefinitions.proto        | 30 +++++++++++++++++++
 .../ProtoUtils/ProtoCatalog.cs                | 20 +++++++++++--
 3 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100644 Google.Api.Generator.Tests/ProtoTests/DuplicateResourceDefinitions/DuplicateResourceDefinitions.proto

diff --git a/Google.Api.Generator.Tests/ProtoTest.cs b/Google.Api.Generator.Tests/ProtoTest.cs
index 64d4922a..188602a1 100644
--- a/Google.Api.Generator.Tests/ProtoTest.cs
+++ b/Google.Api.Generator.Tests/ProtoTest.cs
@@ -324,5 +324,13 @@ public void PublishingSettings() => ProtoTestSingle(
 
         [Fact]
         public void BuildLro() => BuildTest("Lro");
+
+        [Fact]
+        public void DuplicateResourceDefinitions()
+        {
+            var exception = ProtoTestSingleFailure<InvalidOperationException>("DuplicateResourceDefinitions");
+            Assert.Contains("Multiple definitions", exception.Message);
+            Assert.Contains("TestResource", exception.Message);
+        }
     }
 }
diff --git a/Google.Api.Generator.Tests/ProtoTests/DuplicateResourceDefinitions/DuplicateResourceDefinitions.proto b/Google.Api.Generator.Tests/ProtoTests/DuplicateResourceDefinitions/DuplicateResourceDefinitions.proto
new file mode 100644
index 00000000..4e1e4305
--- /dev/null
+++ b/Google.Api.Generator.Tests/ProtoTests/DuplicateResourceDefinitions/DuplicateResourceDefinitions.proto
@@ -0,0 +1,30 @@
+syntax = "proto3";
+
+package testing.duplicateresourcedefinitions;
+
+import "google/api/client.proto";
+import "google/api/resource.proto";
+
+// Deliberately define the same resource twice, to provoke an error.
+option (google.api.resource_definition) = {
+  type: "test.googleapis.com/TestResource"
+  pattern: "projects/{project}"
+};
+
+option (google.api.resource_definition) = {
+  type: "test.googleapis.com/TestResource"
+  pattern: "organizations/{organization}"
+};
+
+service Basic {
+  option (google.api.default_host) = "basic.example.com";
+  option (google.api.oauth_scopes) = "scope1,scope2";
+
+  rpc AMethod(Request) returns(Response);
+}
+
+message Request {
+}
+
+message Response {
+}
diff --git a/Google.Api.Generator/ProtoUtils/ProtoCatalog.cs b/Google.Api.Generator/ProtoUtils/ProtoCatalog.cs
index d2e47136..1e9de8e6 100644
--- a/Google.Api.Generator/ProtoUtils/ProtoCatalog.cs
+++ b/Google.Api.Generator/ProtoUtils/ProtoCatalog.cs
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 using Google.Protobuf.Reflection;
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
@@ -42,8 +43,10 @@ public ProtoCatalog(
             _services = allDescriptors.SelectMany(desc => desc.Services).ToDictionary(x => x.FullName);
             _resourcesByFileName = ResourceDetails.LoadResourceDefinitionsByFileName(allDescriptors, commonResourcesConfigs, librarySettings).GroupBy(x => x.FileName)
                 .ToImmutableDictionary(x => x.Key, x => (IReadOnlyList<ResourceDetails.Definition>)x.ToImmutableList());
-            var resourcesByUrt = _resourcesByFileName.Values.SelectMany(x => x).ToDictionary(x => x.UnifiedResourceTypeName);
-            var resourcesByPatternComparison = _resourcesByFileName.Values.SelectMany(x => x)
+            var allResources = _resourcesByFileName.Values.SelectMany(x => x);
+            ValidateUniqueResourceTypeNames();
+            var resourcesByUrt = allResources.ToDictionary(x => x.UnifiedResourceTypeName);
+            var resourcesByPatternComparison = allResources
                 .SelectMany(def => def.Patterns.Where(x => !x.IsWildcard).Select(x => (x.Template.ParentComparisonString, def)))
                 .GroupBy(x => x.ParentComparisonString, x => x.def)
                 .ToImmutableDictionary(x => x.Key, x => (IReadOnlyList<ResourceDetails.Definition>)x.ToImmutableList());
@@ -57,6 +60,19 @@ public ProtoCatalog(
                 .ToDictionary(x => x.field.FullName, x => x.res);
 
             IEnumerable<MessageDescriptor> MsgPlusNested(MessageDescriptor msgDesc) => msgDesc.NestedTypes.SelectMany(MsgPlusNested).Append(msgDesc);
+
+            void ValidateUniqueResourceTypeNames()
+            {
+                var multiDefinitions = allResources
+                    .GroupBy(x => x.UnifiedResourceTypeName)
+                    .Where(g => g.Count() > 1)
+                    .Select(g => g.Key)
+                    .ToList();
+                if (multiDefinitions.Any())
+                {
+                    throw new InvalidOperationException($"Multiple definitions found for the following resources: {string.Join(",", multiDefinitions)}");
+                }
+            }
         }
 
         private readonly string _defaultPackage;