reta commented on a change in pull request #697:
URL: https://github.com/apache/cxf/pull/697#discussion_r493106096
##########
File path:
rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java
##########
@@ -137,22 +143,70 @@ public Object getActualEntity() {
return lastEntity != null ? lastEntity : entity;
}
+ @Override
public Object getEntity() {
return InjectionUtils.getEntity(getActualEntity());
}
+ @Override
public boolean hasEntity() {
- return getActualEntity() != null;
+ // per spec, need to check if the stream exists and if it has data.
+ Object actualEntity = getActualEntity();
+ if (actualEntity == null) {
+ return false;
+ } else if (actualEntity instanceof InputStream) {
+ final InputStream is = (InputStream) actualEntity;
+ try {
+ if (is.markSupported()) {
+ is.mark(1);
+ int i = is.read();
+ is.reset();
+ return i != -1;
+ } else {
+ try {
+ if (is.available() > 0) {
+ return true;
+ }
+ } catch (IOException ioe) {
+ //Do nothing
+ }
+ int b = is.read();
Review comment:
This would modify the state of the stream, correct?
##########
File path:
rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java
##########
@@ -137,22 +143,70 @@ public Object getActualEntity() {
return lastEntity != null ? lastEntity : entity;
}
+ @Override
public Object getEntity() {
return InjectionUtils.getEntity(getActualEntity());
}
+ @Override
public boolean hasEntity() {
- return getActualEntity() != null;
+ // per spec, need to check if the stream exists and if it has data.
+ Object actualEntity = getActualEntity();
+ if (actualEntity == null) {
+ return false;
+ } else if (actualEntity instanceof InputStream) {
+ final InputStream is = (InputStream) actualEntity;
+ try {
+ if (is.markSupported()) {
+ is.mark(1);
+ int i = is.read();
+ is.reset();
+ return i != -1;
+ } else {
+ try {
+ if (is.available() > 0) {
+ return true;
+ }
+ } catch (IOException ioe) {
+ //Do nothing
+ }
+ int b = is.read();
Review comment:
This would modify the state of the stream, correct? May be it would be
simpler to check/construct the PushbackInputStream and than do read / unread?
##########
File path:
rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java
##########
@@ -137,22 +143,70 @@ public Object getActualEntity() {
return lastEntity != null ? lastEntity : entity;
}
+ @Override
public Object getEntity() {
return InjectionUtils.getEntity(getActualEntity());
}
+ @Override
public boolean hasEntity() {
- return getActualEntity() != null;
+ // per spec, need to check if the stream exists and if it has data.
+ Object actualEntity = getActualEntity();
+ if (actualEntity == null) {
+ return false;
+ } else if (actualEntity instanceof InputStream) {
+ final InputStream is = (InputStream) actualEntity;
+ try {
+ if (is.markSupported()) {
+ is.mark(1);
+ int i = is.read();
+ is.reset();
+ return i != -1;
+ } else {
+ try {
+ if (is.available() > 0) {
+ return true;
+ }
+ } catch (IOException ioe) {
+ //Do nothing
+ }
+ int b = is.read();
+ if (b == -1) {
+ return false;
+ }
+ PushbackInputStream pbis;
+ if (is instanceof PushbackInputStream) {
+ pbis = (PushbackInputStream) is;
+ } else {
+ pbis = new PushbackInputStream(is, 1);
+ if (lastEntity != null) {
Review comment:
Unfamiliar with this part, do you have insights about `lastEntity` vs
`entity`?
##########
File path:
rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java
##########
@@ -218,42 +274,46 @@ private Date doGetDate(String dateHeader) {
: HttpUtils.getHttpDate(value.toString());
}
+ @Override
public EntityTag getEntityTag() {
Object header = metadata.getFirst(HttpHeaders.ETAG);
return header == null || header instanceof EntityTag ?
(EntityTag)header
: EntityTag.valueOf(header.toString());
}
+ @Override
public Locale getLanguage() {
Object header = metadata.getFirst(HttpHeaders.CONTENT_LANGUAGE);
return header == null || header instanceof Locale ? (Locale)header
: HttpUtils.getLocale(header.toString());
}
+ @Override
public Date getLastModified() {
return doGetDate(HttpHeaders.LAST_MODIFIED);
}
+ @Override
public int getLength() {
Object header = metadata.getFirst(HttpHeaders.CONTENT_LENGTH);
return HttpUtils.getContentLength(header == null ? null :
header.toString());
}
+ @Override
public URI getLocation() {
Object header = metadata.getFirst(HttpHeaders.LOCATION);
- if (header == null && outMessage != null) {
- header = outMessage.get(Message.REQUEST_URI);
- }
- return header == null || header instanceof URI ? (URI) header
+ return header == null || header instanceof URI ? (URI)header
Review comment:
Sorry, why the `outMessage.get(Message.REQUEST_URI)` got removed?
##########
File path:
rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java
##########
@@ -400,12 +472,30 @@ private Link makeAbsoluteLink(Link link) {
responseMessage.put(Message.PROTOCOL_HEADERS, getHeaders());
lastEntity = JAXRSUtils.readFromMessageBodyReader(readers,
cls, t,
- anns,
-
entityStream,
-
mediaType,
-
responseMessage);
- autoClose(cls, false);
- return castLastEntity();
+ anns,
+ entityStream,
+ mediaType,
+
responseMessage);
+ // close the entity after readEntity is called.
+ T tCastLastEntity = castLastEntity();
+ shouldClose = shouldClose && !(tCastLastEntity instanceof
Closeable)
Review comment:
Shouldn't `AutoCloseable` also be checked?
##########
File path:
rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/ResponseImpl.java
##########
@@ -527,10 +621,47 @@ public String getReasonPhrase() {
return statusEnum != null ? statusEnum.getReasonPhrase() : "";
}
+ @Override
public int getStatusCode() {
return statusCode;
}
};
}
-}
+
+ private enum PrimitiveTypes {
+ BYTE(Byte.class, byte.class) { },
+ SHORT(Short.class, short.class) { },
+ INTEGER(Integer.class, int.class) { },
+ LONG(Long.class, long.class) { },
+ FLOAT(Float.class, float.class) { },
+ DOUBLE(Double.class, double.class) { },
+ BOOLEAN(Boolean.class, boolean.class) { },
+ CHAR(Character.class, char.class) { };
+
+ private final Class<?> wrapper;
+ private final Class<?> primitive;
+
+ PrimitiveTypes(Class<?> wrapper, Class<?> primitive) {
+ this.wrapper = wrapper;
+ this.primitive = primitive;
+ }
+
+ public static PrimitiveTypes forType(Class<?> type) {
+ for (PrimitiveTypes primitive : PrimitiveTypes.values()) {
+ if (primitive.supports(type)) {
+ return primitive;
+ }
+ }
+ return null;
+ }
+
+ public boolean supports(Class<?> type) {
+ return type == wrapper || type == primitive;
+ }
+ }
+
+ private static boolean isBasicType(Class<?> type) {
+ return PrimitiveTypes.forType(type) != null ||
Number.class.isAssignableFrom(type);
Review comment:
You probably could reduce it to `return type.isPrimitive() ||
Number.class.isAssignableFrom(type) ||
Boolean.class.isAssignableFrom(type) || Char.class.isAssignableFrom(type));`
(PrimitiveTypes is not really needed in this case).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
