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.

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

Null case of @Default not covered #1858

Closed
southernkasaist opened this issue Jun 26, 2019 · 4 comments · Fixed by #1864
Closed

Null case of @Default not covered #1858

southernkasaist opened this issue Jun 26, 2019 · 4 comments · Fixed by #1864
Labels
defect
Milestone
0.88.0

Comments

@southernkasaist
Copy link

armeria/core/src/main/java/com/linecorp/armeria/internal/annotation/AnnotatedValueResolver.java

Line 1026 in 61a72e9

defaultValue = getSpecifiedValue(aDefault.value()).get();

When intent to use null for default value like @Param("foo") @Default FooType foo, AnnotatedValueResolver doesn't check if Optional.isPresent but directly get underlying value, which will be ended up with No value present.

Expected is following:

final Optional<String> specifiedValue = getSpecifiedValue(aDefault.value());
defaultValue = specifiedValue.isPresent() ? specifiedValue.get() : null;
@trustin
Copy link
Collaborator

trustin commented Jun 26, 2019

Doh! Are you interested in sending a pull request, or shall I send one based on your suggestion?

@trustin trustin added the defect label Jun 26, 2019
@trustin trustin added this to the 0.88.0 milestone Jun 26, 2019
@southernkasaist
Copy link
Author

Seems that to write correct test cases, I need to take time to have comprehensive understanding of test structures.

So I appreciate if you can take it instead 🙇

@trustin
Copy link
Collaborator

trustin commented Jun 26, 2019

Sure. No problem!

trustin added a commit to trustin/armeria that referenced this issue Jun 28, 2019
@trustin
Handle the case where @Default annotation is specified without a value
a76457a 
Motivation:

A `NoSuchElementException` is raised during server startup time if a
user specified a `@Default` annotation without a value:

    public class MyAnnotatedService {
        @get("/foo")
        public String foo(@param @default String value) {
            return String.valueOf(value);
        }
    }

Modifications:

- Allow having no value in `@Default` annotation

Result:

- Fixes line#1858
@trustin trustin mentioned this issue Jun 28, 2019
Merged
@trustin
Copy link
Collaborator

trustin commented Jun 28, 2019

Will be fixed via #1864

trustin added a commit that referenced this issue Jun 28, 2019
@trustin
Handle the case where @Default annotation is specified withou… (#1864)
db94a79 
Motivation:

A `NoSuchElementException` is raised during server startup time if a
user specified a `@Default` annotation without a value:

    public class MyAnnotatedService {
        @get("/foo")
        public String foo(@param @default String value) {
            return String.valueOf(value);
        }
    }

Modifications:

- Allow having no value in `@Default` annotation

Result:

- Fixes #1858
fmguerreiro pushed a commit to fmguerreiro/armeria that referenced this issue Sep 19, 2020
@trustin
Handle the case where @Default annotation is specified withou… (lin…
ad2c892 
…e#1864)

Motivation:

A `NoSuchElementException` is raised during server startup time if a
user specified a `@Default` annotation without a value:

    public class MyAnnotatedService {
        @get("/foo")
        public String foo(@param @default String value) {
            return String.valueOf(value);
        }
    }

Modifications:

- Allow having no value in `@Default` annotation

Result:

- Fixes line#1858
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
defect
Projects
None yet
Milestone
0.88.0
Development

Successfully merging a pull request may close this issue.

Handle the case where @Default annotation is specified withou… trustin/armeria
2 participants
@trustin @southernkasaist