dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.45k stars 10.03k forks source link

GrpcHttpApi support for paths with wildcards #34524

Closed BrennanConroy closed 2 years ago

BrennanConroy commented 3 years ago

Issue moved from aspnet/AspLabs#350


From @ba32107 on Tuesday, March 30, 2021 1:06:11 PM

Hi,

Thanks for this library - it is extremely useful. As reported in https://github.com/grpc/grpc-dotnet/issues/167 by multiple people, paths with wildcards are not supported. Example 1 Example 2 Example 3

Please could this be added?

Thanks Balazs

BrennanConroy commented 3 years ago

Issue moved from aspnet/AspLabs#350


From @fl0wm0ti0n on Saturday, May 1, 2021 8:06:15 PM

io run also into this i think...

Schweregrad Code Beschreibung Projekt Datei Zeile Unterdrückungszustand Fehler File not found. GrpcService_Test C:\flowTFS\softwareentwicklung\c-sharp-projects\GrpcService-Test\google\api\annotations.proto 1

Schweregrad Code Beschreibung Projekt Datei Zeile Unterdrückungszustand Fehler Import "google/api/annotations.proto" was not found or had errors. GrpcService_Test C:\flowTFS\softwareentwicklung\c-sharp-projects\GrpcService-Test\Protos\greet.proto 5

Error cant find options:

service Greeter {
    // Sends a greeting
    rpc SayHello (HelloRequest) returns (HelloReply) {
        option (google.api.http) = {
            get: "/v1/greeter/{name}"
        };
    }
}

"annotations.proto" errors on finding "import "google/protobuf/descriptor.proto";"

// Copyright (c) 2015, Google Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

syntax = "proto3";

package google.api;

import "google/api/http.proto";
import "google/protobuf/descriptor.proto";

option go_package = "google.golang.org/genproto/googleapis/api/annotations;annotations";
option java_multiple_files = true;
option java_outer_classname = "AnnotationsProto";
option java_package = "com.google.api";
option objc_class_prefix = "GAPI";

extend google.protobuf.MethodOptions {
    // See `HttpRule`.
    HttpRule http = 72295728;
}
BrennanConroy commented 3 years ago

Issue moved from aspnet/AspLabs#350


From @JamesNK on Tuesday, July 20, 2021 2:15:48 AM

Hi

This feature could be difficult. The non-wildcard routes maps directly to the existing ASP.NET Core route syntax. However, wildcard routes will need to be rewritten inside the library before they can be used. That might be simple, or it could be quite difficult.

I don't have much time to work on GrpcHttpApi for the rest of this year, I have .NET 6 features to deliver, but if someone wants to attempt it then a PR is welcome.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

jasonmreding commented 2 years ago

I ran into this issue recently when using:

<PackageReference Include="Microsoft.AspNetCore.Grpc.JsonTranscoding" Version="7.0.0-preview.4.22251.1" />

and version 7.0.100-preview.4.22252.9 of the .Net SDK.

This was pretty surprising to me since the documentation for http.proto specifically shows examples of using "*" as wild card to match against any path segment.

The lack of this capability seems like it will also impact API design or require additional work in the client/server to account for it. For example, using the library.proto as an example, it organizes resources into a resource hierarchy /shelves/{shelf_id}/books/{book_id}. That means creating a new shelf will create a new resource with name = /shelves/{shelf_id}. If this name is later passed as the parent field in ListBooksRequest message, it won't work because the json transcoding won't recognize /v1/{parent=shelves/*}/books as a valid mapping. The best I can do is /v1/shelves/{parent}/books. This means I can either:

  1. Try and add server side logic to automatically convert parent field value from "{shelf_id}" to "/shelves/{shelf_id}".
  2. Require clients to perform the name/parent conversion logic to meet the requirements for the expected resource name rather than the server.
  3. Don't maintain collection names in the path of the resource names. For example, book names would be /{shelf_id}/{book_id} rather than /shelves/{shelf_id}/books/{book_id}.

All of these options seem pretty bad.