Support RequestObject in DocService #1557
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
minwoox
force-pushed
the
requestObject_docservice
branch
4 times, most recently
from
28a783f
to
1db9c8d
Compare
Codecov Report
@@ Coverage Diff @@
## master #1557 +/- ##
===========================================
- Coverage 72.71% 72.7% -0.01%
- Complexity 7387 7402 +15
===========================================
Files 685 688 +3
Lines 29853 29942 +89
Branches 3640 3648 +8
===========================================
+ Hits 21708 21770 +62
- Misses 6263 6282 +19
- Partials 1882 1890 +8
Continue to review full report at Codecov.
|
hyeonjames
approved these changes
Jan 29, 2019
minwoox
force-pushed
the
requestObject_docservice
branch
from
b7908b2
to
6ca1cbf
Compare
trustin
requested changes
Jan 30, 2019
core/src/main/java/com/linecorp/armeria/internal/annotation/AnnotatedBeanFactoryRegistry.java
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/annotation/AnnotatedBeanFactoryRegistry.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/annotation/AnnotatedBeanFactoryRegistry.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/docs/FieldLocation.java
Outdated
Show resolved
Hide resolved
core/src/test/java/com/linecorp/armeria/internal/annotation/AnnotatedHttpDocServiceTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/com/linecorp/armeria/internal/annotation/AnnotatedHttpDocServiceTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/com/linecorp/armeria/internal/annotation/AnnotatedHttpDocServiceTest.java
Show resolved
Hide resolved
hyangtack
reviewed
Jan 30, 2019
core/src/main/java/com/linecorp/armeria/internal/annotation/AnnotatedBeanFactoryRegistry.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/annotation/AnnotatedHttpDocServicePlugin.java
Outdated
Show resolved
Hide resolved
minwoox
force-pushed
the
requestObject_docservice
branch
2 times, most recently
from
02e39a0
to
65ca0f0
Compare
trustin
requested changes
Jan 31, 2019
core/src/main/java/com/linecorp/armeria/internal/annotation/AnnotatedBeanFactoryRegistry.java
Outdated
Show resolved
Hide resolved
thrift/src/test/java/com/linecorp/armeria/server/thrift/ThriftDocServicePluginTest.java
Outdated
Show resolved
Hide resolved
trustin
approved these changes
Jan 31, 2019
hyangtack
approved these changes
Jan 31, 2019
| private final Map<Method, List<AnnotatedValueResolver>> methods; | ||
|
|
||
| AnnotatedBeanFactory(BeanFactoryId beanFactoryId, | ||
| Entry<Constructor<T>, List<AnnotatedValueResolver>> constructor, |
| @@ -0,0 +1,327 @@ | |||
| /* | |||
| * Copyright 2018 LINE Corporation | |||
| final Builder<Method, List<AnnotatedValueResolver>> methodsBuilder = ImmutableMap.builder(); | ||
| final Set<Method> methods = getAllMethods(beanFactoryId.type); | ||
| for (final Method method : methods) { | ||
| final RequestConverter[] converters = method.getAnnotationsByType(RequestConverter.class); |
There was a problem hiding this comment.
nit: How about using recently-added AnnotationUtil#findDeclared for consistency? 😄
| final Builder<Field, AnnotatedValueResolver> builder = ImmutableMap.builder(); | ||
| final Set<Field> fields = getAllFields(beanFactoryId.type); | ||
| for (final Field field : fields) { | ||
| final RequestConverter[] converters = field.getAnnotationsByType(RequestConverter.class); |
| factory.fields().values().forEach(builder::add); | ||
| final List<AnnotatedValueResolver> resolvers = builder.build(); | ||
| if (!resolvers.isEmpty()) { | ||
| // TODO(minwoox) Should we support http element name for the bean type? |
There was a problem hiding this comment.
nit: If this TODO is not necessary, how about removing it and leaving a short description how we handle the bean type?
Motivation: Currently, `RequestObject`s are not shown in `DocService`. Modifications: - Rename `AnnotatedBeanFactory` to `AnnotatedBeanFactoryRegistry` - Add `AnnotatedBeanFactory` which has information about the bean factory - Add `EndpointInfoBuilder` to make it `EndpointInfo` easily - Add `FieldLocation` enum Result: - Better developer experience
minwoox
force-pushed
the
requestObject_docservice
branch
from
aab1392
to
76b9294
Compare
hyangtack
reviewed
Feb 1, 2019
| @@ -372,7 +373,7 @@ | |||
| final RequestObject requestObject = annotatedElement.getAnnotation(RequestObject.class); | |||
| if (requestObject != null) { | |||
| // Find more request converters from a field or parameter. | |||
| final RequestConverter[] converters = typeElement.getAnnotationsByType(RequestConverter.class); | |||
| final List<RequestConverter> converters = findDeclared(typeElement, RequestConverter.class); | |||
minwoox
force-pushed
the
requestObject_docservice
branch
from
76b9294
to
1b54acc
Compare
fmguerreiro
pushed a commit
to fmguerreiro/armeria
that referenced
this pull request
Sep 19, 2020
Motivation: Currently, `RequestObject`s are not shown in `DocService`. Modifications: - Rename `AnnotatedBeanFactory` to `AnnotatedBeanFactoryRegistry` - Add `AnnotatedBeanFactory` which has information about the bean factory - Add `FieldInfoBuilder` to make `FieldInfo` easily - Add `FieldLocation` enum - Change the `DEFAULT` in `FieldRequirement` to `UNSPECIFIED` when it's unknown - Bean fields are now collapsible in `DocService` Result: - Better developer experience
minwoox
added a commit
to minwoox/armeria
that referenced
this pull request
Dec 9, 2022
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 line#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 line#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`.
ikhoon
pushed a commit
that referenced
this pull request
Dec 15, 2022
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`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
Currently,
RequestObjects are not shown inDocService.Modifications:
AnnotatedBeanFactorytoAnnotatedBeanFactoryRegistryAnnotatedBeanFactorywhich has information about the bean factoryFieldInfoBuilderto makeFieldInfoeasilyFieldLocationenumDEFAULTinFieldRequirementtoUNSPECIFIEDwhen it's unknownDocServiceResult: