Skip to content

Commit 43b8da2

Browse files
ScottmitchScottmitch
committed
review comments, reduce StringSerializer public API surface
1 parent 7d2c0ab commit 43b8da2

File tree

11 files changed

+119
-145
lines changed

11 files changed

+119
-145
lines changed

‎servicetalk-http-api/src/main/java/io/servicetalk/http/api/FormUrlEncodedSerializer.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.net.URLDecoder;
2525
import java.net.URLEncoder;
2626
import java.nio.charset.Charset;
27+
import java.util.Collection;
2728
import java.util.HashMap;
2829
import java.util.List;
2930
import java.util.Map;
@@ -32,13 +33,18 @@
3233

3334
import static io.servicetalk.http.api.DefaultHttpRequestMetaData.DEFAULT_MAX_QUERY_PARAMS;
3435
import static io.servicetalk.http.api.UriUtils.decodeQueryParams;
36+
import static java.nio.charset.Charset.availableCharsets;
3537
import static java.util.Collections.emptyMap;
3638

3739
final class FormUrlEncodedSerializer implements SerializerDeserializer<Map<String, List<String>>> {
38-
private static final HashMap<Charset, byte[]> CONTINUATIONS_SEPARATORS = new HashMap<>();
39-
private static final HashMap<Charset, byte[]> KEYVALUE_SEPARATORS = new HashMap<>();
40+
private static final HashMap<Charset, byte[]> CONTINUATIONS_SEPARATORS;
41+
private static final HashMap<Charset, byte[]> KEYVALUE_SEPARATORS;
4042
static {
41-
for (Charset charset : Charset.availableCharsets().values()) {
43+
Collection<Charset> charsets = availableCharsets().values();
44+
final int size = charsets.size();
45+
CONTINUATIONS_SEPARATORS = new HashMap<>(size);
46+
KEYVALUE_SEPARATORS = new HashMap<>(size);
47+
for (Charset charset : charsets) {
4248
final byte[] continuation;
4349
final byte[] keyvalue;
4450
try {

‎servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpHeaderValues.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,6 @@ public final class HttpHeaderValues {
161161
* {@code "text/plain"}
162162
*/
163163
public static final CharSequence TEXT_PLAIN_UTF_8 = newAsciiString("text/plain; charset=UTF-8");
164-
165164
/**
166165
* {@code "trailers"}
167166
*/

‎servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpSerializers.java

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@
1818
import io.servicetalk.serializer.api.SerializerDeserializer;
1919
import io.servicetalk.serializer.api.StreamingSerializerDeserializer;
2020
import io.servicetalk.serializer.utils.FixedLengthStreamingSerializer;
21-
import io.servicetalk.serializer.utils.StringAsciiSerializer;
22-
import io.servicetalk.serializer.utils.StringCharsetSerializer;
23-
import io.servicetalk.serializer.utils.StringUtf8Serializer;
2421
import io.servicetalk.serializer.utils.VarIntLengthStreamingSerializer;
2522

2623
import java.nio.charset.Charset;
@@ -39,6 +36,7 @@
3936
import static io.servicetalk.http.api.HttpHeaderValues.TEXT_PLAIN;
4037
import static io.servicetalk.http.api.HttpHeaderValues.TEXT_PLAIN_US_ASCII;
4138
import static io.servicetalk.http.api.HttpHeaderValues.TEXT_PLAIN_UTF_8;
39+
import static io.servicetalk.serializer.utils.StringSerializer.stringSerializer;
4240
import static java.nio.charset.StandardCharsets.US_ASCII;
4341
import static java.nio.charset.StandardCharsets.UTF_8;
4442

@@ -62,30 +60,30 @@ public final class HttpSerializers {
6260
headers -> headers.set(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED_UTF_8),
6361
headers -> hasContentType(headers, APPLICATION_X_WWW_FORM_URLENCODED, UTF_8));
6462
private static final HttpSerializerDeserializer<String> TEXT_UTF_8 =
65-
new DefaultHttpSerializerDeserializer<>(StringUtf8Serializer.INSTANCE,
63+
new DefaultHttpSerializerDeserializer<>(stringSerializer(UTF_8),
6664
headers -> headers.set(CONTENT_TYPE, TEXT_PLAIN_UTF_8),
6765
headers -> hasContentType(headers, TEXT_PLAIN, UTF_8));
6866
private static final HttpSerializerDeserializer<String> TEXT_ASCII =
69-
new DefaultHttpSerializerDeserializer<>(StringAsciiSerializer.INSTANCE,
67+
new DefaultHttpSerializerDeserializer<>(stringSerializer(US_ASCII),
7068
headers -> headers.set(CONTENT_TYPE, TEXT_PLAIN_US_ASCII),
7169
headers -> hasContentType(headers, TEXT_PLAIN, US_ASCII));
7270
private static final int MAX_BYTES_PER_CHAR_UTF8 = (int) UTF_8.newEncoder().maxBytesPerChar();
7371
private static final HttpStreamingSerializerDeserializer<String> TEXT_STREAMING_FIX_LEN_UTF_8 =
74-
streamingSerializer(new FixedLengthStreamingSerializer<>(StringUtf8Serializer.INSTANCE,
72+
streamingSerializer(new FixedLengthStreamingSerializer<>(stringSerializer(UTF_8),
7573
str -> str.length() * MAX_BYTES_PER_CHAR_UTF8),
7674
headers -> headers.set(CONTENT_TYPE, APPLICATION_TEXT_FIXED_UTF_8),
7775
headers -> hasContentType(headers, APPLICATION_TEXT_FIXED, UTF_8));
7876
private static final HttpStreamingSerializerDeserializer<String> TEXT_STREAMING_FIX_LEN_ASCII =
79-
streamingSerializer(new FixedLengthStreamingSerializer<>(StringAsciiSerializer.INSTANCE, String::length),
77+
streamingSerializer(new FixedLengthStreamingSerializer<>(stringSerializer(US_ASCII), String::length),
8078
headers -> headers.set(CONTENT_TYPE, APPLICATION_TEXT_FIXED_US_ASCII),
8179
headers -> hasContentType(headers, APPLICATION_TEXT_FIXED, US_ASCII));
8280
private static final HttpStreamingSerializerDeserializer<String> TEXT_STREAMING_VAR_LEN_UTF_8 =
83-
streamingSerializer(new VarIntLengthStreamingSerializer<>(StringUtf8Serializer.INSTANCE,
81+
streamingSerializer(new VarIntLengthStreamingSerializer<>(stringSerializer(UTF_8),
8482
str -> str.length() * MAX_BYTES_PER_CHAR_UTF8),
8583
headers -> headers.set(CONTENT_TYPE, APPLICATION_TEXT_VAR_INT_UTF_8),
8684
headers -> hasContentType(headers, APPLICATION_TEXT_VARINT, UTF_8));
8785
private static final HttpStreamingSerializerDeserializer<String> TEXT_STREAMING_VAR_LEN_ASCII =
88-
streamingSerializer(new VarIntLengthStreamingSerializer<>(StringAsciiSerializer.INSTANCE, String::length),
86+
streamingSerializer(new VarIntLengthStreamingSerializer<>(stringSerializer(US_ASCII), String::length),
8987
headers -> headers.set(CONTENT_TYPE, APPLICATION_TEXT_VAR_INT_US_ASCII),
9088
headers -> hasContentType(headers, APPLICATION_TEXT_VARINT, US_ASCII));
9189

@@ -114,7 +112,7 @@ public static HttpSerializerDeserializer<Map<String, List<String>>> formUrlEncod
114112
* href="https://url.spec.whatwg.org/#application/x-www-form-urlencoded">x-www-form-urlencoded specification</a>
115113
*/
116114
public static HttpSerializerDeserializer<Map<String, List<String>>> formUrlEncodedSerializer(Charset charset) {
117-
if (charset == UTF_8) {
115+
if (UTF_8.equals(charset)) {
118116
return FORM_ENCODED_UTF_8;
119117
}
120118
final CharSequence contentType = newAsciiString(APPLICATION_X_WWW_FORM_URLENCODED + "; charset=" +
@@ -152,13 +150,13 @@ public static HttpSerializerDeserializer<String> textSerializerAscii() {
152150
* @return {@link HttpSerializerDeserializer} that can serialize {@link String}s.
153151
*/
154152
public static HttpSerializerDeserializer<String> textSerializer(Charset charset) {
155-
if (charset == UTF_8) {
153+
if (UTF_8.equals(charset)) {
156154
return TEXT_UTF_8;
157-
} else if (charset == US_ASCII) {
155+
} else if (US_ASCII.equals(charset)) {
158156
return TEXT_ASCII;
159157
}
160158
final CharSequence contentType = newAsciiString("text/plain; charset=" + charset.name());
161-
return new DefaultHttpSerializerDeserializer<>(new StringCharsetSerializer(charset),
159+
return new DefaultHttpSerializerDeserializer<>(stringSerializer(charset),
162160
headers -> headers.set(CONTENT_TYPE, contentType),
163161
headers -> hasContentType(headers, TEXT_PLAIN, charset));
164162
}
@@ -221,14 +219,14 @@ public static HttpStreamingSerializerDeserializer<String> textSerializerAsciiVar
221219
* @see FixedLengthStreamingSerializer
222220
*/
223221
public static HttpStreamingSerializerDeserializer<String> textSerializerFixLen(Charset charset) {
224-
if (charset == UTF_8) {
222+
if (UTF_8.equals(charset)) {
225223
return TEXT_STREAMING_FIX_LEN_UTF_8;
226-
} else if (charset == US_ASCII) {
224+
} else if (US_ASCII.equals(charset)) {
227225
return TEXT_STREAMING_FIX_LEN_ASCII;
228226
}
229227
final int maxBytesPerChar = (int) charset.newEncoder().maxBytesPerChar();
230228
CharSequence contentType = newAsciiString(APPLICATION_TEXT_FIXED + "; charset=" + charset.name());
231-
return streamingSerializer(new FixedLengthStreamingSerializer<>(new StringCharsetSerializer(charset),
229+
return streamingSerializer(new FixedLengthStreamingSerializer<>(stringSerializer(charset),
232230
str -> str.length() * maxBytesPerChar),
233231
headers -> headers.set(CONTENT_TYPE, contentType),
234232
headers -> hasContentType(headers, APPLICATION_TEXT_FIXED, charset));
@@ -244,14 +242,14 @@ public static HttpStreamingSerializerDeserializer<String> textSerializerFixLen(C
244242
* @see VarIntLengthStreamingSerializer
245243
*/
246244
public static HttpStreamingSerializerDeserializer<String> textSerializerVarLen(Charset charset) {
247-
if (charset == UTF_8) {
245+
if (UTF_8.equals(charset)) {
248246
return TEXT_STREAMING_VAR_LEN_UTF_8;
249-
} else if (charset == US_ASCII) {
247+
} else if (US_ASCII.equals(charset)) {
250248
return TEXT_STREAMING_VAR_LEN_ASCII;
251249
}
252250
final int maxBytesPerChar = (int) charset.newEncoder().maxBytesPerChar();
253251
CharSequence contentType = newAsciiString(APPLICATION_TEXT_VARINT + "; charset=" + charset.name());
254-
return streamingSerializer(new VarIntLengthStreamingSerializer<>(new StringCharsetSerializer(charset),
252+
return streamingSerializer(new VarIntLengthStreamingSerializer<>(stringSerializer(charset),
255253
str -> str.length() * maxBytesPerChar),
256254
headers -> headers.set(CONTENT_TYPE, contentType),
257255
headers -> hasContentType(headers, APPLICATION_TEXT_VARINT, charset));

‎servicetalk-http-api/src/test/java/io/servicetalk/http/api/StreamingHttpPayloadHolderTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import io.servicetalk.concurrent.test.internal.TestPublisherSubscriber;
2323
import io.servicetalk.serializer.api.StreamingSerializerDeserializer;
2424
import io.servicetalk.serializer.utils.FixedLengthStreamingSerializer;
25-
import io.servicetalk.serializer.utils.StringUtf8Serializer;
2625

2726
import org.junit.jupiter.api.AfterEach;
2827
import org.junit.jupiter.params.ParameterizedTest;
@@ -44,6 +43,7 @@
4443
import static io.servicetalk.http.api.HttpHeaderValues.CHUNKED;
4544
import static io.servicetalk.http.api.HttpProtocolVersion.HTTP_1_1;
4645
import static io.servicetalk.http.api.HttpSerializers.textSerializerUtf8FixLen;
46+
import static io.servicetalk.serializer.utils.StringSerializer.stringSerializer;
4747
import static java.nio.charset.StandardCharsets.UTF_8;
4848
import static java.util.Collections.emptyIterator;
4949
import static java.util.Collections.singletonList;
@@ -79,7 +79,7 @@ private enum SourceType {
7979
}
8080

8181
private static final StreamingSerializerDeserializer<String> UTF8_DESERIALIZER =
82-
new FixedLengthStreamingSerializer<>(StringUtf8Serializer.INSTANCE, String::length);
82+
new FixedLengthStreamingSerializer<>(stringSerializer(UTF_8), String::length);
8383
private HttpHeaders headers;
8484
private HttpHeadersFactory headersFactory;
8585

‎servicetalk-serializer-utils/src/main/java/io/servicetalk/serializer/utils/StringCharsetSerializer.java renamed to ‎servicetalk-serializer-utils/src/main/java/io/servicetalk/serializer/utils/AbstractStringSerializer.java

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,23 @@
2020
import io.servicetalk.serializer.api.SerializerDeserializer;
2121

2222
import java.nio.charset.Charset;
23+
import java.util.Collection;
2324
import java.util.HashMap;
2425
import java.util.Map;
2526

26-
/**
27-
* Serialize/deserialize {@link String}s encoded with a {@link Charset}.
28-
* @see StringAsciiSerializer
29-
* @see StringUtf8Serializer
30-
*/
31-
public final class StringCharsetSerializer implements SerializerDeserializer<String> {
32-
private static final Map<Charset, Integer> MAX_BYTES_PER_CHAR_MAP = new HashMap<>();
27+
import static java.nio.charset.Charset.availableCharsets;
28+
29+
abstract class AbstractStringSerializer implements SerializerDeserializer<String> {
30+
private static final Map<Charset, Integer> MAX_BYTES_PER_CHAR_MAP;
3331
static {
34-
for (Charset charset : Charset.availableCharsets().values()) {
35-
MAX_BYTES_PER_CHAR_MAP.put(charset, (int) charset.newEncoder().maxBytesPerChar());
32+
Collection<Charset> charsets = availableCharsets().values();
33+
MAX_BYTES_PER_CHAR_MAP = new HashMap<>(charsets.size());
34+
for (Charset charset : charsets) {
35+
try {
36+
MAX_BYTES_PER_CHAR_MAP.put(charset, (int) charset.newEncoder().maxBytesPerChar());
37+
} catch (Throwable ignored) {
38+
// ignored
39+
}
3640
}
3741
}
3842

@@ -43,20 +47,20 @@ public final class StringCharsetSerializer implements SerializerDeserializer<Str
4347
* Create a new instance.
4448
* @param charset The charset used for encoding.
4549
*/
46-
public StringCharsetSerializer(final Charset charset) {
50+
AbstractStringSerializer(final Charset charset) {
4751
this.charset = charset;
4852
maxBytesPerChar = MAX_BYTES_PER_CHAR_MAP.getOrDefault(charset, 1);
4953
}
5054

5155
@Override
52-
public String deserialize(final Buffer serializedData, final BufferAllocator allocator) {
56+
public final String deserialize(final Buffer serializedData, final BufferAllocator allocator) {
5357
String result = serializedData.toString(charset);
5458
serializedData.skipBytes(serializedData.readableBytes());
5559
return result;
5660
}
5761

5862
@Override
59-
public Buffer serialize(String toSerialize, BufferAllocator allocator) {
63+
public final Buffer serialize(String toSerialize, BufferAllocator allocator) {
6064
Buffer buffer = allocator.newBuffer(toSerialize.length() * maxBytesPerChar);
6165
serialize(toSerialize, allocator, buffer);
6266
return buffer;

‎servicetalk-serializer-utils/src/main/java/io/servicetalk/serializer/utils/FixedLengthStreamingSerializer.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.function.ToIntFunction;
2727
import javax.annotation.Nullable;
2828

29+
import static java.lang.Integer.BYTES;
2930
import static java.util.Objects.requireNonNull;
3031
import static java.util.function.Function.identity;
3132

@@ -58,9 +59,9 @@ public Publisher<T> deserialize(final Publisher<Buffer> serializedData, final Bu
5859
@Override
5960
public Publisher<Buffer> serialize(final Publisher<T> toSerialize, final BufferAllocator allocator) {
6061
return toSerialize.map(t -> {
61-
Buffer buffer = allocator.newBuffer(4 + bytesEstimator.applyAsInt(t));
62+
Buffer buffer = allocator.newBuffer(BYTES + bytesEstimator.applyAsInt(t));
6263
final int beforeWriterIndex = buffer.writerIndex();
63-
buffer.writerIndex(beforeWriterIndex + 4);
64+
buffer.writerIndex(beforeWriterIndex + BYTES);
6465
serializer.serialize(t, allocator, buffer);
6566
buffer.setInt(beforeWriterIndex, buffer.writerIndex() - beforeWriterIndex - 4);
6667
return buffer;
@@ -74,7 +75,7 @@ private static final class LengthDeframer implements BiFunction<Buffer, BufferAl
7475
@Override
7576
public Buffer apply(final Buffer buffer, final BufferAllocator allocator) {
7677
if (expectedLength < 0) {
77-
if (buffer.readableBytes() < 4) {
78+
if (buffer.readableBytes() < BYTES) {
7879
return null;
7980
}
8081
expectedLength = buffer.readInt();

‎servicetalk-serializer-utils/src/main/java/io/servicetalk/serializer/utils/StringAsciiSerializer.java

Lines changed: 0 additions & 51 deletions
This file was deleted.

‎servicetalk-serializer-utils/src/main/java/io/servicetalk/serializer/utils/StringSerializer.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright © 2021 Apple Inc. and the ServiceTalk project authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.servicetalk.serializer.utils;
17+
18+
import io.servicetalk.buffer.api.Buffer;
19+
import io.servicetalk.buffer.api.BufferAllocator;
20+
import io.servicetalk.serializer.api.SerializerDeserializer;
21+
22+
import java.nio.charset.Charset;
23+
24+
import static java.nio.charset.StandardCharsets.US_ASCII;
25+
import static java.nio.charset.StandardCharsets.UTF_8;
26+
27+
/**
28+
* Serialize/deserialize {@link String}s encoded with a {@link Charset}.
29+
*/
30+
public final class StringSerializer extends AbstractStringSerializer {
31+
private static final SerializerDeserializer<String> UTF_8_SERIALIZER = new AbstractStringSerializer(UTF_8) {
32+
@Override
33+
public void serialize(final String toSerialize, final BufferAllocator allocator, final Buffer buffer) {
34+
buffer.writeUtf8(toSerialize);
35+
}
36+
};
37+
private static final SerializerDeserializer<String> US_ASCII_SERIALIZER = new AbstractStringSerializer(US_ASCII) {
38+
@Override
39+
public void serialize(final String toSerialize, final BufferAllocator allocator, final Buffer buffer) {
40+
buffer.writeAscii(toSerialize);
41+
}
42+
};
43+
44+
/**
45+
* Create a new instance.
46+
* @param charset The charset used for encoding.
47+
*/
48+
private StringSerializer(final Charset charset) {
49+
super(charset);
50+
}
51+
52+
/**
53+
* Create a new instance.
54+
* @param charset The charset used for encoding.
55+
* @return A serializer that uses {@code charset} for encoding.
56+
*/
57+
public static SerializerDeserializer<String> stringSerializer(final Charset charset) {
58+
if (UTF_8.equals(charset)) {
59+
return UTF_8_SERIALIZER;
60+
} else if (US_ASCII.equals(charset)) {
61+
return US_ASCII_SERIALIZER;
62+
}
63+
return new StringSerializer(charset);
64+
}
65+
}

0 commit comments

Comments
 (0)