From 31c48791153625e542cba76588d201367026bb8f Mon Sep 17 00:00:00 2001 From: kojilin Date: Tue, 25 Aug 2020 18:56:25 +0900 Subject: [PATCH] Allow a user choose a path when a Thrift/gRPC service has more than two paths --- .../armeria/server/docs/DocService.java | 2 +- .../armeria/server/docs/MethodInfo.java | 3 +- .../src/containers/MethodPage/DebugPage.tsx | 53 ++++++++++++++++++- .../src/lib/transports/grpc-unframed.ts | 3 +- docs-client/src/lib/transports/thrift.ts | 3 +- .../server/grpc/GrpcDocServicePlugin.java | 7 ++- .../server/grpc/GrpcDocServicePluginTest.java | 35 ++++++++---- .../server/thrift/ThriftDocServicePlugin.java | 12 ++++- .../thrift/ThriftDocServicePluginTest.java | 37 +++++++++++++ 9 files changed, 137 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/server/docs/DocService.java b/core/src/main/java/com/linecorp/armeria/server/docs/DocService.java index ba057c2eb9a4..470214c2be5e 100644 --- a/core/src/main/java/com/linecorp/armeria/server/docs/DocService.java +++ b/core/src/main/java/com/linecorp/armeria/server/docs/DocService.java @@ -333,7 +333,7 @@ private ServiceInfo addServiceExamples(ServiceInfo service) { // generated by the plugin. concatAndDedup(exampleHeaders.get(m.name()), m.exampleHeaders()), concatAndDedup(exampleRequests.get(m.name()), m.exampleRequests()), - examplePaths.get(m.name()), + concatAndDedup(examplePaths.get(m.name()), m.examplePaths()), exampleQueries.get(m.name()), m.httpMethod(), m.docString()))::iterator, Iterables.concat(service.exampleHeaders(), exampleHeaders.get("")), diff --git a/core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java b/core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java index 2fd4e392ac32..86a4ccd5756a 100644 --- a/core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java +++ b/core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java @@ -118,8 +118,7 @@ public MethodInfo(String name, this.exampleRequests = ImmutableList.copyOf(requireNonNull(exampleRequests, "exampleRequests")); requireNonNull(examplePaths, "examplePaths"); - final ImmutableList.Builder examplePathsBuilder = - ImmutableList.builderWithExpectedSize(Iterables.size(examplePaths)); + final ImmutableList.Builder examplePathsBuilder = ImmutableList.builder(); for (String path : examplePaths) { final PathAndQuery pathAndQuery = PathAndQuery.parse(path); checkArgument(pathAndQuery != null, "examplePaths contains an invalid path: %s", path); diff --git a/docs-client/src/containers/MethodPage/DebugPage.tsx b/docs-client/src/containers/MethodPage/DebugPage.tsx index f0b844176a26..59695827c9d2 100644 --- a/docs-client/src/containers/MethodPage/DebugPage.tsx +++ b/docs-client/src/containers/MethodPage/DebugPage.tsx @@ -28,6 +28,7 @@ import React, { ChangeEvent, useCallback, useEffect, + useMemo, useReducer, useState, } from 'react'; @@ -195,6 +196,26 @@ const DebugPage: React.FunctionComponent = ({ setSnackbarOpen(false); }, []); + const validateRpcEndpointPath = useCallback( + (newEndpointPath: string) => { + if (!newEndpointPath) { + throw new Error('You must specify the endpoint path.'); + } + if ( + !method.endpoints + .map((endpoint) => endpoint.pathMapping) + .includes(newEndpointPath) + ) { + throw new Error( + `The path: '${newEndpointPath}' should be one of the: ${method.endpoints.map( + (endpoint) => endpoint.pathMapping, + )}`, + ); + } + }, + [method], + ); + const validateEndpointPath = useCallback( (newEndpointPath: string) => { if (!newEndpointPath) { @@ -324,6 +345,9 @@ const DebugPage: React.FunctionComponent = ({ `'${host}${escapeSingleQuote(additionalPath)}'` + `${queries.length > 0 ? `?${escapeSingleQuote(queries)}` : ''}'`; } + } else if (additionalPath.length > 0) { + validateRpcEndpointPath(additionalPath); + uri = `'${host}${escapeSingleQuote(additionalPath)}'`; } else { uri = `'${host}${escapeSingleQuote(path)}'`; } @@ -358,6 +382,7 @@ const DebugPage: React.FunctionComponent = ({ additionalQueries, exactPathMapping, validateEndpointPath, + validateRpcEndpointPath, additionalPath, ]); @@ -390,6 +415,8 @@ const DebugPage: React.FunctionComponent = ({ if (!exactPathMapping) { executedEndpointPath = params.get('endpoint_path') || undefined; } + } else { + executedEndpointPath = params.get('endpoint_path') || undefined; } const headersText = params.get('headers'); @@ -442,6 +469,12 @@ const DebugPage: React.FunctionComponent = ({ validateEndpointPath(additionalPath); params.set('endpoint_path', additionalPath); } + } else if (additionalPath.length > 0) { + validateRpcEndpointPath(additionalPath); + params.set('endpoint_path', additionalPath); + } else { + // Fall back to default endpoint. + params.delete('endpoint_path'); } if (headers) { @@ -482,10 +515,28 @@ const DebugPage: React.FunctionComponent = ({ requestBody, exactPathMapping, validateEndpointPath, + validateRpcEndpointPath, additionalPath, history, ]); + const supportedExamplePaths = useMemo(() => { + const transport = TRANSPORTS.getDebugTransport(method); + if (!transport) { + throw new Error("This method doesn't have a debug transport."); + } + return examplePaths.filter((path) => + method.endpoints.some((endpoint) => { + return ( + endpoint.pathMapping === path.value && + endpoint.availableMimeTypes.some((mimeType) => { + return transport.supportsMimeType(mimeType); + }) + ); + }), + ); + }, [examplePaths, method]); + return (
@@ -507,7 +558,7 @@ const DebugPage: React.FunctionComponent = ({ . { if (!bodyJson) { throw new Error('A gRPC request must have body.'); @@ -47,7 +48,7 @@ export default class GrpcUnframedTransport extends Transport { hdrs.set(name, value); } - const httpResponse = await fetch(endpoint.pathMapping, { + const httpResponse = await fetch(endpointPath || endpoint.pathMapping, { headers: hdrs, method: 'POST', body: bodyJson, diff --git a/docs-client/src/lib/transports/thrift.ts b/docs-client/src/lib/transports/thrift.ts index 10d0d6e1df8b..f8bc22c1ffd2 100644 --- a/docs-client/src/lib/transports/thrift.ts +++ b/docs-client/src/lib/transports/thrift.ts @@ -44,6 +44,7 @@ export default class ThriftTransport extends Transport { method: Method, headers: { [name: string]: string }, bodyJson?: string, + endpointPath?: string, ): Promise { if (!bodyJson) { throw new Error('A Thrift request must have body.'); @@ -58,7 +59,7 @@ export default class ThriftTransport extends Transport { hdrs.set(name, value); } - const httpResponse = await fetch(endpoint.pathMapping, { + const httpResponse = await fetch(endpointPath || endpoint.pathMapping, { headers: hdrs, method: 'POST', body: `{"method": "${thriftMethod}", "type": "CALL", "args": ${bodyJson}}`, diff --git a/grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/GrpcDocServicePlugin.java b/grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/GrpcDocServicePlugin.java index 10b6867d7178..0e273f1689b2 100644 --- a/grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/GrpcDocServicePlugin.java +++ b/grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/GrpcDocServicePlugin.java @@ -34,6 +34,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Streams; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.EnumDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor; @@ -276,6 +277,10 @@ static MethodInfo newMethodInfo(MethodDescriptor method, ServiceEntry service) { return builder.availableMimeTypes(e.availableMimeTypes()).build(); }) .collect(toImmutableSet()); + + final List examplePaths = + Streams.stream(methodEndpoints).map(EndpointInfo::pathMapping).collect(toImmutableList()); + return new MethodInfo( method.getName(), namedMessageSignature(method.getOutputType()), @@ -286,7 +291,7 @@ static MethodInfo newMethodInfo(MethodDescriptor method, ServiceEntry service) { methodEndpoints, /* exampleHeaders */ ImmutableList.of(), defaultExamples(method), - /* examplePaths */ ImmutableList.of(), + examplePaths, /* exampleQueries */ ImmutableList.of(), HttpMethod.POST, /* docString */ null); diff --git a/grpc/src/test/java/com/linecorp/armeria/internal/server/grpc/GrpcDocServicePluginTest.java b/grpc/src/test/java/com/linecorp/armeria/internal/server/grpc/GrpcDocServicePluginTest.java index e91c00209786..6132e6c6c465 100644 --- a/grpc/src/test/java/com/linecorp/armeria/internal/server/grpc/GrpcDocServicePluginTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/internal/server/grpc/GrpcDocServicePluginTest.java @@ -81,16 +81,31 @@ void servicesTest() throws Exception { UnitTestServiceGrpc.SERVICE_NAME, ReconnectServiceGrpc.SERVICE_NAME); - services.get(TestServiceGrpc.SERVICE_NAME).methods().forEach(m -> m.endpoints().forEach(e -> { - assertThat(e.pathMapping()).isEqualTo("/armeria.grpc.testing.TestService/" + m.name()); - })); - services.get(UnitTestServiceGrpc.SERVICE_NAME).methods().forEach(m -> m.endpoints().forEach(e -> { - assertThat(e.pathMapping()).isEqualTo("/test/armeria.grpc.testing.UnitTestService/" + m.name()); - })); - services.get(ReconnectServiceGrpc.SERVICE_NAME).methods().forEach(m -> m.endpoints().forEach(e -> { - assertThat(e.pathMapping()).isEqualTo("/reconnect/armeria.grpc.testing.ReconnectService/" + - m.name()); - })); + services.get(TestServiceGrpc.SERVICE_NAME).methods().forEach(m -> { + m.endpoints().forEach(e -> { + assertThat(e.pathMapping()).isEqualTo("/armeria.grpc.testing.TestService/" + m.name()); + }); + m.examplePaths().forEach(p -> { + assertThat(p).isEqualTo("/armeria.grpc.testing.TestService/" + m.name()); + }); + }); + services.get(UnitTestServiceGrpc.SERVICE_NAME).methods().forEach(m -> { + m.endpoints().forEach(e -> { + assertThat(e.pathMapping()).isEqualTo("/test/armeria.grpc.testing.UnitTestService/" + m.name()); + }); + m.examplePaths().forEach(p -> { + assertThat(p).isEqualTo("/test/armeria.grpc.testing.UnitTestService/" + m.name()); + }); + }); + services.get(ReconnectServiceGrpc.SERVICE_NAME).methods().forEach(m -> { + m.endpoints().forEach(e -> { + assertThat(e.pathMapping()).isEqualTo("/reconnect/armeria.grpc.testing.ReconnectService/" + + m.name()); + }); + m.examplePaths().forEach(p -> { + assertThat(p).isEqualTo("/reconnect/armeria.grpc.testing.ReconnectService/" + m.name()); + }); + }); } @Test diff --git a/thrift0.13/src/main/java/com/linecorp/armeria/internal/server/thrift/ThriftDocServicePlugin.java b/thrift0.13/src/main/java/com/linecorp/armeria/internal/server/thrift/ThriftDocServicePlugin.java index a638c069f3c0..b1175426bd36 100644 --- a/thrift0.13/src/main/java/com/linecorp/armeria/internal/server/thrift/ThriftDocServicePlugin.java +++ b/thrift0.13/src/main/java/com/linecorp/armeria/internal/server/thrift/ThriftDocServicePlugin.java @@ -53,7 +53,9 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Streams; +import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.thrift.ThriftProtocolFactories; import com.linecorp.armeria.server.Route; import com.linecorp.armeria.server.RoutePathType; @@ -266,7 +268,15 @@ private static MethodInfo newMethodInfo(String name, .map(TypeSignature::ofNamed) .collect(toImmutableList()); - return new MethodInfo(name, returnTypeSignature, parameters, exceptionTypeSignatures, endpoints); + final List examplePaths = + Streams.stream(endpoints).map(EndpointInfo::pathMapping).collect(toImmutableList()); + + return new MethodInfo(name, returnTypeSignature, parameters, exceptionTypeSignatures, endpoints, + ImmutableList.of(), + ImmutableList.of(), + examplePaths, + ImmutableList.of(), + HttpMethod.POST, null); } private static NamedTypeInfo newNamedTypeInfo(TypeSignature typeSignature) { diff --git a/thrift0.13/src/test/java/com/linecorp/armeria/internal/server/thrift/ThriftDocServicePluginTest.java b/thrift0.13/src/test/java/com/linecorp/armeria/internal/server/thrift/ThriftDocServicePluginTest.java index b9fd031d68bf..aaf2cff0780c 100644 --- a/thrift0.13/src/test/java/com/linecorp/armeria/internal/server/thrift/ThriftDocServicePluginTest.java +++ b/thrift0.13/src/test/java/com/linecorp/armeria/internal/server/thrift/ThriftDocServicePluginTest.java @@ -97,11 +97,48 @@ void servicesTest() { assertThat(methods).containsKey("bar4"); final MethodInfo bar4 = methods.get("bar4"); assertThat(bar4.exampleRequests()).isEmpty(); + assertThat(bar4.examplePaths()).containsExactly("/foo"); assertThat(bar4.endpoints()) .containsExactly(EndpointInfo.builder("*", "/foo") .defaultFormat(ThriftSerializationFormats.COMPACT).build()); } + @Test + public void multiplePaths() { + final Server server = + Server.builder() + .service(Route.builder() + .exact("/foo1") + .build(), + THttpService.ofFormats(mock(FooService.AsyncIface.class), + ThriftSerializationFormats.COMPACT)) + .service(Route.builder() + .exact("/foo2") + .build(), + THttpService.ofFormats(mock(FooService.AsyncIface.class), + ThriftSerializationFormats.COMPACT)) + .build(); + final ServiceSpecification specification = generator.generateSpecification( + ImmutableSet.copyOf(server.serviceConfigs()), + unifyFilter((plugin, service, method) -> true, + (plugin, service, method) -> false)); + + final ServiceInfo fooServiceInfo = specification.services().iterator().next(); + final Map methods = + fooServiceInfo.methods().stream() + .collect(toImmutableMap(MethodInfo::name, Function.identity())); + final MethodInfo bar4 = methods.get("bar4"); + assertThat(bar4.examplePaths()).containsExactlyInAnyOrder("/foo1", "/foo2"); + assertThat(bar4.endpoints()) + .containsExactlyInAnyOrder( + EndpointInfo.builder("*", "/foo1") + .defaultFormat(ThriftSerializationFormats.COMPACT) + .build(), + EndpointInfo.builder("*", "/foo2") + .defaultFormat(ThriftSerializationFormats.COMPACT) + .build()); + } + @Test void include() {