GeoServer

scheduling for prompt resolution since this is one easy to fix that bothers me since a while ago

fixed on 1.7.x from revisions #9983 to #9989.
ported to trunk at revision #10003

The new code does a straight equals() match against the output format string. But this often does not suffice for cases like kml where the output format is specified with a "+" which gets stripped out, so a looser check is required.

I am surprised there are no unit tests that catch this one. I think before continuing with this improvement we should have a unit test for each output format.

Also, when in doubt about some piece of come that does not seem to make sense (like the sloppy format matching) ask, there's usually a reason without a test

quoting the mailing list so we better keep the discussion here on the issue:
Agreed, we don't want regressions...
Yet, I wonder about one thing.
The old KMZMapProducerFactory stated the official mime type had no "+xml" at the end:
/**

• Official KMZ mime type
  */
  static final String MIME_TYPE = "application/vnd.google-earth.kmz";

And that's why I didn't include an alias for the map producer as did for other formats like SVG, or "rss", "application/rss xml", application/rss+xml.

I can easily add an alias for "kml" or whatever, but do we actually want the output formats in a request to be "whatever starts with..." It doesn't seem quite right to me. I mean, the output formats are meant to be the ones stated in the capabilities document, not whatever starts with a given string...

That's almost it, I didn't allowed that because that was not supposed to happen in my understanding. Yet I see now there's actually a missing alias in order to use just "kmz". Just that before kmz where not stated in the capabilities neither...

I propose to use the aliases as stick to what we state we support in the capabilities. Thoughts?

Agreed there should be more unit tests. That gets clearer by the fact there wasn't a previous test for that format options or it would have failed after the changes.

So, by Andrea's comment, it seems the kml output formats could be expressed in various ways? are there a limited set of them so we can use aliases and advertise in the capabilities? or should I find a way to make a sloppy matching without the factory's canProduce() method?

Worked on this and I think I have a fair solution.
This would be the deal:

interface GetMapProducer{
  
  /** the actual content type for the response header, does not need to match the requested output format */
  String getContentType();
  
  /** the output format names this producer can be invoked by, and the ones to state in capabilities */
  List<String>getOutputFormatNames();

  /** shall be called before produceMap with the actual output format the request was made with. Some producers depend on it */
  void setOutputFormat(String outputFormat);
  
  /** the output format the request was made with */
  String getOutputFormat();

  void produceMap();
  void setMapContext(WMSMapContext context);
  ....
}

This way we decouple the mime type to be written as a response header and what we state in getcaps, better separation of concerns than having a mixin of them.
Also, this is so since some producers does completelly different one thing and the other. for example, the openlayers one states the format application/openlayers, but the mime type it sends is text/html.
Others (png, tiff, geotiff) makes a decision based on the requested output format, producing a different result based on it (png vs png8, tiff vs tiff8, etc), but the output mime type shall be the same "image/png"
That is certainly awful, like making a code path fork depending on the output format requested seems to imply those should be different MapProducers... we could fix than later, but for now this is the proposal to avoid regressions

comments?

marking as resolved so anyone that notices and wants to test for a bit can do it

All these issues have been in "resolved" state for at least one month (many of them, much more than that). Since no one has reopened them, I'm mass-switching them to closed state. Reopen if you feel the issue has not been addressed properly.