Skip to content

Commit

Permalink
remove shouldSetResponseContent
Browse files Browse the repository at this point in the history
  • Loading branch information
@ikhoon
ikhoon committed Jul 24, 2024
1 parent fe399a2 commit ff4c77e
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 32 deletions.
15 changes: 6 additions & 9 deletions grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/AbstractServerCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ protected final void maybeCancel() {
if (!closeCalled) {
cancelled = true;
try (SafeCloseable ignore = ctx.push()) {
close(new ServerStatusAndMetadata(Status.CANCELLED, new Metadata(), true, true));
close(new ServerStatusAndMetadata(Status.CANCELLED, new Metadata(), true));
}
}
}
Expand All @@ -210,15 +210,15 @@ protected final void maybeCancel() {
public final void close(Status status, Metadata metadata) {
final Throwable cause = status.getCause();
if (cause == null) {
close(new ServerStatusAndMetadata(status, metadata, false));
close(new ServerStatusAndMetadata(status, metadata));
return;
}

Status newStatus = exceptionHandler.handle(ctx, status, cause, metadata);
if (status.getDescription() != null) {
newStatus = newStatus.withDescription(status.getDescription());
}
close(new ServerStatusAndMetadata(newStatus, metadata, true), cause);
close(new ServerStatusAndMetadata(newStatus, metadata), cause);
}

public final void close(Throwable exception) {
Expand All @@ -227,7 +227,7 @@ public final void close(Throwable exception) {

protected final void close(Throwable exception, boolean cancelled) {
final StatusAndMetadata statusAndMetadata = exceptionHandler.handle(ctx, exception);
close(new ServerStatusAndMetadata(statusAndMetadata.status(), statusAndMetadata.metadata(), true,
close(new ServerStatusAndMetadata(statusAndMetadata.status(), statusAndMetadata.metadata(),
cancelled), exception);
}

Expand All @@ -252,14 +252,12 @@ private void doClose(ServerStatusAndMetadata statusAndMetadata, @Nullable Throwa
if (isCancelled()) {
// No need to write anything to client if cancelled already.
statusAndMetadata.shouldCancel(true);
statusAndMetadata.shouldSetResponseContent(true);
closeListener(statusAndMetadata);
return;
}

if (status.getCode() == Code.CANCELLED && status.getCause() instanceof ClosedStreamException) {
statusAndMetadata.shouldCancel(true);
statusAndMetadata.shouldSetResponseContent(true);
closeListener(statusAndMetadata);
return;
}
Expand All @@ -282,7 +280,6 @@ private void doClose(ServerStatusAndMetadata statusAndMetadata, @Nullable Throwa
protected abstract void doClose(ServerStatusAndMetadata statusAndMetadata);

protected final void closeListener(ServerStatusAndMetadata statusAndMetadata) {
final boolean setResponseContent = statusAndMetadata.shouldSetResponseContent();
final boolean cancelled = statusAndMetadata.isShouldCancel();
if (!listenerClosed) {
listenerClosed = true;
Expand All @@ -292,7 +289,7 @@ protected final void closeListener(ServerStatusAndMetadata statusAndMetadata) {
ctx.logBuilder().requestContent(GrpcLogUtil.rpcRequest(method, simpleMethodName), null);
}

if (setResponseContent) {
if (!ctx.log().isAvailable(RequestLogProperty.RESPONSE_CONTENT)) {
ctx.logBuilder().responseContent(GrpcLogUtil.rpcResponse(statusAndMetadata, firstResponse()),
null);
}
Expand Down Expand Up @@ -339,7 +336,7 @@ public void onRequestMessage(DeframedMessage message, boolean endOfStream) {
final Status status = Status.INTERNAL.withDescription(
"More than one request messages for unary call or server streaming " +
"call");
closeListener(new ServerStatusAndMetadata(status, new Metadata(), true, true));
closeListener(new ServerStatusAndMetadata(status, new Metadata(), true));
return;
}
messageReceived = true;
Expand Down
20 changes: 3 additions & 17 deletions grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/ServerStatusAndMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,13 @@
public final class ServerStatusAndMetadata extends StatusAndMetadata {

private boolean shouldCancel;
// Set true if response content log should be written
private boolean setResponseContent;

public ServerStatusAndMetadata(Status status, @Nullable Metadata metadata, boolean setResponseContent) {
public ServerStatusAndMetadata(Status status, @Nullable Metadata metadata) {
super(status, metadata);
this.setResponseContent = setResponseContent;
}

public ServerStatusAndMetadata(Status status, @Nullable Metadata metadata, boolean setResponseContent,
boolean shouldCancel) {
public ServerStatusAndMetadata(Status status, @Nullable Metadata metadata, boolean shouldCancel) {
super(status, metadata);
this.setResponseContent = setResponseContent;
this.shouldCancel = shouldCancel;
}

Expand All @@ -54,16 +49,8 @@ public void shouldCancel(boolean cancel) {
shouldCancel = cancel;
}

public void shouldSetResponseContent(boolean setResponseContent) {
this.setResponseContent = setResponseContent;
}

public boolean shouldSetResponseContent() {
return setResponseContent;
}

public ServerStatusAndMetadata withStatus(Status status) {
return new ServerStatusAndMetadata(status, metadata(), shouldSetResponseContent(), isShouldCancel());
return new ServerStatusAndMetadata(status, metadata(), isShouldCancel());
}

@Override
Expand All @@ -72,7 +59,6 @@ public String toString() {
.add("status", status())
.add("metadata", metadata())
.add("shouldCancel", shouldCancel)
.add("setResponseContent", setResponseContent)
.toString();
}
}
2 changes: 1 addition & 1 deletion grpc/src/main/java/com/linecorp/armeria/server/grpc/FramedGrpcService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ private static <I, O> void startCall(ServerMethodDefinition<I, O> methodDef, Ser
ctx.whenRequestCancelling().handle((cancellationCause, unused) -> {
final StatusAndMetadata statusAndMetadata = call.exceptionHandler().handle(ctx, cancellationCause);
call.close(new ServerStatusAndMetadata(statusAndMetadata.status(), statusAndMetadata.metadata(),
true, true));
true));
return null;
});
}
Expand Down
4 changes: 1 addition & 3 deletions grpc/src/main/java/com/linecorp/armeria/server/grpc/StreamingServerCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ public void doClose(ServerStatusAndMetadata statusAndMetadata) {
} else {
// A stream was closed already.
statusAndMetadata.shouldCancel(true);
statusAndMetadata.shouldSetResponseContent(true);
closeListener(statusAndMetadata);
return;
}
Expand All @@ -225,7 +224,6 @@ public void doClose(ServerStatusAndMetadata statusAndMetadata) {
res.close();
}
} finally {
statusAndMetadata.shouldSetResponseContent(false);
closeListener(statusAndMetadata);
}
}
Expand Down Expand Up @@ -274,6 +272,6 @@ public void transportReportStatus(Status status, Metadata metadata) {
// failure there's no need to notify the server listener of it).
return;
}
closeListener(new ServerStatusAndMetadata(status, metadata, true, true));
closeListener(new ServerStatusAndMetadata(status, metadata, true));
}
}
3 changes: 1 addition & 2 deletions grpc/src/main/java/com/linecorp/armeria/server/grpc/UnaryServerCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,13 @@ public void doClose(ServerStatusAndMetadata statusAndMetadata) {

// Set responseContent before closing stream to use responseCause in error handling
ctx.logBuilder().responseContent(GrpcLogUtil.rpcResponse(statusAndMetadata, responseMessage), null);
statusAndMetadata.shouldSetResponseContent(false);
resFuture.complete(response);
} catch (Exception ex) {
final StatusAndMetadata statusAndMetadata0 = exceptionHandler().handle(ctx, ex);
final Status status = statusAndMetadata0.status();
final Metadata metadata = statusAndMetadata0.metadata();
assert metadata != null;
statusAndMetadata = new ServerStatusAndMetadata(status, metadata, true);
statusAndMetadata = new ServerStatusAndMetadata(status, metadata);

final ResponseHeadersBuilder trailersBuilder = defaultResponseHeaders().toBuilder();
final HttpResponse response = HttpResponse.of(
Expand Down

0 comments on commit ff4c77e

Please sign in to comment.