Skip to content

Commit

Permalink
Fixed a bug where StructInfo is not linked in DocService when a pro…
Browse files Browse the repository at this point in the history
…tobuf is used both for gRPC and annotated (#4885)

Motivation:

If a generated protobuf `Message` is used with both a gRPC service and an annotated service, `StructInfo.alias()` could be removed while merging `StructInfo`s. Because `ServiceSpecification.merge()` deduplicates `StructInfo`s that have the same name.

https://github.com/line/armeria/blob/6b7b1b0b638f3d780f5910a133daf24902af8d93/core/src/main/java/com/linecorp/armeria/server/docs/ServiceSpecification.java#L151-L155

Background:

gRPC `DocService` creates `StructInfo` from protobuf's `Descriptor`. The generated `Message` class is unknown in the `Descriptor`. Only the message name defined in the `.proto` file can be extracted to `StructInfo`.

However, annotated services make `StructInfo` from `Message` generated by the protobuf compiler. The class name of `Message` is injected as an alias into the `Structinfo` created from the `Descriptor` so that `DocService` links the parameter class to the protobuf `Structinfo`.

https://github.com/line/armeria/blob/8652e5ea544a99790cab391274dde22370b96d35/docs-client/src/lib/specification.tsx#L117-L123

Modifications:

- If two `StructInfo`s have the same name, preserve the one with the alias.
- Miscellaneous) Fixed `JsonSchemaGenerator` so that `StructInfo` can also be found by an alias.

Result:

`DocService` now correctly makes the link to `StructInfo` even when a protobuf `Message` is used in both gRPC services and annotated services.
  • Loading branch information
ikhoon committed Jun 2, 2023
1 parent 841b5b4 commit f4b223a
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMap.Builder;

import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.internal.common.JacksonUtil;
Expand Down Expand Up @@ -71,8 +73,16 @@ static ArrayNode generate(ServiceSpecification serviceSpecification) {

private JsonSchemaGenerator(ServiceSpecification serviceSpecification) {
serviceInfos = serviceSpecification.services();
typeSignatureToStructMapping = serviceSpecification.structs().stream().collect(
toImmutableMap(StructInfo::name, Function.identity()));
final ImmutableMap.Builder<String, StructInfo> typeSignatureToStructMappingBuilder =
ImmutableMap.builderWithExpectedSize(serviceSpecification.structs().size());
for (StructInfo struct : serviceSpecification.structs()) {
typeSignatureToStructMappingBuilder.put(struct.name(), struct);
if (struct.alias() != null) {
// TypeSignature.signature() could be StructInfo.alias() if the type is a protobuf Message.
typeSignatureToStructMappingBuilder.put(struct.alias(), struct);
}
}
typeSignatureToStructMapping = typeSignatureToStructMappingBuilder.build();
typeNameToEnumMapping = serviceSpecification.enums().stream().collect(
toImmutableMap(EnumInfo::name, Function.identity()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -143,7 +144,7 @@ public ServiceSpecification(Iterable<ServiceInfo> services,
this.services = Streams.stream(requireNonNull(services, "services"))
.collect(toImmutableSortedSet(comparing(ServiceInfo::name)));
this.enums = collectDescriptiveTypeInfo(enums, "enums");
this.structs = collectDescriptiveTypeInfo(structs, "structs");
this.structs = collectStructInfo(structs);
this.exceptions = collectDescriptiveTypeInfo(exceptions, "exceptions");
this.exampleHeaders = ImmutableList.copyOf(requireNonNull(exampleHeaders, "exampleHeaders"));
}
Expand All @@ -154,6 +155,24 @@ private static <T extends DescriptiveTypeInfo> Set<T> collectDescriptiveTypeInfo
.collect(toImmutableSortedSet(comparing(DescriptiveTypeInfo::name)));
}

private static Set<StructInfo> collectStructInfo(Iterable<StructInfo> structInfos) {
requireNonNull(structInfos, "structInfos");
return Streams.stream(structInfos)
.collect(Collectors.toMap(StructInfo::name, Function.identity(),
(a, b) -> {
// If the name is duplicate, prefer the one with alias.
if (a.alias() != null) {
return a;
}
if (b.alias() != null) {
return b;
}
return a;
}))
.values().stream()
.collect(toImmutableSortedSet(comparing(StructInfo::name)));
}

/**
* Returns the metadata about the services in this specification.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation licenses this file to you 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:
*
* https://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.
*/

package com.linecorp.armeria.server.docs;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;

import com.google.common.collect.ImmutableList;

class ServiceSpecificationTest {

@Test
void preferAliasedStructInfo() {
final StructInfo noAlias = new StructInfo("foo", ImmutableList.of());
final StructInfo aliased = noAlias.withAlias("bar");
ServiceSpecification specification =
new ServiceSpecification(ImmutableList.of(), ImmutableList.of(),
ImmutableList.of(noAlias, aliased), ImmutableList.of());
assertThat(specification.structs()).containsExactly(aliased);

specification = new ServiceSpecification(ImmutableList.of(), ImmutableList.of(),
ImmutableList.of(aliased, noAlias), ImmutableList.of());
assertThat(specification.structs()).containsExactly(aliased);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation licenses this file to you 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:
*
* https://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.
*/

package com.linecorp.armeria.it.grpc;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import com.fasterxml.jackson.databind.JsonNode;

import com.linecorp.armeria.client.BlockingWebClient;
import com.linecorp.armeria.common.CommonPools;
import com.linecorp.armeria.grpc.testing.Messages;
import com.linecorp.armeria.internal.common.grpc.TestServiceImpl;
import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.server.annotation.ConsumesJson;
import com.linecorp.armeria.server.annotation.Post;
import com.linecorp.armeria.server.annotation.ProducesJson;
import com.linecorp.armeria.server.docs.DocService;
import com.linecorp.armeria.server.grpc.GrpcService;
import com.linecorp.armeria.server.logging.LoggingService;
import com.linecorp.armeria.testing.junit5.server.ServerExtension;

class DuplicateStructInfoSpecificationTest {

@RegisterExtension
static ServerExtension server = new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) {
sb.annotatedService(new SimpleAnnotatedService());
sb.service(GrpcService.builder()
.addService(new TestServiceImpl(CommonPools.blockingTaskExecutor()))
.enableUnframedRequests(true)
.build());
sb.serviceUnder("/docs", new DocService());
sb.decorator(LoggingService.newDecorator());
}
};

@Test
void shouldHaveAliasInSpecification() throws InterruptedException {
final BlockingWebClient client = server.blockingWebClient();
final JsonNode spec = client.prepare()
.get("/docs/specification.json").asJson(JsonNode.class)
.execute().content();
boolean found = false;
for (JsonNode struct : spec.get("structs")) {
if ("armeria.grpc.testing.SimpleRequest".equals(struct.get("name").asText())) {
found = true;
assertThat(struct.get("alias").asText())
.isEqualTo("com.linecorp.armeria.grpc.testing.Messages$SimpleRequest");
}
}
assertThat(found)
.describedAs("Failed to find a StructInfo for 'armeria.grpc.testing.SimpleRequest'")
.isTrue();
}

private static class SimpleAnnotatedService {

@ConsumesJson
@ProducesJson
@Post("/echo")
public Messages.SimpleRequest echo(Messages.SimpleRequest request) {
return request;
}
}
}

0 comments on commit f4b223a

Please sign in to comment.