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

Make 'none+' serialization format as optional. #2241

Merged
merged 17 commits into from Nov 12, 2019
Merged

Make 'none+' serialization format as optional. #2241

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
96079fb
Make 'none+' serialization format as optional. client.uri() will not …
rmohta Nov 8, 2019
f340b56
Prefer early return style
rmohta Nov 8, 2019
dcb6da3
Review comments
rmohta Nov 8, 2019
84b60a3
Checksyle error fixed
rmohta Nov 8, 2019
2f29fbb
Some more review comments. Moved URI normalization to DefaultClientFa…
rmohta Nov 8, 2019
a8e1b9e
Since the URI might be re-created in DefaultClientFactory, replaced =…
rmohta Nov 10, 2019
128d620
Rollback an unintended change
rmohta Nov 10, 2019
75d1350
Review comment. Removed final modifed from method args
rmohta Nov 10, 2019
2b6228a
Review comment for orElseThrow(). Also included a new test class.
rmohta Nov 11, 2019
18c92c2
Review comment. Test classes and methods should not be public, unless…
rmohta Nov 11, 2019
4fb2159
Review comment. UNDEFINED_URI is a special case, handle that.
rmohta Nov 11, 2019
02b480c
Review comment. Use isUndefined() method, hide the constant
rmohta Nov 11, 2019
54a4de5
Use static import, for readability
rmohta Nov 11, 2019
d6be96f
Review comments addressed.
rmohta Nov 11, 2019
865a3cc
Addressed review comments.
rmohta Nov 11, 2019
21ddfb6
Silly miss. Updated
rmohta Nov 11, 2019
1e29419
Review comment.
rmohta Nov 12, 2019
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
5 changes: 4 additions & 1 deletion core/src/main/java/com/linecorp/armeria/client/DefaultClientFactory.java
View file
Open in desktop
Expand Up @@ -178,9 +178,12 @@ void doClose() {

private URI normalizeUri(URI uri, Scheme scheme) {
if (isUndefinedUri(uri)) {
// See https://github.com/line/armeria/pull/2241 discussion for more details.
// We use a special singleton marker URI for clients that do not explicitly define a
// host or scheme at construction time.
// As this isn't created by users, we don't need to normalize it.
return uri;
}

if (scheme.serializationFormat() != SerializationFormat.NONE) {
rmohta marked this conversation as resolved.
Show resolved Hide resolved
return uri;
}
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/com/linecorp/armeria/common/Scheme.java
View file
Open in desktop
Expand Up @@ -79,9 +79,9 @@ public static Optional<Scheme> tryParse(@Nullable String scheme) {
return Optional.empty();
}
final String lowercaseScheme = Ascii.toLowerCase(scheme);
final Optional<Scheme> parsedScheme = Optional.ofNullable(SCHEMES.get(lowercaseScheme));
if (parsedScheme.isPresent()) {
return parsedScheme;
final Scheme parsedScheme = SCHEMES.get(lowercaseScheme);
if (parsedScheme != null) {
return Optional.of(parsedScheme);
}
return Optional.ofNullable(SCHEMES.get(SerializationFormat.NONE.uriText() + '+' + lowercaseScheme));
}
Expand Down
8 changes: 5 additions & 3 deletions core/src/test/java/com/linecorp/armeria/client/ClientBuilderTest.java
View file
Open in desktop
Expand Up @@ -17,7 +17,9 @@
package com.linecorp.armeria.client;

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

import java.net.MalformedURLException;
import java.net.URI;

import org.junit.jupiter.api.Test;

Expand All @@ -33,9 +35,9 @@ void nonePlusSchemeProvided() {
}

@Test
void nonePlusSchemeUriToUrl() {
void nonePlusSchemeUriToUrl() throws MalformedURLException {
final HttpClient client = new ClientBuilder("none+https://google.com/").build(HttpClient.class);
assertThatCode(() -> client.uri().toURL()).doesNotThrowAnyException();
assertThat(client.uri().toURL()).isEqualTo(URI.create("https://google.com/").toURL());
}

@Test
Expand Down
19 changes: 15 additions & 4 deletions core/src/test/java/com/linecorp/armeria/common/SchemeTest.java
View file
Open in desktop
Expand Up @@ -21,6 +21,7 @@

import java.util.Optional;

import org.assertj.core.api.Condition;
import org.junit.jupiter.api.Test;

/**
Expand All @@ -36,15 +37,25 @@ public void tryParse_null() {
@Test
void tryParse_add_none() {
final Optional<Scheme> got = Scheme.tryParse("http");
rmohta marked this conversation as resolved.
Show resolved Hide resolved
assertThat(got.get().serializationFormat()).isEqualTo(SerializationFormat.NONE);
assertThat(got.get().sessionProtocol()).isEqualTo(SessionProtocol.HTTP);
final Condition<Scheme> condition = new Condition<>(scheme ->
rmohta marked this conversation as resolved.
Show resolved Hide resolved
(scheme.sessionProtocol() ==
SessionProtocol.HTTP) &&
(scheme.serializationFormat() ==
SerializationFormat.NONE),
"none+http");
assertThat(got).hasValueSatisfying(condition);
}

@Test
void tryParse_with_none() {
final Optional<Scheme> got = Scheme.tryParse("http+none");
assertThat(got.get().serializationFormat()).isEqualTo(SerializationFormat.NONE);
assertThat(got.get().sessionProtocol()).isEqualTo(SessionProtocol.HTTP);
final Condition<Scheme> condition = new Condition<>(scheme ->
(scheme.sessionProtocol() ==
SessionProtocol.HTTP) &&
(scheme.serializationFormat() ==
SerializationFormat.NONE),
"none+http");
assertThat(got).hasValueSatisfying(condition);
}

@Test
Expand Down