Skip to content

Commit

Permalink
fix: Only generate files for mixins if that's *all* we're generating
Browse files Browse the repository at this point in the history
There are various reasons why it's hard to pass the "correct" set of protos into the generator; this is a workaround which should at least improve matters.
  • Loading branch information
jskeet committed Apr 21, 2023
1 parent f3d3f8b commit 63d9aa0
Showing 1 changed file with 38 additions and 7 deletions.
45 changes: 38 additions & 7 deletions Google.Api.Generator/CodeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ internal static class CodeGenerator
new Regex(@"^System\.?.*", RegexOptions.Compiled | RegexOptions.CultureInvariant),
};

private static readonly IReadOnlyList<string> AllowedAdditionalServices = new List<string>
private static readonly IReadOnlyList<ServiceDescriptor> AllowedAdditionalServices = new List<ServiceDescriptor>
{
IAMPolicy.Descriptor.FullName,
Locations.Descriptor.FullName,
Operations.Descriptor.FullName,
IAMPolicy.Descriptor,
Locations.Descriptor,
Operations.Descriptor,
}.AsReadOnly();

public static IEnumerable<ResultFile> Generate(FileDescriptorSet descriptorSet, string package, IClock clock,
Expand Down Expand Up @@ -121,6 +121,7 @@ public static IEnumerable<ResultFile> Generate(IReadOnlyList<FileDescriptorProto

// Collect all service details here to emit one `gapic_metadata.json` file for multi-package situations (e.g. Kms with Iam)
var allServiceDetails = new List<ServiceDetails>();
var resultFilesByProtoPackage = new Dictionary<string, List<ResultFile>>();
foreach (var singlePackageFileDescs in byPackage)
{
// Find the right library settings by matching the proto package we're publishing.
Expand All @@ -134,15 +135,22 @@ public static IEnumerable<ResultFile> Generate(IReadOnlyList<FileDescriptorProto
$"Found namespaces '{string.Join(", ", namespaces)}' in package '{singlePackageFileDescs.Key}'.");
}
var catalog = new ProtoCatalog(singlePackageFileDescs.Key, descriptors, singlePackageFileDescs, commonResourcesConfigs, librarySettings);
var files = new List<ResultFile>();
foreach (var resultFile in GeneratePackage(
namespaces[0], singlePackageFileDescs, catalog, clock,
grpcServiceConfig, serviceConfig, allServiceDetails,
transports, requestNumericEnumJsonEncoding, librarySettings))
{
yield return resultFile;
files.Add(resultFile);
}
if (files.Count > 0)
{
resultFilesByProtoPackage[singlePackageFileDescs.Key] = files;
}
}

var resultFiles = GetResultFilesToCreate(resultFilesByProtoPackage);

// We assume that the first service we've generated corresponds to the service config (if we have one),
// and is a service from the primary library we're generating. This is used for API validation and
// gapic_metadata.json generation. This means it doesn't matter (for gapic_metadata.json)
Expand All @@ -155,12 +163,12 @@ public static IEnumerable<ResultFile> Generate(IReadOnlyList<FileDescriptorProto
{
// Generate gapic_metadata.json, if there are any services.
var gapicMetadataJsonContent = MetadataGenerator.GenerateGapicMetadataJson(primaryLibraryServices);
yield return new ResultFile("gapic_metadata.json", gapicMetadataJsonContent);
resultFiles.Add(new ResultFile("gapic_metadata.json", gapicMetadataJsonContent));
}

var unhandledApis = (serviceConfig?.Apis.Select(api => api.Name) ?? Enumerable.Empty<string>())
.Except(primaryLibraryServices.Select(s => s.ServiceFullName))
.Except(AllowedAdditionalServices)
.Except(AllowedAdditionalServices.Select(s => s.FullName))
.ToList();

if (unhandledApis.Any())
Expand All @@ -169,6 +177,29 @@ public static IEnumerable<ResultFile> Generate(IReadOnlyList<FileDescriptorProto
}

ValidateTransports(transports, allServiceDetails);
return resultFiles;

// Returns the result files we should actually create, ignoring mix-ins unless we're actually
// generating the mix-in API.
// This is necessary because Bazel passes more files to generate than we really want.
// Note that this method may modify filesByProtoPackage - we're not using it after calling this method though.
static List<ResultFile> GetResultFilesToCreate(Dictionary<string, List<ResultFile>> filesByProtoPackage)
{
var mixinPackages = AllowedAdditionalServices.Select(svc => svc.File.Package).ToHashSet();

// Unless we're *only* generating mixins, remove all mixins.
if (!filesByProtoPackage.Keys.All(mixinPackages.Contains))
{
foreach (var pkg in mixinPackages)
{
filesByProtoPackage.Remove(pkg);
}
}

// We may still have multiple packages here, and that's probably not
// ideal, but it's not too bad - and it deals with the common situation.
return filesByProtoPackage.Values.SelectMany(list => list).ToList();
}
}

private static void ValidateTransports(ApiTransports transports, List<ServiceDetails> services)
Expand Down

0 comments on commit 63d9aa0

Please sign in to comment.