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

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

Merged
merged 17 commits into from Nov 12, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Expand Up @@ -96,18 +96,17 @@ public final <T> T newClient(Scheme scheme, Endpoint endpoint, @Nullable String
protected final Scheme validateScheme(URI uri) {
requireNonNull(uri, "uri");

final String scheme = uri.getScheme();
if (scheme == null) {
throw new IllegalArgumentException("URI with missing scheme: " + uri);
}

if (uri.getAuthority() == null) {
throw new IllegalArgumentException("URI with missing authority: " + uri);
}

final String scheme = uri.getScheme();
if (scheme == null) {
rmohta marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalArgumentException("URI with missing scheme: " + uri);
}
final Optional<Scheme> parsedSchemeOpt = Scheme.tryParse(scheme);
if (!parsedSchemeOpt.isPresent()) {
throw new IllegalArgumentException("URI with unknown scheme: " + uri);
throw new IllegalArgumentException("URI with undefined scheme: " + uri);
}

final Scheme parsedScheme = parsedSchemeOpt.get();
Expand Down
Expand Up @@ -21,6 +21,8 @@

import com.google.common.base.MoreObjects;

import com.linecorp.armeria.common.SerializationFormat;

/**
* Default {@link ClientBuilderParams} implementation.
*/
Expand All @@ -37,7 +39,7 @@ public class DefaultClientBuilderParams implements ClientBuilderParams {
public DefaultClientBuilderParams(ClientFactory factory, URI uri, Class<?> type,
ClientOptions options) {
this.factory = requireNonNull(factory, "factory");
this.uri = requireNonNull(uri, "uri");
this.uri = cleanUri(requireNonNull(uri, "uri"));
this.type = requireNonNull(type, "type");
this.options = requireNonNull(options, "options");
}
Expand Down Expand Up @@ -70,4 +72,14 @@ public String toString() {
.add("type", type)
.add("options", options).toString();
}

protected final URI cleanUri(final URI uri) {
rmohta marked this conversation as resolved.
Show resolved Hide resolved
if (!uri.getScheme().contains(SerializationFormat.NONE.uriText())) {
rmohta marked this conversation as resolved.
Show resolved Hide resolved
return uri;
}
final String newScheme = uri.getScheme()
.replace(SerializationFormat.NONE.uriText(), "")
.replace("+", "");
return URI.create(newScheme + uri.toString().substring(uri.getScheme().length()));
}
}
18 changes: 11 additions & 7 deletions core/src/main/java/com/linecorp/armeria/common/Scheme.java
Expand Up @@ -78,8 +78,13 @@ public static Optional<Scheme> tryParse(@Nullable String scheme) {
if (scheme == null) {
return Optional.empty();
}

return Optional.ofNullable(SCHEMES.get(Ascii.toLowerCase(scheme)));
final Optional<Scheme> parsedScheme = Optional.ofNullable(SCHEMES.get(Ascii.toLowerCase(scheme)));
if (parsedScheme.isPresent()) {
return parsedScheme;
}
return Optional.ofNullable(SCHEMES.get(Ascii.toLowerCase(SerializationFormat.NONE.uriText() +
'+' +
scheme)));
rmohta marked this conversation as resolved.
Show resolved Hide resolved
rmohta marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -90,12 +95,11 @@ public static Optional<Scheme> tryParse(@Nullable String scheme) {
* there is no such {@link Scheme} available
*/
public static Scheme parse(String scheme) {
final Scheme res = SCHEMES.get(Ascii.toLowerCase(requireNonNull(scheme, "scheme")));
if (res == null) {
throw new IllegalArgumentException("scheme: " + scheme);
final Optional<Scheme> parsedScheme = tryParse(requireNonNull(scheme, "scheme"));
if (parsedScheme.isPresent()) {
return parsedScheme.get();
}

return res;
throw new IllegalArgumentException("scheme: " + scheme);
rmohta marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
@@ -0,0 +1,43 @@
/*
* Copyright 2015 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.client;

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

import org.junit.jupiter.api.Test;

public class ClientBuilderTest {

@Test
public void nonePlusSchemeProvided() {
final HttpClient client = new ClientBuilder("none+https://google.com/").build(HttpClient.class);
assertThat(client.uri().toString()).isEqualTo("https://google.com/");
}

@Test
public void nonePlusSchemeUriToUrl() {
final HttpClient client = new ClientBuilder("none+https://google.com/").build(HttpClient.class);
assertThatCode(() -> client.uri().toURL()).doesNotThrowAnyException();
rmohta marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
public void noSchemeShouldDefaultToNone() {
final HttpClient client = new ClientBuilder("https://google.com/").build(HttpClient.class);
assertThat(client.uri().toString()).isEqualTo("https://google.com/");
}
}