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

Use configured gson instance for toString #772

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ Fixed issues: <https://github.com/eclipse-lsp4j/lsp4j/milestone/29?closed=1>

Breaking API changes:

* The Message class now has a new transient field, `jsonHandler` to enable the `toString` implementation to properly format messages when custom type adapters are in used.
jonahgraham marked this conversation as resolved.
Show resolved Hide resolved
* For consumers that have their own custom serializers they may need to be updated to ensure that transient fields are skipped, for example by using [`Modifier.isTransient`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/reflect/Modifier.html#isTransient(int))
jonahgraham marked this conversation as resolved.
Show resolved Hide resolved
* See [#768](https://github.com/eclipse-lsp4j/lsp4j/issues/768) for detailed discussion.
* The name field in WorkspaceFolder is no longer optional according to the specification.
* See [#741](https://github.com/eclipse-lsp4j/lsp4j/issues/741) for detailed discussion.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ protected RemoteEndpoint createRemoteEndpoint(MessageJsonHandler jsonHandler) {
else
remoteEndpoint = new DebugRemoteEndpoint(outgoingMessageStream, localEndpoint, exceptionHandler);
jsonHandler.setMethodProvider(remoteEndpoint);
remoteEndpoint.setJsonHandler(jsonHandler);
return remoteEndpoint;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public DebugRemoteEndpoint(MessageConsumer out, Endpoint localEndpoint,
@Override
protected DebugRequestMessage createRequestMessage(String method, Object parameter) {
DebugRequestMessage requestMessage = new DebugRequestMessage();
requestMessage.setJsonHandler(getJsonHandler());
requestMessage.setId(nextSeqId.incrementAndGet());
requestMessage.setMethod(method);
requestMessage.setParams(parameter);
Expand All @@ -47,6 +48,7 @@ protected DebugRequestMessage createRequestMessage(String method, Object paramet
@Override
protected DebugResponseMessage createResponseMessage(RequestMessage requestMessage) {
DebugResponseMessage responseMessage = new DebugResponseMessage();
responseMessage.setJsonHandler(getJsonHandler());
responseMessage.setResponseId(nextSeqId.incrementAndGet());
responseMessage.setRawId(requestMessage.getRawId());
responseMessage.setMethod(requestMessage.getMethod());
Expand All @@ -56,6 +58,7 @@ protected DebugResponseMessage createResponseMessage(RequestMessage requestMessa
@Override
protected DebugNotificationMessage createNotificationMessage(String method, Object parameter) {
DebugNotificationMessage notificationMessage = new DebugNotificationMessage();
notificationMessage.setJsonHandler(getJsonHandler());
notificationMessage.setId(nextSeqId.incrementAndGet());
notificationMessage.setMethod(method);
notificationMessage.setParams(parameter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,20 +343,23 @@ private Message createMessage(String messageType, int seq, int request_seq, Stri
switch (messageType) {
case "request": {
DebugRequestMessage message = new DebugRequestMessage();
message.setJsonHandler(handler);
message.setId(seq);
message.setMethod(method);
message.setParams(params);
return message;
}
case "event": {
DebugNotificationMessage message = new DebugNotificationMessage();
message.setJsonHandler(handler);
message.setId(seq);
message.setMethod(method);
message.setParams(body);
return message;
}
case "response": {
DebugResponseMessage message = new DebugResponseMessage();
message.setJsonHandler(handler);
message.setId(request_seq);
message.setResponseId(seq);
message.setMethod(method);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,9 @@ public Launcher<T> create() {

// Create the JSON handler, remote endpoint and remote proxy
MessageJsonHandler jsonHandler = createJsonHandler();
if (messageTracer != null) {
messageTracer.setJsonHandler(jsonHandler);
}
pisv marked this conversation as resolved.
Show resolved Hide resolved
RemoteEndpoint remoteEndpoint = createRemoteEndpoint(jsonHandler);
T remoteProxy = createProxy(remoteEndpoint);

Expand Down Expand Up @@ -352,6 +355,7 @@ protected RemoteEndpoint createRemoteEndpoint(MessageJsonHandler jsonHandler) {
else
remoteEndpoint = new RemoteEndpoint(outgoingMessageStream, localEndpoint, exceptionHandler);
jsonHandler.setMethodProvider(remoteEndpoint);
remoteEndpoint.setJsonHandler(jsonHandler);
return remoteEndpoint;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@
******************************************************************************/
package org.eclipse.lsp4j.jsonrpc;

import org.eclipse.lsp4j.jsonrpc.TracingMessageConsumer.RequestMetadata;

import java.io.PrintWriter;
import java.time.Clock;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;

import org.eclipse.lsp4j.jsonrpc.TracingMessageConsumer.RequestMetadata;
import org.eclipse.lsp4j.jsonrpc.json.MessageJsonHandler;

/**
* Wraps a {@link MessageConsumer} with one that logs in a way that the LSP Inspector can parse. *
* https://microsoft.github.io/language-server-protocol/inspector/
Expand All @@ -28,14 +29,21 @@ public class MessageTracer implements Function<MessageConsumer, MessageConsumer>
private final PrintWriter printWriter;
private final Map<String, RequestMetadata> sentRequests = new HashMap<>();
private final Map<String, RequestMetadata> receivedRequests = new HashMap<>();
private MessageJsonHandler jsonHandler;

MessageTracer(PrintWriter printWriter) {
this.printWriter = Objects.requireNonNull(printWriter);
}

public void setJsonHandler(MessageJsonHandler jsonHandler) {
this.jsonHandler = jsonHandler;
}

@Override
public MessageConsumer apply(MessageConsumer messageConsumer) {
return new TracingMessageConsumer(
TracingMessageConsumer tracingMessageConsumer = new TracingMessageConsumer(
messageConsumer, sentRequests, receivedRequests, printWriter, Clock.systemDefaultZone());
tracingMessageConsumer.setJsonHandler(jsonHandler);
return tracingMessageConsumer;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ private static ResponseError fallbackResponseError(String header, Throwable thro
private final MessageConsumer out;
private final Endpoint localEndpoint;
private final Function<Throwable, ResponseError> exceptionHandler;
private MessageJsonHandler jsonHandler;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that RemoteEndpoint now has the reference to a MessageJsonHandler to be able to inject the handler into the messages it creates.


private final AtomicInteger nextRequestId = new AtomicInteger();
private final Map<String, PendingRequestInfo> sentRequestMap = new LinkedHashMap<>();
Expand Down Expand Up @@ -116,6 +117,14 @@ public RemoteEndpoint(MessageConsumer out, Endpoint localEndpoint) {
this(out, localEndpoint, DEFAULT_EXCEPTION_HANDLER);
}

public void setJsonHandler(MessageJsonHandler jsonHandler) {
this.jsonHandler = jsonHandler;
}

public MessageJsonHandler getJsonHandler() {
return jsonHandler;
}

/**
* Send a notification to the remote endpoint.
*/
Expand All @@ -132,6 +141,7 @@ public void notify(String method, Object parameter) {

protected NotificationMessage createNotificationMessage(String method, Object parameter) {
NotificationMessage notificationMessage = new NotificationMessage();
notificationMessage.setJsonHandler(getJsonHandler());
notificationMessage.setJsonrpc(MessageConstants.JSONRPC_VERSION);
notificationMessage.setMethod(method);
notificationMessage.setParams(parameter);
Expand Down Expand Up @@ -168,6 +178,7 @@ public boolean cancel(boolean mayInterruptIfRunning) {

protected RequestMessage createRequestMessage(String method, Object parameter) {
RequestMessage requestMessage = new RequestMessage();
requestMessage.setJsonHandler(getJsonHandler());
requestMessage.setId(String.valueOf(nextRequestId.incrementAndGet()));
requestMessage.setMethod(method);
requestMessage.setParams(parameter);
Expand Down Expand Up @@ -361,6 +372,7 @@ protected void handleResponseIssues(ResponseMessage responseMessage, List<Messag

protected ResponseMessage createResponseMessage(RequestMessage requestMessage) {
ResponseMessage responseMessage = new ResponseMessage();
responseMessage.setJsonHandler(getJsonHandler());
responseMessage.setRawId(requestMessage.getRawId());
responseMessage.setJsonrpc(MessageConstants.JSONRPC_VERSION);
return responseMessage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class TracingMessageConsumer implements MessageConsumer {
private final PrintWriter printWriter;
private final Clock clock;
private final DateTimeFormatter dateTimeFormatter;
private MessageJsonHandler jsonHandler;

/**
* @param messageConsumer The {@link MessageConsumer} to wrap.
Expand Down Expand Up @@ -86,6 +87,10 @@ public TracingMessageConsumer(
}
}

public void setJsonHandler(MessageJsonHandler jsonHandler) {
this.jsonHandler = jsonHandler;
}

/**
* Constructs a log string for a given {@link Message}. The type of the {@link MessageConsumer}
* determines if we're sending or receiving a message. The type of the @{link Message} determines
Expand Down Expand Up @@ -122,33 +127,33 @@ private String consumeMessageSending(Message message, Instant now, String date)
RequestMetadata requestMetadata = new RequestMetadata(method, now);
sentRequests.put(id, requestMetadata);
Object params = requestMessage.getParams();
String paramsJson = MessageJsonHandler.toString(params);
String paramsJson = toString(params);
String format = "[Trace - %s] Sending request '%s - (%s)'\nParams: %s\n\n\n";
return String.format(format, date, method, id, paramsJson);
} else if (message instanceof ResponseMessage) {
ResponseMessage responseMessage = (ResponseMessage) message;
String id = responseMessage.getId();
RequestMetadata requestMetadata = receivedRequests.remove(id);
if (requestMetadata == null) {
LOG.log(WARNING, String.format("Unmatched response message: %s", message));
LOG.log(WARNING, String.format("Unmatched response message: %s", toString(message)));
return null;
}
String method = requestMetadata.method;
long latencyMillis = now.toEpochMilli() - requestMetadata.start.toEpochMilli();
Object result = responseMessage.getResult();
String resultJson = MessageJsonHandler.toString(result);
String resultJson = toString(result);
String format =
"[Trace - %s] Sending response '%s - (%s)'. Processing request took %sms\nResult: %s\n\n\n";
return String.format(format, date, method, id, latencyMillis, resultJson);
} else if (message instanceof NotificationMessage) {
NotificationMessage notificationMessage = (NotificationMessage) message;
String method = notificationMessage.getMethod();
Object params = notificationMessage.getParams();
String paramsJson = MessageJsonHandler.toString(params);
String paramsJson = toString(params);
String format = "[Trace - %s] Sending notification '%s'\nParams: %s\n\n\n";
return String.format(format, date, method, paramsJson);
} else {
LOG.log(WARNING, String.format("Unknown message type: %s", message));
LOG.log(WARNING, String.format("Unknown message type: %s", toString(message)));
return null;
}
}
Expand All @@ -161,38 +166,42 @@ private String consumeMessageReceiving(Message message, Instant now, String date
RequestMetadata requestMetadata = new RequestMetadata(method, now);
receivedRequests.put(id, requestMetadata);
Object params = requestMessage.getParams();
String paramsJson = MessageJsonHandler.toString(params);
String paramsJson = toString(params);
String format = "[Trace - %s] Received request '%s - (%s)'\nParams: %s\n\n\n";
return String.format(format, date, method, id, paramsJson);
} else if (message instanceof ResponseMessage) {
ResponseMessage responseMessage = (ResponseMessage) message;
String id = responseMessage.getId();
RequestMetadata requestMetadata = sentRequests.remove(id);
if (requestMetadata == null) {
LOG.log(WARNING, String.format("Unmatched response message: %s", message));
LOG.log(WARNING, String.format("Unmatched response message: %s", toString(message)));
return null;
}
String method = requestMetadata.method;
long latencyMillis = now.toEpochMilli() - requestMetadata.start.toEpochMilli();
Object result = responseMessage.getResult();
String resultJson = MessageJsonHandler.toString(result);
String resultJson = toString(result);
Object error = responseMessage.getError();
String errorJson = MessageJsonHandler.toString(error);
String errorJson = toString(error);
String format = "[Trace - %s] Received response '%s - (%s)' in %sms\nResult: %s\nError: %s\n\n\n";
return String.format(format, date, method, id, latencyMillis, resultJson, errorJson);
} else if (message instanceof NotificationMessage) {
NotificationMessage notificationMessage = (NotificationMessage) message;
String method = notificationMessage.getMethod();
Object params = notificationMessage.getParams();
String paramsJson = MessageJsonHandler.toString(params);
String paramsJson = toString(params);
String format = "[Trace - %s] Received notification '%s'\nParams: %s\n\n\n";
return String.format(format, date, method, paramsJson);
} else {
LOG.log(WARNING, String.format("Unknown message type: %s", message));
LOG.log(WARNING, String.format("Unknown message type: %s", toString(message)));
return null;
}
}

private String toString(Object object) {
return jsonHandler != null ? jsonHandler.format(object) : MessageJsonHandler.toString(object);
}

/** Data class for holding pending request metadata. */
public static class RequestMetadata {
final String method;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonIOException;
import com.google.gson.JsonNull;
import com.google.gson.JsonParseException;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
import com.google.gson.stream.JsonWriter;
import com.google.gson.stream.MalformedJsonException;

/**
Expand All @@ -48,7 +50,7 @@ public class MessageJsonHandler {
public static final JsonRpcMethod CANCEL_METHOD = JsonRpcMethod.notification("$/cancelRequest", CancelParams.class);

private final Gson gson;

private final Map<String, JsonRpcMethod> supportedMethods;

private MethodProvider methodProvider;
Expand Down Expand Up @@ -119,6 +121,8 @@ public Message parseMessage(Reader input) throws JsonParseException {
Message message = gson.fromJson(jsonReader, Message.class);

if (message != null) {
message.setJsonHandler(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call might seem unnecessary, since we already set the MessageJsonHandler in MessageTypeAdapter and DebugMessageTypeAdapter as soon as a message is created, but it serves those clients that register their own message type adapter (similarly to how DebugMessageTypeAdapter is registered), but don't set the MessageJsonHandler for created messages.


// Check whether the input has been fully consumed
try {
if (jsonReader.peek() != JsonToken.END_DOCUMENT) {
Expand All @@ -144,10 +148,32 @@ public String serialize(Message message) {
public void serialize(Message message, Writer output) throws JsonIOException {
gson.toJson(message, Message.class, output);
}



/**
* Perform JSON serialization of the given object using the configuration of JSON-RPC messages
* enhanced with the pretty printing option.
*/
public String format(Object object) {
StringWriter writer = new StringWriter();
henryju marked this conversation as resolved.
Show resolved Hide resolved
JsonWriter jsonWriter = null;
try {
jsonWriter = gson.newJsonWriter(writer);
// Equivalent to set pretty printing on the gson builder
jsonWriter.setIndent(" ");
} catch (IOException e) {
throw new JsonIOException(e);
}
if (object != null) {
gson.toJson(object, object.getClass(), jsonWriter);
} else {
gson.toJson(JsonNull.INSTANCE, jsonWriter);
}
return writer.toString();
}


private static MessageJsonHandler toStringInstance;

/**
* Perform JSON serialization of the given object using the default configuration of JSON-RPC messages
* enhanced with the pretty printing option.
Expand All @@ -160,5 +186,5 @@ public static String toString(Object object) {
}
return toStringInstance.gson.toJson(object);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -379,13 +379,15 @@ protected Message createMessage(String jsonrpc, Either<String, Number> id, Strin
Object responseResult, ResponseError responseError) throws JsonParseException {
if (id != null && method != null) {
RequestMessage message = new RequestMessage();
message.setJsonHandler(handler);
message.setJsonrpc(jsonrpc);
message.setRawId(id);
message.setMethod(method);
message.setParams(params);
return message;
} else if (id != null) {
ResponseMessage message = new ResponseMessage();
message.setJsonHandler(handler);
message.setJsonrpc(jsonrpc);
message.setRawId(id);
if (responseError != null)
Expand All @@ -395,6 +397,7 @@ protected Message createMessage(String jsonrpc, Either<String, Number> id, Strin
return message;
} else if (method != null) {
NotificationMessage message = new NotificationMessage();
message.setJsonHandler(handler);
message.setJsonrpc(jsonrpc);
message.setMethod(method);
message.setParams(params);
Expand Down
Loading
Loading