Skip to content

Commit

Permalink
Remove FieldInfo.childFieldInfo (#4566)
Browse files Browse the repository at this point in the history
Remove `FieldInfo.childFieldInfo`
Motivation:
We have introduced `TypeSignature` not to parse a struct type in place when creating `DocService` but put the struct in the list of `StructInfo` and link to it using `TypeSignature` in #422.
We could have the definition of a struct in only one place so we could dedup the information.
However, we have started to parse a bean(which is a struct) in place to represent a `@RequestObject` type in #1557.
To represent the type, `FieldInfo.childFieldInfo` was added and it turned out to be a bad decision.
Because after that, we have lots of duplicate information on structs. Also, the way that some of the structs use a link using `TypeSignature` whereas the rest of them don't, brought us the inconsistency of parsing and reorganizing the `DocService`.

Modifications:
- Remove `FieldInfo.childFieldInfos`.
  - `FieldInfo` now has only a `TypeSignature` and uses the `TypeSignature` to link to the corresponding struct.
  - The nested struct is not parsed in place.
- Add `REQUEST_OBJECT` `TypeSignature` for a `@RequestObject`.
- `FieldInfo`'s name for a `@RequestObject` is now the parameter name.
  - It was `class.getSimpleName` before which doesn't represent the field information properly.

Result:
- Unified way to create and organize `DocService`.
  • Loading branch information
minwoox authored Dec 15, 2022
1 parent 39efeb2 commit 6b7b1b0
Show file tree
Hide file tree
Showing 24 changed files with 356 additions and 1,252 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ static Set<AnnotatedValueResolver> uniqueResolverSet() {
return new TreeSet<>((o1, o2) -> {
final String o1Name = o1.httpElementName();
final String o2Name = o2.httpElementName();
if (o1Name != null && o1Name.equals(o2Name) && o1.annotationType() == o2.annotationType()) {
if (o1Name.equals(o2Name) && o1.annotationType() == o2.annotationType()) {
return 0;
}
// We are not ordering, but just finding duplicate elements.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
import com.linecorp.armeria.server.docs.ServiceSpecification;
import com.linecorp.armeria.server.docs.StructInfo;
import com.linecorp.armeria.server.docs.TypeSignature;
import com.linecorp.armeria.server.docs.TypeSignatureType;

/**
* A {@link DocServicePlugin} implementation that supports the {@link AnnotatedService}.
Expand Down Expand Up @@ -108,10 +109,6 @@ public final class AnnotatedDocServicePlugin implements DocServicePlugin {
static final TypeSignature STRING = TypeSignature.ofBase("string");
@VisibleForTesting
static final TypeSignature BINARY = TypeSignature.ofBase("binary");
@VisibleForTesting
static final TypeSignature BEAN = TypeSignature.ofBase("bean");
@VisibleForTesting
static final TypeSignature OBJECT = TypeSignature.ofBase("object");

private static final ObjectWriter objectWriter = JacksonUtil.newDefaultObjectMapper()
.writerWithDefaultPrettyPrinter();
Expand Down Expand Up @@ -150,8 +147,7 @@ public ServiceSpecification generateSpecification(Set<ServiceConfig> serviceConf
if (!filter.test(name(), className, methodName)) {
return;
}
addMethodInfo(methodInfos, sc.virtualHost().hostnamePattern(), service, serviceClass,
descriptiveTypeInfoProvider);
addMethodInfo(methodInfos, sc.virtualHost().hostnamePattern(), service, serviceClass);
addServiceDescription(serviceDescription, serviceClass);
}
});
Expand All @@ -166,15 +162,13 @@ private static void addServiceDescription(Map<Class<?>, DescriptionInfo> service

private static void addMethodInfo(Map<Class<?>, Set<MethodInfo>> methodInfos,
String hostnamePattern, AnnotatedService service,
Class<?> serviceClass,
DescriptiveTypeInfoProvider descriptiveTypeInfoProvider) {
Class<?> serviceClass) {
final Route route = service.route();
final EndpointInfo endpoint = endpointInfo(route, hostnamePattern);
final Method method = service.method();
final int overloadId = service.overloadId();
final TypeSignature returnTypeSignature = getReturnTypeSignature(method);
final List<FieldInfo> fieldInfos = fieldInfos(service.annotatedValueResolvers(),
descriptiveTypeInfoProvider);
final List<FieldInfo> fieldInfos = fieldInfos(service.annotatedValueResolvers());
route.methods().forEach(
httpMethod -> {
final MethodInfo methodInfo = new MethodInfo(
Expand Down Expand Up @@ -242,11 +236,10 @@ private static Set<MediaType> availableMimeTypes(Route route) {
return builder.build();
}

private static List<FieldInfo> fieldInfos(List<AnnotatedValueResolver> resolvers,
DescriptiveTypeInfoProvider descriptiveTypeInfoProvider) {
private static List<FieldInfo> fieldInfos(List<AnnotatedValueResolver> resolvers) {
final ImmutableList.Builder<FieldInfo> fieldInfosBuilder = ImmutableList.builder();
for (AnnotatedValueResolver resolver : resolvers) {
final FieldInfo fieldInfo = fieldInfo(resolver, descriptiveTypeInfoProvider);
final FieldInfo fieldInfo = fieldInfo(resolver);
if (fieldInfo != null) {
fieldInfosBuilder.add(fieldInfo);
}
Expand All @@ -255,8 +248,7 @@ private static List<FieldInfo> fieldInfos(List<AnnotatedValueResolver> resolvers
}

@Nullable
private static FieldInfo fieldInfo(AnnotatedValueResolver resolver,
DescriptiveTypeInfoProvider descriptiveTypeInfoProvider) {
private static FieldInfo fieldInfo(AnnotatedValueResolver resolver) {
final Class<? extends Annotation> annotationType = resolver.annotationType();
if (annotationType == RequestObject.class) {
final BeanFactoryId beanFactoryId = resolver.beanFactoryId();
Expand All @@ -267,34 +259,28 @@ private static FieldInfo fieldInfo(AnnotatedValueResolver resolver,
factory.methods().values().forEach(resolvers -> resolvers.forEach(builder::add));
factory.fields().values().forEach(builder::add);
final List<AnnotatedValueResolver> resolvers = builder.build();
if (!resolvers.isEmpty()) {
// Just use the simple name of the bean class as the field name.
return FieldInfo.builder(beanFactoryId.type().getSimpleName(), BEAN,
fieldInfos(resolvers, descriptiveTypeInfoProvider))
.build();
if (resolvers.isEmpty()) {
// Do not create a FieldInfo if resolvers is empty.
return null;
}

final Class<?> type = beanFactoryId.type();
final DescriptiveTypeSignature typeSignature =
new RequestObjectTypeSignature(TypeSignatureType.STRUCT, type.getName(), type,
new AnnotatedValueResolversWrapper(resolvers));
return FieldInfo.builder(resolver.httpElementName(), typeSignature)
.requirement(resolver.shouldExist() ?
FieldRequirement.REQUIRED : FieldRequirement.OPTIONAL)
.descriptionInfo(resolver.description())
.build();
} else {
// DescriptiveTypeInfoProvider may provide DescriptiveTypeInfo for the implicit request object.
final Class<?> elementType = resolver.elementType();
DescriptiveTypeInfo descriptiveTypeInfo =
descriptiveTypeInfoProvider.newDescriptiveTypeInfo(elementType);
if (descriptiveTypeInfo == null) {
descriptiveTypeInfo =
DEFAULT_REQUEST_DESCRIPTIVE_TYPE_INFO_PROVIDER.newDescriptiveTypeInfo(elementType);
}
if (descriptiveTypeInfo instanceof StructInfo &&
!((StructInfo) descriptiveTypeInfo).fields().isEmpty()) {
return FieldInfo.builder(descriptiveTypeInfo.name(), OBJECT,
((StructInfo) descriptiveTypeInfo).fields())
.requirement(resolver.shouldExist() ?
FieldRequirement.REQUIRED : FieldRequirement.OPTIONAL)
.build();
} else {
return FieldInfo.builder(elementType.getName(), toTypeSignature(elementType))
.requirement(resolver.shouldExist() ?
FieldRequirement.REQUIRED : FieldRequirement.OPTIONAL)
.build();
}
final DescriptiveTypeSignature typeSignature = TypeSignature.ofStruct(elementType);
return FieldInfo.builder(resolver.httpElementName(), typeSignature)
.requirement(resolver.shouldExist() ?
FieldRequirement.REQUIRED : FieldRequirement.OPTIONAL)
.descriptionInfo(resolver.description())
.build();
}
}

Expand All @@ -317,10 +303,7 @@ private static FieldInfo fieldInfo(AnnotatedValueResolver resolver,
} else {
signature = toTypeSignature(resolver.elementType());
}
final String name = resolver.httpElementName();
assert name != null;

return FieldInfo.builder(name, signature)
return FieldInfo.builder(resolver.httpElementName(), signature)
.location(location(resolver))
.requirement(resolver.shouldExist() ? FieldRequirement.REQUIRED
: FieldRequirement.OPTIONAL)
Expand Down Expand Up @@ -479,16 +462,25 @@ static ServiceSpecification generate(Map<Class<?>, DescriptionInfo> serviceDescr
requestDescriptiveTypes));
}

private static DescriptiveTypeInfo newDescriptiveTypeInfo(
@VisibleForTesting
static DescriptiveTypeInfo newDescriptiveTypeInfo(
DescriptiveTypeSignature typeSignature,
DescriptiveTypeInfoProvider provider,
Set<DescriptiveTypeSignature> requestDescriptiveTypes) {
final Object typeDescriptor = typeSignature.descriptor();
if (typeSignature instanceof RequestObjectTypeSignature) {
final Object annotatedValueResolvers =
((RequestObjectTypeSignature) typeSignature).annotatedValueResolvers();
if (annotatedValueResolvers instanceof AnnotatedValueResolversWrapper) {
final AnnotatedValueResolversWrapper resolvers =
(AnnotatedValueResolversWrapper) annotatedValueResolvers;
return new StructInfo(typeSignature.name(), fieldInfos(resolvers.resolvers()));
}
}
DescriptiveTypeInfo descriptiveTypeInfo = provider.newDescriptiveTypeInfo(typeDescriptor);
if (descriptiveTypeInfo != null) {
return descriptiveTypeInfo;
}

if (requestDescriptiveTypes.contains(typeSignature)) {
descriptiveTypeInfo =
DEFAULT_REQUEST_DESCRIPTIVE_TYPE_INFO_PROVIDER.newDescriptiveTypeInfo(typeDescriptor);
Expand Down Expand Up @@ -525,4 +517,17 @@ public String serializeExampleRequest(String serviceName, String methodName,
public String toString() {
return AnnotatedDocServicePlugin.class.getSimpleName();
}

private static class AnnotatedValueResolversWrapper {

private final List<AnnotatedValueResolver> resolvers;

AnnotatedValueResolversWrapper(List<AnnotatedValueResolver> resolvers) {
this.resolvers = resolvers;
}

List<AnnotatedValueResolver> resolvers() {
return resolvers;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,17 @@ static String findName(Header header, Object nameRetrievalTarget) {
return toHeaderName(getName(nameRetrievalTarget));
}

/**
* Returns the name of the specified element or the default name if it can't get.
*/
static String getNameOrDefault(Object element, String defaultName) {
try {
return getName(element);
} catch (Exception ignored) {
return defaultName;
}
}

/**
* Returns the name of the {@link Parameter} or the {@link Field}.
* @param element either {@link Parameter} or {@link Field}
Expand Down
Loading

0 comments on commit 6b7b1b0

Please sign in to comment.