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

Remove Trailers #807

Merged
merged 1 commit into from
Aug 14, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public void testCredentialsThrows() throws IOException {
Metadata.Headers headers = new Metadata.Headers();
interceptedCall.start(listener, headers);
ArgumentCaptor<Status> statusCaptor = ArgumentCaptor.forClass(Status.class);
Mockito.verify(listener).onClose(statusCaptor.capture(), isA(Metadata.Trailers.class));
Mockito.verify(listener).onClose(statusCaptor.capture(), isA(Metadata.class));
Assert.assertNull(headers.getAll(AUTHORIZATION));
Mockito.verify(call, never()).start(listener, headers);
Assert.assertEquals(Status.Code.UNAUTHENTICATED, statusCaptor.getValue().getCode());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ public void onMessage(ByteBuf message) {

@Override
public void onHalfClose() {
call.close(Status.OK, new Metadata.Trailers());
call.close(Status.OK, new Metadata());
}

@Override
Expand Down Expand Up @@ -287,7 +287,7 @@ public void onMessage(ByteBuf message) {

@Override
public void onHalfClose() {
call.close(Status.OK, new Metadata.Trailers());
call.close(Status.OK, new Metadata());
}

@Override
Expand Down Expand Up @@ -323,7 +323,7 @@ public void onMessage(ByteBuf message) {

@Override
public void onHalfClose() {
call.close(Status.OK, new Metadata.Trailers());
call.close(Status.OK, new Metadata());
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/io/grpc/ChannelImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ private InactiveTransport(Status s) {
@Override
public ClientStream newStream(
MethodDescriptor<?, ?> method, Headers headers, ClientStreamListener listener) {
listener.closed(shutdownStatus, new Metadata.Trailers());
listener.closed(shutdownStatus, new Metadata());
return new ClientCallImpl.NoopClientStream();
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/io/grpc/ClientCall.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public abstract static class Listener<T> {
* @param status the result of the remote call.
* @param trailers metadata provided at call completion.
*/
public abstract void onClose(Status status, Metadata.Trailers trailers);
public abstract void onClose(Status status, Metadata trailers);

/**
* This indicates that the ClientCall is now capable of sending additional messages (via
Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/io/grpc/ClientCallImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public boolean isReady() {
private void closeCallPrematurely(ClientStreamListener listener, Status status) {
Preconditions.checkState(stream == null, "Stream already created");
stream = new NoopClientStream();
listener.closed(status, new Metadata.Trailers());
listener.closed(status, new Metadata());
}

private ScheduledFuture<?> startDeadlineTimer(long timeoutMicros) {
Expand Down Expand Up @@ -267,7 +267,7 @@ public void run() {
}

@Override
public void closed(Status status, Metadata.Trailers trailers) {
public void closed(Status status, Metadata trailers) {
if (status.getCode() == Status.Code.CANCELLED && deadlineNanoTime != null) {
// When the server's deadline expires, it can only reset the stream with CANCEL and no
// description. Since our timer may be delayed in firing, we double-check the deadline and
Expand All @@ -276,11 +276,11 @@ public void closed(Status status, Metadata.Trailers trailers) {
if (deadlineNanoTime <= System.nanoTime()) {
status = Status.DEADLINE_EXCEEDED;
// Replace trailers to prevent mixing sources of status and trailers.
trailers = new Metadata.Trailers();
trailers = new Metadata();
}
}
final Status savedStatus = status;
final Metadata.Trailers savedTrailers = trailers;
final Metadata savedTrailers = trailers;
callExecutor.execute(new Runnable() {
@Override
public void run() {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/io/grpc/ClientInterceptors.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public final void start(Listener<RespT> responseListener, Metadata.Headers heade
// to a NO-OP one to prevent the IllegalStateException. The user will finally get notified
// about the error through the listener.
delegate = (ClientCall<ReqT, RespT>) NOOP_CALL;
responseListener.onClose(Status.fromThrowable(e), new Metadata.Trailers());
responseListener.onClose(Status.fromThrowable(e), new Metadata());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void onMessage(RespT message) {
}

@Override
public void onClose(Status status, Metadata.Trailers trailers) {
public void onClose(Status status, Metadata trailers) {
delegate().onClose(status, trailers);
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/io/grpc/ForwardingServerCall.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public boolean isReady() {
}

@Override
public void close(Status status, Metadata.Trailers trailers) {
public void close(Status status, Metadata trailers) {
delegate().close(status, trailers);
}

Expand Down
29 changes: 11 additions & 18 deletions core/src/main/java/io/grpc/Metadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
* </p>
*/
@NotThreadSafe
public abstract class Metadata {
public class Metadata {

/**
* All binary headers should have this suffix in their names. Vice versa.
Expand Down Expand Up @@ -108,7 +108,7 @@ public Integer parseAsciiString(String serialized) {
* Constructor called by the transport layer when it receives binary metadata.
*/
// TODO(louiscryan): Convert to use ByteString so we can cache transformations
private Metadata(byte[]... binaryValues) {
public Metadata(byte[]... binaryValues) {
for (int i = 0; i < binaryValues.length; i++) {
String name = new String(binaryValues[i], US_ASCII);
storeAdd(name, new MetadataEntry(name.endsWith(BINARY_HEADER_SUFFIX), binaryValues[++i]));
Expand All @@ -118,7 +118,7 @@ private Metadata(byte[]... binaryValues) {
/**
* Constructor called by the application layer when it wants to send metadata.
*/
private Metadata() {}
public Metadata() {}

private void storeAdd(String name, MetadataEntry value) {
List<MetadataEntry> values = store.get(name);
Expand Down Expand Up @@ -286,6 +286,11 @@ public void merge(Metadata other, Set<Key<?>> keys) {
}
}

@Override
public String toString() {
return "Metadata(" + toStringInternal() + ")";
}

private String toStringInternal() {
return store.toString();
}
Expand Down Expand Up @@ -375,31 +380,19 @@ public String toString() {
/**
* Concrete instance for metadata attached to the end of the call. Only provided by
* servers.
*
* @deprecated use Metadata instead.
*/
@Deprecated
public static class Trailers extends Metadata {
/**
* Called by the transport layer to create trailers from their binary serialized values.
*
* <p>This method does not copy the provided byte arrays. The byte arrays must not be mutated.
*/
public Trailers(byte[]... headers) {
super(headers);
}

/**
* Called by the application layer to construct trailers prior to passing them to the
* transport for serialization.
*/
public Trailers() {
}

@Override
public String toString() {
return "Trailers(" + super.toStringInternal() + ")";
}
}


/**
* Marshaller for metadata values that are serialized into raw binary.
*/
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/io/grpc/ServerCall.java
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,12 @@ public boolean isReady() {
* status} is not equal to {@link Status#OK}, then the call is said to have failed.
*
* <p>If {@code status} is not {@link Status#CANCELLED} and no errors or cancellations are known
* to have occured, then a {@link Listener#onComplete} notification should be expected.
* to have occurred, then a {@link Listener#onComplete} notification should be expected.
* Otherwise {@link Listener#onCancel} has been or will be called.
*
* @throws IllegalStateException if call is already {@code close}d
*/
public abstract void close(Status status, Metadata.Trailers trailers);
public abstract void close(Status status, Metadata trailers);

/**
* Returns {@code true} when the call is cancelled and the server is encouraged to abort
Expand Down
16 changes: 8 additions & 8 deletions core/src/main/java/io/grpc/ServerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -308,14 +308,14 @@ public void run() {
if (method == null) {
stream.close(
Status.UNIMPLEMENTED.withDescription("Method not found: " + methodName),
new Metadata.Trailers());
new Metadata());
timeout.cancel(true);
return;
}
listener = startCall(stream, methodName, method.getMethodDefinition(), timeout,
headers);
} catch (Throwable t) {
stream.close(Status.fromThrowable(t), new Metadata.Trailers());
stream.close(Status.fromThrowable(t), new Metadata());
timeout.cancel(true);
throw Throwables.propagate(t);
} finally {
Expand Down Expand Up @@ -410,9 +410,9 @@ private void setListener(ServerStreamListener listener) {
}

/**
* Like {@link ServerCall#close(Status, Metadata.Trailers)}, but thread-safe for internal use.
* Like {@link ServerCall#close(Status, Metadata)}, but thread-safe for internal use.
*/
private void internalClose(Status status, Metadata.Trailers trailers) {
private void internalClose(Status status, Metadata trailers) {
// TODO(ejona86): this is not thread-safe :)
stream.close(status, trailers);
}
Expand All @@ -425,7 +425,7 @@ public void run() {
try {
getListener().messageRead(message);
} catch (Throwable t) {
internalClose(Status.fromThrowable(t), new Metadata.Trailers());
internalClose(Status.fromThrowable(t), new Metadata());
throw Throwables.propagate(t);
}
}
Expand All @@ -440,7 +440,7 @@ public void run() {
try {
getListener().halfClosed();
} catch (Throwable t) {
internalClose(Status.fromThrowable(t), new Metadata.Trailers());
internalClose(Status.fromThrowable(t), new Metadata());
throw Throwables.propagate(t);
}
}
Expand Down Expand Up @@ -504,7 +504,7 @@ public void sendMessage(RespT message) {
stream.writeMessage(resp);
stream.flush();
} catch (Throwable t) {
close(Status.fromThrowable(t), new Metadata.Trailers());
close(Status.fromThrowable(t), new Metadata());
throw Throwables.propagate(t);
}
}
Expand All @@ -515,7 +515,7 @@ public boolean isReady() {
}

@Override
public void close(Status status, Metadata.Trailers trailers) {
public void close(Status status, Metadata trailers) {
Preconditions.checkState(!closeCalled, "call already closed");
closeCalled = true;
stream.close(status, trailers);
Expand Down
8 changes: 4 additions & 4 deletions core/src/main/java/io/grpc/inprocess/InProcessTransport.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public void run() {
public synchronized ClientStream newStream(MethodDescriptor<?, ?> method,
Metadata.Headers headers, ClientStreamListener clientStreamListener) {
if (shutdownStatus != null) {
clientStreamListener.closed(shutdownStatus, new Metadata.Trailers());
clientStreamListener.closed(shutdownStatus, new Metadata());
return new NoopClientStream();
}
InProcessStream stream = new InProcessStream();
Expand Down Expand Up @@ -195,7 +195,7 @@ private class InProcessServerStream implements ServerStream {
@GuardedBy("this")
private Status clientNotifyStatus;
@GuardedBy("this")
private Metadata.Trailers clientNotifyTrailers;
private Metadata clientNotifyTrailers;
// Only is intended to prevent double-close when client cancels.
@GuardedBy("this")
private boolean closed;
Expand Down Expand Up @@ -266,7 +266,7 @@ public synchronized void writeHeaders(Metadata.Headers headers) {
}

@Override
public void close(Status status, Metadata.Trailers trailers) {
public void close(Status status, Metadata trailers) {
synchronized (this) {
if (closed) {
return;
Expand Down Expand Up @@ -306,7 +306,7 @@ private synchronized boolean internalCancel(Status status) {
log.log(Level.WARNING, "Exception closing stream", t);
}
}
clientStreamListener.closed(status, new Metadata.Trailers());
clientStreamListener.closed(status, new Metadata());
return true;
}
}
Expand Down
12 changes: 6 additions & 6 deletions core/src/main/java/io/grpc/internal/AbstractClientStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public abstract class AbstractClientStream<IdT> extends AbstractStream<IdT>
// Stored status & trailers to report when deframer completes or
// transportReportStatus is directly called.
private Status status;
private Metadata.Trailers trailers;
private Metadata trailers;
private Runnable closeListenerTask;


Expand Down Expand Up @@ -100,7 +100,7 @@ protected void inboundTransportError(Status errorStatus) {
}
// For transport errors we immediately report status to the application layer
// and do not wait for additional payloads.
transportReportStatus(errorStatus, false, new Metadata.Trailers());
transportReportStatus(errorStatus, false, new Metadata());
}

/**
Expand Down Expand Up @@ -165,7 +165,7 @@ protected final void deframeFailed(Throwable cause) {
* @param trailers the received trailers
* @param status the status extracted from the trailers
*/
protected void inboundTrailersReceived(Metadata.Trailers trailers, Status status) {
protected void inboundTrailersReceived(Metadata trailers, Status status) {
Preconditions.checkNotNull(trailers, "trailers");
if (inboundPhase() == Phase.STATUS) {
log.log(Level.INFO, "Received trailers on closed stream {0}\n {1}\n {2}",
Expand Down Expand Up @@ -212,7 +212,7 @@ protected final void internalSendFrame(WritableBuffer frame, boolean endOfStream
* @param trailers new instance of {@code Trailers}, either empty or those returned by the server
*/
public void transportReportStatus(final Status newStatus, boolean stopDelivery,
final Metadata.Trailers trailers) {
final Metadata trailers) {
Preconditions.checkNotNull(newStatus, "newStatus");

boolean closingLater = closeListenerTask != null && !stopDelivery;
Expand Down Expand Up @@ -240,7 +240,7 @@ public void transportReportStatus(final Status newStatus, boolean stopDelivery,
/**
* Creates a new {@link Runnable} to close the listener with the given status/trailers.
*/
private Runnable newCloseListenerTask(final Status status, final Metadata.Trailers trailers) {
private Runnable newCloseListenerTask(final Status status, final Metadata trailers) {
return new Runnable() {
@Override
public void run() {
Expand All @@ -252,7 +252,7 @@ public void run() {
/**
* Closes the listener if not previously closed.
*/
private void closeListener(Status newStatus, Metadata.Trailers trailers) {
private void closeListener(Status newStatus, Metadata trailers) {
if (!listenerClosed) {
listenerClosed = true;
closeDeframer();
Expand Down
Loading