ISPN-3016 REST Metadata headers #1799
Conversation
|
"metadata headers" makes me think of the new metadata stuff that Galder is introducing in core. Wouldn't it be clearer to call them "topology headers", to make it more like HotRod? |
| val selectedMediaType = if (variant != null) variant.getMediaType.toString else "application/x-java-serialized-object" | ||
| selectedMediaType match { | ||
| case MediaType.APPLICATION_JSON => Response.ok.`type`(selectedMediaType).lastModified(lastMod).expires(expires).topology(cacheName, key, wantMetadata(metadata)).entity(streamIt(jsonMapper.writeValue(_, obj))).build | ||
| case MediaType.APPLICATION_XML => Response.ok.`type`(selectedMediaType).lastModified(lastMod).expires(expires).topology(cacheName, key, wantMetadata(metadata)).entity(streamIt(xstream.toXML(obj, _))).build |
There was a problem hiding this comment.
These lines are getting really long, I think they could use some wrapping.
|
@danberindei Why topology headers? The stuff @tristantarrant is working on is precisely relative to the metadata information (expiry time...etc). |
|
Hmmm, I find it a bit confusing that the REST can return different things when asking for an entry depending on the configuration, i.e. if ALWAYS is set. Having developed the Hot Rod protocol originally, I would have avoid doing that, it makes the results from the server more impredictable since they're not only based on the input parameters, but also in the configuration. Why do we need this? To avoid sending a parameter everytime? |
|
@danberindei Although at the moment the "metadata headers" stuff is for topology information only, I want to make it future proof in case we want to return further information. |
|
made some further changes, and also added infinispan/infinispan-server#98 for the server-side configuration I have renamed MetadataHeaders to ExtendedHeaders, since "standard metadata headers" should always be part of the response (last modified, expires, etc). I have removed "ALWAYS" from the enumeration. |
|
Rebased |
|
Checking again... |
| @@ -134,6 +180,23 @@ class Server(@Context request: Request, @Context servletContext: ServletContext, | |||
| } | |||
| } | |||
|
|
|||
| implicit private class extendedHeaders(val bld: Response.ResponseBuilder) { | |||
There was a problem hiding this comment.
If it's a class, it should start with capital... Extended...
There was a problem hiding this comment.
yeah, i forgot to find a better name for that, since otherwise it conflicted with the ExtendedHeaders class in configuration
There was a problem hiding this comment.
Given that implicits are relatively obscure for a lot of people, you should add some javadoc why you're using it and what it's effects are. This will help future maintainers :)
There was a problem hiding this comment.
Defo need a better name, otherwise it can be even worse with people getting confused with the other class.
|
Renamed the implicit class, renamed the primary owner header, added a comment |
https://issues.jboss.org/browse/ISPN-3016
Please note that I removed the "lastModified" field from the MIMECacheEntry class and I'm now using the InternalCacheEntry data to compute that field.
https://issues.jboss.org/browse/ISPN-3016