Skip to content

Commit

Permalink
Reviewed JSON APIs. (#1510)
Browse files Browse the repository at this point in the history 
Removed obsolete methods, not necessary because blocking transports have been removed.
Changed signatures to return List<Message> rather than Message[], to avoid multiple conversions to List.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
@sbordet
sbordet committed Oct 16, 2023
1 parent 4d11ff5 commit 372fca0
Show file tree
Hide file tree
Showing 35 changed files with 190 additions and 278 deletions.
3 changes: 1 addition & 2 deletions .../cometd-java-client-common/src/main/java/org/cometd/client/transport/ClientTransport.java
View file
Expand Up @@ -16,7 +16,6 @@
package org.cometd.client.transport;

import java.text.ParseException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ScheduledExecutorService;
Expand Down Expand Up @@ -151,7 +150,7 @@ public void terminate() {
public abstract void send(TransportListener listener, List<Message.Mutable> messages);

protected List<Message.Mutable> parseMessages(String content) throws ParseException {
return new ArrayList<>(List.of(jsonContext.parse(content)));
return jsonContext.parse(content);
}

protected String generateJSON(List<Message.Mutable> messages) {
Expand Down
5 changes: 3 additions & 2 deletions ...p/cometd-java-client-http-tests/src/test/java/org/cometd/client/http/JSONContextTest.java
View file
Expand Up @@ -15,6 +15,7 @@
*/
package org.cometd.client.http;

import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -82,8 +83,8 @@ public void testAsyncParser(Class<JSONContextServer> jsonContextServerClass, Cla
byte[] bytes = json.getBytes(StandardCharsets.UTF_8);

for (int i = 0; i < bytes.length; ++i) {
clientParser.parse(bytes, i, 1);
serverParser.parse(bytes, i, 1);
clientParser.parse(ByteBuffer.wrap(bytes, i, 1));
serverParser.parse(ByteBuffer.wrap(bytes, i, 1));
}
List<Message.Mutable> clientMessages = clientParser.complete();
List<ServerMessage.Mutable> serverMessages = serverParser.complete();
Expand Down
10 changes: 5 additions & 5 deletions ...va-client-http-tests/src/test/java/org/cometd/client/http/MultipleClientSessionsTest.java
View file
Expand Up @@ -334,9 +334,9 @@ public void testMultipleClientSession_WhenSameClientSendsTwoConnects() throws Ex
Assertions.assertEquals(1, cookies.size());
HttpCookie browserCookie = cookies.get(0);
Assertions.assertEquals("BAYEUX_BROWSER", browserCookie.getName());
Message.Mutable[] messages = parser.parse(handshake.getContentAsString());
Assertions.assertEquals(1, messages.length);
String clientId = messages[0].getClientId();
List<Message.Mutable> messages = parser.parse(handshake.getContentAsString());
Assertions.assertEquals(1, messages.size());
String clientId = messages.get(0).getClientId();

String connectContent1 = "[{" +
"\"id\":\"2\"," +
Expand Down Expand Up @@ -408,8 +408,8 @@ public void testMultipleClientSession_WhenSameClientSendsTwoConnects() throws Ex
.send();
Assertions.assertEquals(200, connect4.getStatus());
messages = parser.parse(connect4.getContentAsString());
Assertions.assertEquals(1, messages.length);
Message.Mutable message = messages[0];
Assertions.assertEquals(1, messages.size());
Message.Mutable message = messages.get(0);
Map<String, Object> advice = message.getAdvice(true);
Assertions.assertFalse(advice.containsKey("multiple-clients"));
}
Expand Down
10 changes: 2 additions & 8 deletions cometd-java/cometd-java-common/src/main/java/org/cometd/common/BufferingJSONAsyncParser.java
View file
Expand Up @@ -17,7 +17,7 @@

import java.nio.ByteBuffer;
import java.text.ParseException;
import java.util.List;

import org.cometd.bayeux.Message;
import org.eclipse.jetty.util.Utf8StringBuilder;

Expand All @@ -35,11 +35,6 @@ public BufferingJSONAsyncParser(JSONContext<? extends Message.Mutable> jsonConte
this.jsonContext = jsonContext;
}

@Override
public void parse(byte[] bytes, int offset, int length) {
buffer.append(bytes, offset, length);
}

@Override
public void parse(ByteBuffer byteBuffer) {
buffer.append(byteBuffer);
Expand All @@ -51,8 +46,7 @@ public <R> R complete() {
try {
String json = buffer.toString();
buffer.reset();
Message.Mutable[] result = jsonContext.parse(json);
return (R)List.of(result);
return (R)jsonContext.parse(json);
} catch (ParseException x) {
throw new IllegalArgumentException(x);
}
Expand Down
28 changes: 1 addition & 27 deletions cometd-java/cometd-java-common/src/main/java/org/cometd/common/JSONContext.java
View file
Expand Up @@ -28,23 +28,14 @@
* @param <T> the type of message
*/
public interface JSONContext<T extends Message.Mutable> {
/**
* <p>Parses an array of messages from the given reader.</p>
*
* @param reader the reader to parse from
* @return an array of messages
* @throws ParseException in case of parsing errors
*/
public T[] parse(Reader reader) throws ParseException;

/**
* <p>Parses an array of messages from the given string.</p>
*
* @param json the JSON string to parse from
* @return an array of messages
* @throws ParseException in case of parsing errors
*/
public T[] parse(String json) throws ParseException;
public List<T> parse(String json) throws ParseException;

/**
* @return a new {@link AsyncParser} instance, or null if non-blocking parsing is not supported
Expand All @@ -61,14 +52,6 @@ public default AsyncParser newAsyncParser() {
*/
public String generate(T message);

/**
* <p>Converts a list of messages to a JSON string.</p>
*
* @param messages the list of messages to stringify
* @return the JSON string for the messages
*/
public String generate(List<T> messages);

/**
* @return a synchronous JSON parser to parse any JSON string
*/
Expand Down Expand Up @@ -105,15 +88,6 @@ public interface Parser {
* A non-blocking JSON parser.
*/
public interface AsyncParser {
/**
* @param bytes the bytes chunk to parse
* @param offset the offset to start parsing from
* @param length the number of bytes to parse
*/
public default void parse(byte[] bytes, int offset, int length) {
parse(ByteBuffer.wrap(bytes, offset, length));
}

/**
* @param buffer the buffer chunk to parse
*/
Expand Down
67 changes: 17 additions & 50 deletions cometd-java/cometd-java-common/src/main/java/org/cometd/common/JacksonJSONContext.java
View file
Expand Up @@ -20,41 +20,34 @@
import java.nio.ByteBuffer;
import java.text.ParseException;
import java.util.List;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.core.async.ByteArrayFeeder;
import com.fasterxml.jackson.core.async.ByteBufferFeeder;
import com.fasterxml.jackson.core.async.NonBlockingInputFeeder;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.type.CollectionType;
import com.fasterxml.jackson.databind.util.TokenBuffer;
import org.cometd.bayeux.Message;

public abstract class JacksonJSONContext<M extends Message.Mutable, I extends M> {
private final ObjectMapper objectMapper = new ObjectMapper();
private final JavaType rootArrayType;
private final CollectionType collectionType;

protected JacksonJSONContext() {
rootArrayType = objectMapper.constructType(rootArrayClass());
collectionType = objectMapper.getTypeFactory().constructCollectionType(List.class, messageClass());
}

public ObjectMapper getObjectMapper() {
return objectMapper;
}

protected abstract Class<I[]> rootArrayClass();
protected abstract Class<I> messageClass();

public M[] parse(Reader reader) throws ParseException {
public List<M> parse(String json) throws ParseException {
try {
return getObjectMapper().readValue(reader, rootArrayType);
} catch (IOException x) {
throw (ParseException)new ParseException("", -1).initCause(x);
}
}

public M[] parse(String json) throws ParseException {
try {
return getObjectMapper().readValue(json, rootArrayType);
return getObjectMapper().readValue(json, collectionType);
} catch (IOException x) {
throw (ParseException)new ParseException(json, -1).initCause(x);
}
Expand All @@ -77,16 +70,6 @@ public String generate(M message) {
}
}

public String generate(List<M> messages) {
try {
Message.Mutable[] mutables = new Message.Mutable[messages.size()];
messages.toArray(mutables);
return getObjectMapper().writeValueAsString(mutables);
} catch (IOException x) {
throw new RuntimeException(x);
}
}

public JSONContext.Parser getParser() {
return new ObjectMapperParser();
}
Expand Down Expand Up @@ -126,38 +109,24 @@ public AsyncJsonParser(JsonParser jsonParser) {
this.tokenBuffer = new TokenBuffer(jsonParser);
}

@Override
public void parse(byte[] bytes, int offset, int length) {
try {
NonBlockingInputFeeder feeder = jsonParser.getNonBlockingInputFeeder();
if (feeder instanceof ByteArrayFeeder) {
((ByteArrayFeeder)feeder).feedInput(bytes, offset, offset + length);
parseInput();
} else if (feeder instanceof ByteBufferFeeder) {
parse(ByteBuffer.wrap(bytes, offset, length));
} else {
throw new UnsupportedOperationException();
}
} catch (IOException x) {
throw new IllegalStateException(x);
}
}

@Override
public void parse(ByteBuffer buffer) {
try {
NonBlockingInputFeeder feeder = jsonParser.getNonBlockingInputFeeder();
if (feeder instanceof ByteBufferFeeder) {
((ByteBufferFeeder)feeder).feedInput(buffer);
NonBlockingInputFeeder input = jsonParser.getNonBlockingInputFeeder();
if (input instanceof ByteBufferFeeder feeder) {
feeder.feedInput(buffer);
parseInput();
} else if (feeder instanceof ByteArrayFeeder) {
} else if (input instanceof ByteArrayFeeder feeder) {
if (buffer.hasArray()) {
parse(buffer.array(), buffer.arrayOffset(), buffer.remaining());
int startIndex = buffer.arrayOffset() + buffer.position();
int endIndex = startIndex + buffer.remaining();
feeder.feedInput(buffer.array(), startIndex, endIndex);
} else {
byte[] bytes = new byte[buffer.remaining()];
buffer.get(bytes);
parse(bytes, 0, bytes.length);
feeder.feedInput(bytes, 0, bytes.length);
}
parseInput();
} else {
throw new UnsupportedOperationException();
}
Expand All @@ -176,15 +145,13 @@ private void parseInput() throws IOException {
}
}

@SuppressWarnings("unchecked")
@Override
public <R> R complete() {
try {
NonBlockingInputFeeder feeder = jsonParser.getNonBlockingInputFeeder();
feeder.endOfInput();
jsonParser.nextToken();
M[] result = objectMapper.readValue(tokenBuffer.asParser(), objectMapper.constructType(rootArrayClass()));
return (R)List.of(result);
return objectMapper.readValue(tokenBuffer.asParser(), collectionType);
} catch (IOException x) {
throw new IllegalArgumentException(x);
}
Expand Down
4 changes: 2 additions & 2 deletions cometd-java/cometd-java-common/src/main/java/org/cometd/common/JacksonJSONContextClient.java
View file
Expand Up @@ -19,7 +19,7 @@

public class JacksonJSONContextClient extends JacksonJSONContext<Message.Mutable, HashMapMessage> implements JSONContext.Client {
@Override
protected Class<HashMapMessage[]> rootArrayClass() {
return HashMapMessage[].class;
protected Class<HashMapMessage> messageClass() {
return HashMapMessage.class;
}
}
39 changes: 11 additions & 28 deletions cometd-java/cometd-java-common/src/main/java/org/cometd/common/JettyJSONContext.java
View file
Expand Up @@ -42,20 +42,9 @@ public AsyncJSON.Factory getAsyncJSONFactory() {
return _jsonFactory;
}

protected abstract M newRoot();
protected abstract M newMessage();

protected abstract M[] newRootArray(int size);

public M[] parse(Reader reader) throws ParseException {
try {
Object object = _messagesParser.parse(new JSON.ReaderSource(reader));
return adapt(object);
} catch (Exception x) {
throw (ParseException)new ParseException("", -1).initCause(x);
}
}

public M[] parse(String json) throws ParseException {
public List<M> parse(String json) throws ParseException {
try {
Object object = _messagesParser.parse(new JSON.StringSource(json));
return adapt(object);
Expand All @@ -70,29 +59,23 @@ public JSONContext.AsyncParser newAsyncParser() {
}

@SuppressWarnings("unchecked")
private M[] adapt(Object object) {
private List<M> adapt(Object object) {
if (object == null) {
return null;
}
if (object.getClass().isArray()) {
return (M[])object;
}
if (object instanceof List list) {
return (M[])list.toArray(this::newRootArray);
return list;
}
if (object.getClass().isArray()) {
return List.of((M[])object);
}
M[] result = newRootArray(1);
result[0] = (M)object;
return result;
return List.of((M)object);
}

public String generate(M message) {
return _messageParser.toJSON(message);
}

public String generate(List<M> messages) {
return _messagesParser.toJSON(messages);
}

public JSONContext.Parser getParser() {
return new JSONParser();
}
Expand Down Expand Up @@ -130,7 +113,7 @@ public void addConvertorFor(String name, Convertor convertor) {
private class MessageJSON extends FieldJSON {
@Override
protected Map<String, Object> newMap() {
return newRoot();
return newMessage();
}

@Override
Expand All @@ -151,7 +134,7 @@ public MessagesJSON() {

@Override
protected Map<String, Object> newMap() {
return newRoot();
return newMessage();
}

@Override
Expand Down Expand Up @@ -239,7 +222,7 @@ public AsyncJSON newAsyncJSON() {
@Override
protected Map<String, Object> newObject(Context context) {
if (context.depth() <= 1) {
return newRoot();
return newMessage();
}
return super.newObject(context);
}
Expand Down
7 changes: 1 addition & 6 deletions cometd-java/cometd-java-common/src/main/java/org/cometd/common/JettyJSONContextClient.java
View file
Expand Up @@ -19,12 +19,7 @@

public class JettyJSONContextClient extends JettyJSONContext<Message.Mutable> implements JSONContext.Client {
@Override
protected Message.Mutable newRoot() {
protected Message.Mutable newMessage() {
return new HashMapMessage();
}

@Override
protected Message.Mutable[] newRootArray(int size) {
return new Message.Mutable[size];
}
}

0 comments on commit 372fca0

Please sign in to comment.