Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow setting example paths and queries for DocService #2546

Merged
merged 12 commits into from Mar 10, 2020
Merged

Allow setting example paths and queries for DocService #2546

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
108a21d
Provide a way to set example paths and queries when building DocService
ikhoon Mar 2, 2020
cd07028
Allow adding multiple example paths and queries
ikhoon Mar 3, 2020
5825c19
Fix checkstyle and clean up
ikhoon Mar 3, 2020
23e5674
Clean up
ikhoon Mar 3, 2020
a53840a
Add param comments
ikhoon Mar 3, 2020
f5957e4
Fix NPE
ikhoon Mar 4, 2020
10c9559
Address comments by @hexoul / Remove white space
ikhoon Mar 4, 2020
a159727
Address comments by @trustin
ikhoon Mar 6, 2020
773418f
Address comments by @trustin
ikhoon Mar 9, 2020
908e99e
Address comments by @trustin / Use normalized path and queryString
ikhoon Mar 9, 2020
10d07ee
Address comments by @trustin / Preserve the original queryString
ikhoon Mar 10, 2020
45826bb
Preserve original queryString in DocService client
ikhoon Mar 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion core/src/main/java/com/linecorp/armeria/server/docs/DocService.java
View file
Open in desktop
Expand Up @@ -99,6 +99,8 @@ public static DocServiceBuilder builder() {

private final Map<String, ListMultimap<String, HttpHeaders>> exampleHttpHeaders;
private final Map<String, ListMultimap<String, String>> exampleRequests;
private final Map<String, ListMultimap<String, String>> examplePaths;
private final Map<String, ListMultimap<String, String>> exampleQueries;
private final DocServiceFilter filter;

@Nullable
Expand All @@ -108,14 +110,18 @@ public static DocServiceBuilder builder() {
* Creates a new instance.
*/
public DocService() {
this(ImmutableMap.of(), ImmutableMap.of(), ImmutableList.of(), DocServiceBuilder.ALL_SERVICES);
this(/* exampleHttpHeaders */ ImmutableMap.of(), /* exampleRequests */ ImmutableMap.of(),
/* examplePaths */ ImmutableMap.of(), /* exampleQueries */ ImmutableMap.of(),
/* injectedScriptSuppliers */ ImmutableList.of(), DocServiceBuilder.ALL_SERVICES);
}

/**
* Creates a new instance with example HTTP headers and example requests and injected scripts.
*/
DocService(Map<String, ListMultimap<String, HttpHeaders>> exampleHttpHeaders,
Map<String, ListMultimap<String, String>> exampleRequests,
Map<String, ListMultimap<String, String>> examplePaths,
Map<String, ListMultimap<String, String>> exampleQueries,
List<BiFunction<ServiceRequestContext, HttpRequest, String>> injectedScriptSuppliers,
DocServiceFilter filter) {

Expand All @@ -130,6 +136,8 @@ public DocService() {
"com/linecorp/armeria/server/docs")));
this.exampleHttpHeaders = immutableCopyOf(exampleHttpHeaders, "exampleHttpHeaders");
this.exampleRequests = immutableCopyOf(exampleRequests, "exampleRequests");
this.examplePaths = immutableCopyOf(examplePaths, "examplePaths");
this.exampleQueries = immutableCopyOf(exampleQueries, "exampleQueries");
this.filter = requireNonNull(filter, "filter");
}

Expand Down Expand Up @@ -235,6 +243,8 @@ private static MethodInfo addMethodDocStrings(ServiceInfo service, MethodInfo me
method.endpoints(),
method.exampleHttpHeaders(),
method.exampleRequests(),
method.examplePaths(),
method.exampleQueries(),
method.httpMethod(),
docString(service.name() + '/' + method.name(), method.docString(), docStrings));
}
Expand Down Expand Up @@ -311,6 +321,10 @@ private ServiceInfo addServiceExamples(ServiceInfo service) {
this.exampleHttpHeaders.getOrDefault(service.name(), ImmutableListMultimap.of());
final ListMultimap<String, String> exampleRequests =
this.exampleRequests.getOrDefault(service.name(), ImmutableListMultimap.of());
final ListMultimap<String, String> examplePaths =
this.examplePaths.getOrDefault(service.name(), ImmutableListMultimap.of());
final ListMultimap<String, String> exampleQueries =
this.exampleQueries.getOrDefault(service.name(), ImmutableListMultimap.of());

// Reconstruct ServiceInfo with the examples.
return new ServiceInfo(
Expand All @@ -323,6 +337,8 @@ private ServiceInfo addServiceExamples(ServiceInfo service) {
// generated by the plugin.
concatAndDedup(exampleHttpHeaders.get(m.name()), m.exampleHttpHeaders()),
concatAndDedup(exampleRequests.get(m.name()), m.exampleRequests()),
examplePaths.get(m.name()),
exampleQueries.get(m.name()),
m.httpMethod(), m.docString()))::iterator,
Iterables.concat(service.exampleHttpHeaders(),
exampleHttpHeaders.get("")),
Expand Down
96 changes: 94 additions & 2 deletions core/src/main/java/com/linecorp/armeria/server/docs/DocServiceBuilder.java
View file
Open in desktop
Expand Up @@ -31,6 +31,7 @@

import com.linecorp.armeria.common.HttpHeaders;
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.internal.common.PathAndQuery;
import com.linecorp.armeria.server.ServiceRequestContext;

/**
Expand All @@ -55,6 +56,8 @@ public final class DocServiceBuilder {

private final Map<String, ListMultimap<String, HttpHeaders>> exampleHttpHeaders = new HashMap<>();
private final Map<String, ListMultimap<String, String>> exampleRequests = new HashMap<>();
private final Map<String, ListMultimap<String, String>> examplePaths = new HashMap<>();
private final Map<String, ListMultimap<String, String>> exampleQueries = new HashMap<>();
private final List<BiFunction<ServiceRequestContext, HttpRequest, String>> injectedScriptSuppliers =
new ArrayList<>();

Expand Down Expand Up @@ -183,6 +186,95 @@ private DocServiceBuilder exampleHttpHeaders0(String serviceName, String methodN
return this;
}

/**
* Adds the specified example paths for the method with the specified service and method name.
*/
public DocServiceBuilder examplePaths(Class<?> serviceType, String methodName, String... examplePaths) {
ikhoon marked this conversation as resolved.
Show resolved Hide resolved
requireNonNull(serviceType, "serviceType");
return examplePaths(serviceType.getName(), methodName, examplePaths);
}

/**
* Adds the specified example paths for the method with the specified service and method name.
*/
public DocServiceBuilder examplePaths(Class<?> serviceType, String methodName,
Iterable<String> examplePaths) {
ikhoon marked this conversation as resolved.
Show resolved Hide resolved
requireNonNull(serviceType, "serviceType");
return examplePaths(serviceType.getName(), methodName, examplePaths);
}

/**
* Adds the specified example paths for the method with the specified service and method name.
*/
public DocServiceBuilder examplePaths(String serviceName, String methodName, String... examplePaths) {
ikhoon marked this conversation as resolved.
Show resolved Hide resolved
requireNonNull(examplePaths, "examplePaths");
return examplePaths(serviceName, methodName, ImmutableList.copyOf(examplePaths));
}

/**
* Adds the specified example paths for the method with the specified service and method name.
*/
public DocServiceBuilder examplePaths(String serviceName, String methodName,
Iterable<String> examplePaths) {
ikhoon marked this conversation as resolved.
Show resolved Hide resolved
requireNonNull(serviceName, "serviceName");
checkArgument(!serviceName.isEmpty(), "serviceName is empty.");
requireNonNull(methodName, "methodName");
checkArgument(!methodName.isEmpty(), "methodName is empty.");
requireNonNull(examplePaths, "examplePaths");
for (String examplePath : examplePaths) {
requireNonNull(examplePath, "examplePaths contains null");
examplePaths.forEach(path -> checkArgument(PathAndQuery.parse(path) != null,
"examplePaths contain an invalid path: {}", path));
this.examplePaths.computeIfAbsent(serviceName, unused -> ArrayListMultimap.create())
.put(methodName, examplePath);
}
return this;
}

/**
* Adds the specified example query strings for the method with the specified service and method name.
*/
public DocServiceBuilder exampleQueries(Class<?> serviceType, String methodName, String... queryStrings) {
requireNonNull(serviceType, "serviceType");
return exampleQueries(serviceType.getName(), methodName, queryStrings);
}

/**
* Adds the specified example query strings for the method with the specified service and method name.
*/
public DocServiceBuilder exampleQueries(Class<?> serviceType, String methodName, Iterable<String> queries) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

queries -> queryStrings

requireNonNull(serviceType, "serviceType");
return exampleQueries(serviceType.getName(), methodName, queries);
}

/**
* Adds the specified example query strings for the method with the specified service and method name.
*/
public DocServiceBuilder exampleQueries(String serviceName, String methodName, String... queryStrings) {
requireNonNull(queryStrings, "queryStrings");
return exampleQueries(serviceName, methodName, ImmutableList.copyOf(queryStrings));
}

/**
* Adds the specified example query strings for the method with the specified service and method name.
*/
public DocServiceBuilder exampleQueries(String serviceName, String methodName, Iterable<String> queries) {
ikhoon marked this conversation as resolved.
Show resolved Hide resolved
requireNonNull(serviceName, "serviceName");
checkArgument(!serviceName.isEmpty(), "serviceName is empty.");
requireNonNull(methodName, "methodName");
checkArgument(!methodName.isEmpty(), "methodName is empty.");
requireNonNull(queries, "queries");
for (String query : queries) {
requireNonNull(query, "queries contains null");
final PathAndQuery pathAndQuery = PathAndQuery.parse(query.startsWith("?") ? query : '?' + query);
checkArgument(pathAndQuery != null,
"exampleQueries contain an invalid query string: {}", query);
exampleQueries.computeIfAbsent(serviceName, unused -> ArrayListMultimap.create())
.put(methodName, query);
}
return this;
}

/**
* Adds the example requests for the method with the specified service type and method name.
* This method is a shortcut to:
Expand Down Expand Up @@ -452,7 +544,7 @@ private static String[] guessAndSerializeExampleRequest(Object exampleRequest) {
* Returns a newly-created {@link DocService} based on the properties of this builder.
*/
public DocService build() {
return new DocService(exampleHttpHeaders, exampleRequests, injectedScriptSuppliers,
unifyFilter(includeFilter, excludeFilter));
return new DocService(exampleHttpHeaders, exampleRequests, examplePaths, exampleQueries,
injectedScriptSuppliers, unifyFilter(includeFilter, excludeFilter));
}
}
28 changes: 26 additions & 2 deletions core/src/main/java/com/linecorp/armeria/server/docs/MethodInfo.java
View file
Open in desktop
Expand Up @@ -53,6 +53,8 @@ public final class MethodInfo {
private final Set<EndpointInfo> endpoints;
private final List<HttpHeaders> exampleHttpHeaders;
private final List<String> exampleRequests;
private final List<String> examplePaths;
private final List<String> exampleQueries;
private final HttpMethod httpMethod;
@Nullable
private final String docString;
Expand All @@ -78,8 +80,10 @@ public MethodInfo(String name,
Iterable<EndpointInfo> endpoints,
HttpMethod httpMethod,
@Nullable String docString) {
this(name, returnTypeSignature, parameters, exceptionTypeSignatures,
endpoints, ImmutableList.of(), ImmutableList.of(), httpMethod, docString);
this(name, returnTypeSignature, parameters, exceptionTypeSignatures, endpoints,
/* exampleHttpHeaders */ ImmutableList.of(), /* exampleRequests */ ImmutableList.of(),
/* examplePaths */ ImmutableList.of(), /* exampleQueries */ ImmutableList.of(),
httpMethod, docString);
}

/**
Expand All @@ -92,6 +96,8 @@ public MethodInfo(String name,
Iterable<EndpointInfo> endpoints,
Iterable<HttpHeaders> exampleHttpHeaders,
Iterable<String> exampleRequests,
Iterable<String> examplePaths,
Iterable<String> exampleQueries,
HttpMethod httpMethod,
@Nullable String docString) {
this.name = requireNonNull(name, "name");
Expand All @@ -108,6 +114,8 @@ public MethodInfo(String name,
this.exampleHttpHeaders = ImmutableList.copyOf(requireNonNull(exampleHttpHeaders,
"exampleHttpHeaders"));
this.exampleRequests = ImmutableList.copyOf(requireNonNull(exampleRequests, "exampleRequests"));
this.examplePaths = ImmutableList.copyOf(requireNonNull(examplePaths, "examplePaths"));
this.exampleQueries = ImmutableList.copyOf(requireNonNull(exampleQueries, "exampleQueries"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about validating all paths and query strings, using PathAndQuery and QueryParams.fromQueryString()?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not addressed yet.

this.httpMethod = requireNonNull(httpMethod, "httpMethod");
this.docString = Strings.emptyToNull(docString);
}
Expand Down Expand Up @@ -169,6 +177,22 @@ public List<String> exampleRequests() {
return exampleRequests;
}

/**
* Returns the example paths of the method.
*/
@JsonProperty
public List<String> examplePaths() {
return examplePaths;
}

/**
* Returns the example queries of the method.
*/
@JsonProperty
public List<String> exampleQueries() {
return exampleQueries;
}

/**
* Returns the HTTP method of this method.
*/
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/com/linecorp/armeria/server/docs/ServiceInfo.java
View file
Open in desktop
Expand Up @@ -123,8 +123,8 @@ static Set<MethodInfo> mergeEndpoints(Iterable<MethodInfo> methodInfos) {
return new MethodInfo(value.name(), value.returnTypeSignature(),
value.parameters(), value.exceptionTypeSignatures(),
endpointInfos, value.exampleHttpHeaders(),
value.exampleRequests(), value.httpMethod(),
value.docString());
value.exampleRequests(), value.examplePaths(), value.exampleQueries(),
value.httpMethod(), value.docString());
}
});
}
Expand Down
48 changes: 45 additions & 3 deletions ...rc/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedDocServiceTest.java
View file
Open in desktop
Expand Up @@ -46,6 +46,7 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.TextNode;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;

Expand Down Expand Up @@ -106,9 +107,15 @@ protected void configure(ServerBuilder sb) throws Exception {
DocService.builder()
.exampleHttpHeaders(EXAMPLE_HEADERS_ALL)
.exampleHttpHeaders(MyService.class, EXAMPLE_HEADERS_SERVICE)
.exampleHttpHeaders(MyService.class,"pathParams", EXAMPLE_HEADERS_METHOD)
.exampleHttpHeaders(MyService.class, "pathParams", EXAMPLE_HEADERS_METHOD)
.examplePaths(MyService.class, "pathParams",
"/service/hello1/foo/hello3/bar")
.exampleQueries(MyService.class, "foo", "query=10", "query=20")
.exampleRequestForMethod(MyService.class, "pathParams",
ImmutableList.of(mapper.readTree("{\"hello\":\"armeria\"}")))
.examplePaths(MyService.class, "pathParamsWithQueries",
"/service/hello1/foo", "/service/hello1/bar")
.exampleQueries(MyService.class, "pathParamsWithQueries", "hello3=hello4")
.exclude(DocServiceFilter.ofMethodName(MyService.class.getName(), "exclude1").or(
DocServiceFilter.ofMethodName(MyService.class.getName(), "exclude2")))
.build());
Expand All @@ -128,6 +135,7 @@ public void jsonSpecification() throws InterruptedException {
addAllMethodsMethodInfos(methodInfos);
addIntsMethodInfo(methodInfos);
addPathParamsMethodInfo(methodInfos);
addPathParamsWithQueriesMethodInfo(methodInfos);
addRegexMethodInfo(methodInfos);
addPrefixMethodInfo(methodInfos);
addConsumesMethodInfo(methodInfos);
Expand Down Expand Up @@ -207,6 +215,19 @@ private static void addPathParamsMethodInfo(Map<Class<?>, Set<MethodInfo>> metho
methodInfos.computeIfAbsent(MyService.class, unused -> new HashSet<>()).add(methodInfo);
}

private static void addPathParamsWithQueriesMethodInfo(Map<Class<?>, Set<MethodInfo>> methodInfos) {
final EndpointInfo endpoint = EndpointInfo.builder("*", "/service/hello1/{hello2}")
.availableMimeTypes(MediaType.JSON_UTF_8)
.build();
final List<FieldInfo> fieldInfos = ImmutableList.of(
FieldInfo.builder("hello2", STRING).requirement(REQUIRED).location(PATH).build(),
FieldInfo.builder("hello3", STRING).requirement(REQUIRED).location(QUERY).build());
final MethodInfo methodInfo = new MethodInfo(
"pathParamsWithQueries", STRING, fieldInfos, ImmutableList.of(),
ImmutableList.of(endpoint), HttpMethod.GET, null);
methodInfos.computeIfAbsent(MyService.class, unused -> new HashSet<>()).add(methodInfo);
}

private static void addRegexMethodInfo(Map<Class<?>, Set<MethodInfo>> methodInfos) {
final EndpointInfo endpoint = EndpointInfo.builder("*", "regex:/(bar|baz)")
.regexPathPrefix("prefix:/service/")
Expand Down Expand Up @@ -285,13 +306,29 @@ private static void addExamples(JsonNode json) {
service.get("methods").forEach(method -> {
final String methodName = method.get("name").textValue();
final ArrayNode exampleHttpHeaders = (ArrayNode) method.get("exampleHttpHeaders");
if (MyService.class.getName().equals(serviceName) &&
"pathParams".equals(methodName)) {
if (MyService.class.getName().equals(serviceName) && "pathParams".equals(methodName)) {
exampleHttpHeaders.add(mapper.valueToTree(EXAMPLE_HEADERS_METHOD));
final ArrayNode exampleRequests = (ArrayNode) method.get("exampleRequests");
exampleRequests.add('{' + System.lineSeparator() +
" \"hello\" : \"armeria\"" + System.lineSeparator() +
'}');
final ArrayNode examplePaths = (ArrayNode) method.get("examplePaths");
examplePaths.add(TextNode.valueOf("/service/hello1/foo/hello3/bar"));
}

if (MyService.class.getName().equals(serviceName) && "foo".equals(methodName)) {
final ArrayNode exampleQueries = (ArrayNode) method.get("exampleQueries");
exampleQueries.add(TextNode.valueOf("query=10"));
exampleQueries.add(TextNode.valueOf("query=20"));
}

if (MyService.class.getName().equals(serviceName) &&
"pathParamsWithQueries".equals(methodName)) {
final ArrayNode examplePaths = (ArrayNode) method.get("examplePaths");
examplePaths.add(TextNode.valueOf("/service/hello1/foo"));
examplePaths.add(TextNode.valueOf("/service/hello1/bar"));
final ArrayNode exampleQueries = (ArrayNode) method.get("exampleQueries");
exampleQueries.add(TextNode.valueOf("hello3=hello4"));
}
});
});
Expand Down Expand Up @@ -347,6 +384,11 @@ public String pathParams(@Param String hello2, @Param String hello4) {
return hello2 + ' ' + hello4;
}

@Get("/hello1/:hello2")
public String pathParamsWithQueries(@Param String hello2, @Param String hello3) {
return hello2 + ' ' + hello3;
}

@Get("regex:/(bar|baz)")
public List<String>[] regex(@Param MyEnum myEnum) {
final MyEnum[] values = MyEnum.values();
Expand Down