Unable to add proto files from nested directories used in imports #677

@StefH commented on GitHub (Mar 13, 2025):

I think some changes can be done to WireMockProtoFileResolver:

Use GetInvalidPathChars instead of GetInvalidFileNameChars
only strip the comment characters and some spaces and keep the rest

This would allow:

"// \\path\\filename.proto"
or
"// /path/filename.proto"

@StefH commented on GitHub (Mar 13, 2025): I think some changes can be done to **WireMockProtoFileResolver**: - Use `GetInvalidPathChars` instead of `GetInvalidFileNameChars` - only strip the comment characters and some spaces and keep the rest This would allow: - `"// \\path\\filename.proto"` or - `"// /path/filename.proto"`

adam commented

@siewers commented on GitHub (Mar 13, 2025):

That would be great.
Would it be possible to provide the proto definitions with the file path expected in the imports?

If I have an import referencing protos/v1/something.proto, it would be great if I could create a dictionary with these files upfront, like this:

Dictionary<string, string> protoDefintions = 
[
    { "protos/v1/something.proto", "proto file contents" }
]

Or perhaps using an type:

var protoDefinitions = new ProtoDefintions
{
    new ProtoDefinition(filePath: "protos/v1/something.proto", fileContents: "proto file contents")
}

This could perhaps even be extended to something like this:

var protoDefinitions = ProtoDefinitions.FromDirectory(directory: pathToProtoFiles, relativeTo: Directory.GetCurrentDirectory());

That way I wouldn't need to manually "hack" the files to provide it with a filename inside the file contents itself.

@siewers commented on GitHub (Mar 13, 2025): That would be great. Would it be possible to provide the proto definitions with the file path expected in the imports? If I have an import referencing `protos/v1/something.proto`, it would be great if I could create a dictionary with these files upfront, like this: ```csharp Dictionary<string, string> protoDefintions = [ { "protos/v1/something.proto", "proto file contents" } ] ``` Or perhaps using an type: ``` var protoDefinitions = new ProtoDefintions { new ProtoDefinition(filePath: "protos/v1/something.proto", fileContents: "proto file contents") } ``` This could perhaps even be extended to something like this: ```csharp var protoDefinitions = ProtoDefinitions.FromDirectory(directory: pathToProtoFiles, relativeTo: Directory.GetCurrentDirectory()); ``` That way I wouldn't need to manually "hack" the files to provide it with a filename inside the file contents itself.

adam commented

@StefH commented on GitHub (Mar 13, 2025):

The last one is the most user friendly I guess. The ProtoDefinitionHelper.FromDirectory(...) method returns a dictionary which should be usable in WireMock. However, this needs a lot a changes.

So maybe adding a method ProtoDefinitionHelper.FromDirectory which modified the files by adding the first line comment, and then returning a IReadOnlyCollection<string> protoDefinitions with all the modified protos --> this only needs this new method.

@StefH commented on GitHub (Mar 13, 2025): The last one is the most user friendly I guess. The `ProtoDefinitionHelper.FromDirectory(...)` method returns a dictionary which should be usable in WireMock. However, this needs a lot a changes. So maybe adding a method `ProtoDefinitionHelper.FromDirectory` which modified the files by adding the first line comment, and then returning a `IReadOnlyCollection<string> protoDefinitions` with all the modified protos --> this only needs this new method.

adam commented

@siewers commented on GitHub (Mar 13, 2025):

That would be an acceptable compromise I'd say 👍
The comments themselves are harmless and as long as you are not inspecting the in-memory data yourself, there should be no confusion.

@siewers commented on GitHub (Mar 13, 2025): That would be an acceptable compromise I'd say 👍 The comments themselves are harmless and as long as you are not inspecting the in-memory data yourself, there should be no confusion.

adam commented

@siewers commented on GitHub (Mar 13, 2025):

On a possibly unrelated note, would it be possible to get access to the deserialized proto instance through the log entries? I'm using this to assert the received messages in WireMock, but I cannot find a way to get the deserialized instances.

Let me know if I should create a separate issue for this.

@siewers commented on GitHub (Mar 13, 2025): On a possibly unrelated note, would it be possible to get access to the deserialized proto instance through the log entries? I'm using this to assert the received messages in WireMock, but I cannot find a way to get the deserialized instances. Let me know if I should create a separate issue for this.

adam commented

@StefH commented on GitHub (Mar 13, 2025):

On a possibly unrelated note, would it be possible to get access to the deserialized proto instance through the log entries? I'm using this to assert the received messages in WireMock, but I cannot find a way to get the deserialized instances.

Let me know if I should create a separate issue for this.

By deserialized, do you mean the bytes or the json?

@StefH commented on GitHub (Mar 13, 2025): > On a possibly unrelated note, would it be possible to get access to the deserialized proto instance through the log entries? I'm using this to assert the received messages in WireMock, but I cannot find a way to get the deserialized instances. > > Let me know if I should create a separate issue for this. By deserialized, do you mean the bytes or the json?

adam commented

@StefH commented on GitHub (Mar 13, 2025):

That would be an acceptable compromise I'd say 👍 The comments themselves are harmless and as long as you are not inspecting the in-memory data yourself, there should be no confusion.

I think this PR should fix it, can you review?
https://github.com/WireMock-Net/WireMock.Net/pull/1263

Preview version is: 1.7.4-ci-19802 (https://github.com/WireMock-Net/WireMock.Net/wiki/MyGet-preview-versions)

@StefH commented on GitHub (Mar 13, 2025): > That would be an acceptable compromise I'd say 👍 The comments themselves are harmless and as long as you are not inspecting the in-memory data yourself, there should be no confusion. I think this PR should fix it, can you review? https://github.com/WireMock-Net/WireMock.Net/pull/1263 Preview version is: `1.7.4-ci-19802` (https://github.com/WireMock-Net/WireMock.Net/wiki/MyGet-preview-versions)

adam commented

@siewers commented on GitHub (Mar 13, 2025):

On a possibly unrelated note, would it be possible to get access to the deserialized proto instance through the log entries? I'm using this to assert the received messages in WireMock, but I cannot find a way to get the deserialized instances.

Let me know if I should create a separate issue for this.

By deserialized, do you mean the bytes or the json?

I would like to have access to the deserialized JSON from the gRPC type.
I guess it isn't realistic to provide access to the gRPC instance as the types are generated dynamically, but perhaps available as a dynamic object could be helpful?

@siewers commented on GitHub (Mar 13, 2025): > > On a possibly unrelated note, would it be possible to get access to the deserialized proto instance through the log entries? I'm using this to assert the received messages in WireMock, but I cannot find a way to get the deserialized instances. > > > > Let me know if I should create a separate issue for this. > > By deserialized, do you mean the bytes or the json? I would like to have access to the deserialized JSON from the gRPC type. I guess it isn't realistic to provide access to the gRPC instance as the types are generated dynamically, but perhaps available as a dynamic object could be helpful?

adam commented

@StefH commented on GitHub (Mar 13, 2025):

On a possibly unrelated note, would it be possible to get access to the deserialized proto instance through the log entries? I'm using this to assert the received messages in WireMock, but I cannot find a way to get the deserialized instances.
Let me know if I should create a separate issue for this.

By deserialized, do you mean the bytes or the json?

I would like to have access to the deserialized JSON from the gRPC type. I guess it isn't realistic to provide access to the gRPC instance as the types are generated dynamically, but perhaps available as a dynamic object could be helpful?

Should already work?

@StefH commented on GitHub (Mar 13, 2025): > > > On a possibly unrelated note, would it be possible to get access to the deserialized proto instance through the log entries? I'm using this to assert the received messages in WireMock, but I cannot find a way to get the deserialized instances. > > > Let me know if I should create a separate issue for this. > > > > > > By deserialized, do you mean the bytes or the json? > > I would like to have access to the deserialized JSON from the gRPC type. I guess it isn't realistic to provide access to the gRPC instance as the types are generated dynamically, but perhaps available as a dynamic object could be helpful? Should already work? ![Image](https://github.com/user-attachments/assets/5d05a706-d9ee-469d-a82d-72a1e4d8c2d8)

adam commented

@siewers commented on GitHub (Mar 13, 2025):

I mean the request message. When I make a gRPC request to WireMock, I don't see the JSON unless I set the transformer on the response.

@siewers commented on GitHub (Mar 13, 2025): I mean the request message. When I make a gRPC request to WireMock, I don't see the JSON unless I set the transformer on the response.

adam commented

@StefH commented on GitHub (Mar 14, 2025):

I mean the request message. When I make a gRPC request to WireMock, I don't see the JSON unless I set the transformer on the response.

OK, in that case, it;s better to create a new issue.

@StefH commented on GitHub (Mar 14, 2025): > I mean the request message. When I make a gRPC request to WireMock, I don't see the JSON unless I set the transformer on the response. OK, in that case, it;s better to create a new issue.

adam commented

@siewers commented on GitHub (Mar 14, 2025):

I must be doing something wrong. The new version works, but I'm confused about how to use it.

First thing, the imports in the proto files use a forward slash (/) and not a back slash (\), like this:

import "opentelemetry/proto/logs/v1/logs.proto";

This could be solved by normalizing the path separators in the WireMockProtoFileResolver.Exists method. I'm unsure if this path separator is the proto file convention, but on Windows, the implementation in the ProtoDefinitionHelper doesn't work.

It also only seems to work if I add the proto definitions at the request level, but then I need to specify the "primary" proto file as the first one.

With OpenTelmetry, there are multiple primary files, so I cannot access them with this new implementation.

This is what I'm currently doing:

internal sealed class OpenTelemetryMockServer : IDisposable
{
    private readonly WireMockServer _server;

    public OpenTelemetryMockServer(ITestOutputHelper? testOutputHelper = null)
    {
        var serverSettings = new WireMockServerSettings
        {
            UseHttp2 = true,
        };

        if (testOutputHelper is not null)
        {
            serverSettings.Logger = new TestOutputHelperWireMockLogger(testOutputHelper);
        }

        var openTelemetryProtoDefinitionsPath = Directory.GetCurrentDirectory();

        var protoDefinitionsMap = Directory.EnumerateFiles(openTelemetryProtoDefinitionsPath, "*.proto", SearchOption.AllDirectories)
                                           .ToDictionary(Path.GetFileNameWithoutExtension, ParseProtoFile);

        _server = WireMockServer.Start(serverSettings);

        var defaultResponse = Response.Create()
                                      .WithTransformer()
                                      .WithStatusCode(200);

        _server.Given(
                   Request.Create()
                          .WithPath("/opentelemetry.proto.collector.trace.v1.TraceService/Export")
                          .WithBodyAsProtoBuf([
                              protoDefinitionsMap["trace_service"],
                              protoDefinitionsMap["trace"],
                              protoDefinitionsMap["resource"],
                              protoDefinitionsMap["common"],
                          ], "OpenTelemetry.Proto.Collector.Trace.V1.ExportTraceServiceRequest")
               )
               .RespondWith(defaultResponse);

        _server.Given(
                   Request.Create()
                          .WithPath("/opentelemetry.proto.collector.metrics.v1.MetricsService/Export")
                          .WithBodyAsProtoBuf([
                                  protoDefinitionsMap["metrics_service"],
                                  protoDefinitionsMap["metrics"],
                                  protoDefinitionsMap["resource"],
                                  protoDefinitionsMap["common"],
                              ],
                              "OpenTelemetry.Proto.Collector.Metrics.V1.ExportMetricsServiceRequest")
               )
               .RespondWith(defaultResponse);

        _server.Given(
                   Request.Create()
                          .WithPath("/opentelemetry.proto.collector.logs.v1.LogsService/Export")
                          .WithBodyAsProtoBuf([
                                  protoDefinitionsMap["logs_service"],
                                  protoDefinitionsMap["logs"],
                                  protoDefinitionsMap["resource"],
                                  protoDefinitionsMap["common"],
                              ],
                              "OpenTelemetry.Proto.Collector.Logs.V1.ExportLogsServiceRequest")
               )
               .RespondWith(defaultResponse);
    }

    public string Endpoint => _server.Url!;

    public void Dispose()
    {
        _server.Dispose();
    }

    public IReadOnlyList<JObject> GetTelemetryItems()
    {
        return [.. _server.LogEntries.Select(entry => entry.RequestMessage.BodyAsJson).OfType<object>().Cast<JObject>()];
    }

    private string ParseProtoFile(string filePath)
    {
        var sb = new StringBuilder(File.ReadAllText(filePath));

        sb.Insert(0, Environment.NewLine);
        var relativeProtoFilePath = Path.GetRelativePath(Directory.GetCurrentDirectory(), filePath)
                                        .Replace("\\", "/");

        sb.Insert(0, $"// {relativeProtoFilePath}"); // Used by WireMock to identify the file

        return sb.ToString();
    }
}

@siewers commented on GitHub (Mar 14, 2025): I must be doing something wrong. The new version works, but I'm confused about how to use it. First thing, the imports in the proto files use a forward slash (`/`) and not a back slash (`\`), like this: ```proto import "opentelemetry/proto/logs/v1/logs.proto"; ``` This could be solved by normalizing the path separators in the `WireMockProtoFileResolver.Exists` method. I'm unsure if this path separator is the proto file convention, but on Windows, the implementation in the `ProtoDefinitionHelper` doesn't work. It also only seems to work if I add the proto definitions at the request level, but then I need to specify the "primary" proto file as the first one. With OpenTelmetry, there are multiple primary files, so I cannot access them with this new implementation. This is what I'm currently doing: ```csharp internal sealed class OpenTelemetryMockServer : IDisposable { private readonly WireMockServer _server; public OpenTelemetryMockServer(ITestOutputHelper? testOutputHelper = null) { var serverSettings = new WireMockServerSettings { UseHttp2 = true, }; if (testOutputHelper is not null) { serverSettings.Logger = new TestOutputHelperWireMockLogger(testOutputHelper); } var openTelemetryProtoDefinitionsPath = Directory.GetCurrentDirectory(); var protoDefinitionsMap = Directory.EnumerateFiles(openTelemetryProtoDefinitionsPath, "*.proto", SearchOption.AllDirectories) .ToDictionary(Path.GetFileNameWithoutExtension, ParseProtoFile); _server = WireMockServer.Start(serverSettings); var defaultResponse = Response.Create() .WithTransformer() .WithStatusCode(200); _server.Given( Request.Create() .WithPath("/opentelemetry.proto.collector.trace.v1.TraceService/Export") .WithBodyAsProtoBuf([ protoDefinitionsMap["trace_service"], protoDefinitionsMap["trace"], protoDefinitionsMap["resource"], protoDefinitionsMap["common"], ], "OpenTelemetry.Proto.Collector.Trace.V1.ExportTraceServiceRequest") ) .RespondWith(defaultResponse); _server.Given( Request.Create() .WithPath("/opentelemetry.proto.collector.metrics.v1.MetricsService/Export") .WithBodyAsProtoBuf([ protoDefinitionsMap["metrics_service"], protoDefinitionsMap["metrics"], protoDefinitionsMap["resource"], protoDefinitionsMap["common"], ], "OpenTelemetry.Proto.Collector.Metrics.V1.ExportMetricsServiceRequest") ) .RespondWith(defaultResponse); _server.Given( Request.Create() .WithPath("/opentelemetry.proto.collector.logs.v1.LogsService/Export") .WithBodyAsProtoBuf([ protoDefinitionsMap["logs_service"], protoDefinitionsMap["logs"], protoDefinitionsMap["resource"], protoDefinitionsMap["common"], ], "OpenTelemetry.Proto.Collector.Logs.V1.ExportLogsServiceRequest") ) .RespondWith(defaultResponse); } public string Endpoint => _server.Url!; public void Dispose() { _server.Dispose(); } public IReadOnlyList<JObject> GetTelemetryItems() { return [.. _server.LogEntries.Select(entry => entry.RequestMessage.BodyAsJson).OfType<object>().Cast<JObject>()]; } private string ParseProtoFile(string filePath) { var sb = new StringBuilder(File.ReadAllText(filePath)); sb.Insert(0, Environment.NewLine); var relativeProtoFilePath = Path.GetRelativePath(Directory.GetCurrentDirectory(), filePath) .Replace("\\", "/"); sb.Insert(0, $"// {relativeProtoFilePath}"); // Used by WireMock to identify the file return sb.ToString(); } } ```

adam commented

@StefH commented on GitHub (Mar 14, 2025):

Thanks for testing.

imports in the proto files indeed should use a forward slash (/), this is fixed, please test the latest preview.
Currently, the first file is skipped because it's assumed to be the main proto file.
But as you noticed, this is not correct.
A solution could be to add an extra argument to FromDirectory(string directory) to indicate which proto file is the main proto, and make sure to add this as first (and do not add a comment)

What do you think?

@StefH commented on GitHub (Mar 14, 2025): Thanks for testing. 1. imports in the proto files indeed should use a forward slash (`/`), this is fixed, please test the latest preview. 2. Currently, the first file is skipped because it's assumed to be the main proto file. But as you noticed, this is not correct. A solution could be to add an extra argument to `FromDirectory(string directory)` to indicate which proto file is the main proto, and make sure to add this as first (and do not add a comment) What do you think?

adam commented

@siewers commented on GitHub (Mar 14, 2025):

I don't think that would work as I load all proto files once, but later, I need to identify the main ones depending on the request.
In my example above, you can see three main proto files, one for each request type. Each of these proto files has dependencies that I need to include as well.

Ideally, I'd like not to have to worry about it at all.
If WireMock can identify the C# type based on the provided message type, why should I have to provide the files for each request?

I'd love to be able to define the list of all proto files at the server level and only specify the type at the request level. That way, there should be a correct mapping between all files and imports.

However, I don't quite understand all the different ways of mapping available, so perhaps there's something I'm missing :)

@siewers commented on GitHub (Mar 14, 2025): I don't think that would work as I load all proto files once, but later, I need to identify the main ones depending on the request. In my example above, you can see three main proto files, one for each request type. Each of these proto files has dependencies that I need to include as well. Ideally, I'd like not to have to worry about it at all. If WireMock can identify the C# type based on the provided message type, why should I have to provide the files for each request? I'd love to be able to define the list of all proto files at the server level and only specify the type at the request level. That way, there should be a correct mapping between all files and imports. However, I don't quite understand all the different ways of mapping available, so perhaps there's something I'm missing :)

adam commented

@StefH commented on GitHub (Mar 14, 2025):

1️⃣

I've changed the code, so that now you can call this once (same as you've done in your example):

var protoDefinitionData = ProtoDefinitionHelper.FromDirectory(directory);

And then provide the correct "List" to each mapping. Where the argument to the ToList is the first (main) proto filename.

var trace_serviceList = protoDefinitionData.ToList("trace_service");

var metrics_serviceList = protoDefinitionData.ToList("metrics_service");

var logs_serviceList = protoDefinitionData.ToList("logs_service");

2️⃣

Ideally, I'd like not to have to worry about it at all.
If WireMock can identify the C# type based on the provided message type, why should I have to provide the files for each request?

I'd love to be able to define the list of all proto files at the server level and only specify the type at the request level. That way, there should be a correct mapping between all files and imports.

This is already possible using an unique id, and maybe this fits your requirement?

See this example code:

server
  .AddProtoDefinition("trace_service-id", trace_serviceList)
  .AddProtoDefinition("metrics_service-id", metrics_serviceList)
  .AddProtoDefinition("logs_service-id", logs_serviceList)

  // First mapping using that trace_service
  _server.Given(
    Request.Create()
      .WithPath("/opentelemetry.proto.collector.trace.v1.TraceService/Export")
      .WithBodyAsProtoBuf("trace_service-id", "OpenTelemetry.Proto.Collector.Trace.V1.ExportTraceServiceRequest")
  )
  .RespondWith(defaultResponse);

Could this work?

@StefH commented on GitHub (Mar 14, 2025): ## :one: I've changed the code, so that now you can call this once (same as you've done in your example): ``` c# var protoDefinitionData = ProtoDefinitionHelper.FromDirectory(directory); ``` And then provide the correct "List" to each mapping. Where the argument to the `ToList` is the first (main) proto filename. ``` c# var trace_serviceList = protoDefinitionData.ToList("trace_service"); var metrics_serviceList = protoDefinitionData.ToList("metrics_service"); var logs_serviceList = protoDefinitionData.ToList("logs_service"); ``` ## :two: > Ideally, I'd like not to have to worry about it at all. If WireMock can identify the C# type based on the provided message type, why should I have to provide the files for each request? > I'd love to be able to define the list of all proto files at the server level and only specify the type at the request level. That way, there should be a correct mapping between all files and imports. This is already possible using an unique id, and maybe this fits your requirement? See this example code: ``` c# server .AddProtoDefinition("trace_service-id", trace_serviceList) .AddProtoDefinition("metrics_service-id", metrics_serviceList) .AddProtoDefinition("logs_service-id", logs_serviceList) // First mapping using that trace_service _server.Given( Request.Create() .WithPath("/opentelemetry.proto.collector.trace.v1.TraceService/Export") .WithBodyAsProtoBuf("trace_service-id", "OpenTelemetry.Proto.Collector.Trace.V1.ExportTraceServiceRequest") ) .RespondWith(defaultResponse); ``` Could this work?

adam commented

@siewers commented on GitHub (Mar 15, 2025):

Yeah, I think so, but I haven't tested it yet.

However, I still find it confusing that I need to create three separate lists when all the proto files are already at the server level.
Again, I'm not sure I understand the idea behind it all (the documentation is rather scarce), but to me it seems to be a bit over complicated.
I didn't know you could or should add multiple different lists at the server level.

What I'd imagine would be a nice way of configuring it would be this:

Add all proto files to the server
Configure each request path and proto message type
Configure deserialization per request

From what I can understand, the proto files are already compiled internally to C# types, so if they are there already, wouldn't that be sufficient to look up the types?

@siewers commented on GitHub (Mar 15, 2025): Yeah, I think so, but I haven't tested it yet. However, I still find it confusing that I need to create three separate lists when all the proto files are already at the server level. Again, I'm not sure I understand the idea behind it all (the documentation is rather scarce), but to me it seems to be a bit over complicated. I didn't know you could or should add multiple different lists at the server level. What I'd imagine would be a nice way of configuring it would be this: 1. Add all proto files to the server 2. Configure each request path and proto message type 3. Configure deserialization per request From what I can understand, the proto files are already compiled internally to C# types, so if they are there already, wouldn't that be sufficient to look up the types?

adam commented

@siewers commented on GitHub (Mar 15, 2025):

So what I think would be the least confusing setup (for me in this particular case), would be something like this:

var protoDefinitions = ProtoDefinitions.FromDirectory(protoDirectory);
server.AddProtoDefinitions(protoDefinitions)
_server.Given(
    Request.Create()
        .WithPath("/opentelemetry.proto.collector.trace.v1.TraceService/Export")
        .WithBodyAsProtoBufType("OpenTelemetry.Proto.Collector.Trace.V1.ExportTraceServiceRequest")
  )
  .RespondWith(defaultResponse);

@siewers commented on GitHub (Mar 15, 2025): So what I think would be the least confusing setup (for me in this particular case), would be something like this: ```csharp var protoDefinitions = ProtoDefinitions.FromDirectory(protoDirectory); server.AddProtoDefinitions(protoDefinitions) _server.Given( Request.Create() .WithPath("/opentelemetry.proto.collector.trace.v1.TraceService/Export") .WithBodyAsProtoBufType("OpenTelemetry.Proto.Collector.Trace.V1.ExportTraceServiceRequest") ) .RespondWith(defaultResponse); ```

adam commented