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

Jump to bottom

Provide immutable MediaType class #823

Closed
gissuebot opened this issue Oct 31, 2014 · 13 comments
Closed

Provide immutable MediaType class #823

gissuebot opened this issue Oct 31, 2014 · 13 comments
Assignees
@gk5885
Labels
package=net status=fixed type=enhancement Make an existing feature better
Milestone
12.0

Comments

@gissuebot
Copy link

gissuebot commented Oct 31, 2014 •
edited by cgdecker

Original issue created by j...@nwsnet.de on 2011-12-14 at 02:08 PM

While the Java Mail API provides a content type class, it still has some drawbacks. Some are that it's not immutable (think constants) and it lacks a (static) constructor to create one just from primary and sub type as separate arguments without specifying a(n empty) list of parameters.

Also, HTTP uses MIME/content types as well, but those projects might not want to depend on an e-mail-related package.

My personal requirements:

  • immutability
  • Allow explicit addressing of the charset both on construction and as a getter (not just having to find it by key in the list of parameters).
  • Provide matching of the primary type, the sub type, parameters, and combinations of all of those (say, primary type and sub type and charset).
  • Maybe support matchers like "text/*", i.e. a wildcard for the sub type (that case might be sufficient). However, comparison with/without parameters (or just a selection of those) might require some further thinking.
  • Handle case (in)sensivity as appropriate. There are different requirements for the parameters of certain MIME types on how to interpret their keys and values regarding that¹.
  • Provide conversion methods from/to ContentType (Java Mail API).
  • Have a thoughtful implementation of equals. I think everything should be compared, including parameters.

¹) From my own investigation:

  • According to RFC 2045, section 5.1, the primary type, subtype, and parameter names must always be matched case insensitive.
  • According to RFC 2046, section 4.1.2, the value of the charset parameter (implicit default: US-ASCII) is case insensitive, but the values of other parameters might not be.

I have implemented an immutable content type myself, but so far only included specifically the charset parameter of text types, but no other parameters (e.g. filename; not to confuse with the parameter of the Content-Disposition: attachment header).

Also, a builder-style approach might be useful to optionally add parameters as needed before the immutable instance is created.

On a related note, the Guava class that contains HTTP header keys is quite nice. Maybe something similar for common content types would make sense? In that case, the content type class' API should allow for composition of pre-defined primary/sub type combinations (plaintext, HTML, CSV, JSON, octet-stream, PDF, images etc.) with parameters to fit specific needs.
@gissuebot
Copy link
Author

Original comment posted by kak@google.com on 2011-12-14 at 11:44 PM

We've got something like this internally...we'll work on open sourcing it in the next month or so.

Owner: kurt.kluever
Labels: Type-Enhancement

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014 •
edited by cgdecker

Original comment posted by j...@nwsnet.de on 2011-12-15 at 09:13 AM

Yay! I'm really looking forward to this, it would help me a lot.

Regarding the separate charset parameter, I came to the conclusion that it might be unnecessary as long as there is a constant for the parameter name available (similar to HttpHeaders, IIRC). A method Optional<String> findParameterValue(String parameterName) should work well.

@gissuebot
Copy link
Author

Original comment posted by kak@google.com on 2011-12-15 at 05:51 PM

Wow, we have almost exactly what you suggested ;-)

public Optional<Charset> charset();

I'm going to discuss this at our weekly API review meeting and see if we can get this out sooner rather than later.

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014 •
edited by cgdecker

Original comment posted by j...@nwsnet.de on 2011-12-15 at 11:22 PM

That'd be excellent, thanks!

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-01-10 at 11:43 PM

We'll get MediaType added to git very soon.

Labels: Milestone-Release12

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014 •
edited by cgdecker

Original comment posted by j...@nwsnet.de on 2012-01-11 at 10:42 AM

Sweet! Thanks for the update.

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014 •
edited by cgdecker

Original comment posted by j...@nwsnet.de on 2012-01-26 at 01:33 PM

How close is this to arrival in git? I'd really love to take a look its API. :)

@gissuebot
Copy link
Author

Original comment posted by kak@google.com on 2012-01-26 at 04:52 PM

It's "close" to hitting git...I'll see what we can do to get it out sometime soon.

Labels: Package-Net

@gissuebot
Copy link
Author

Original comment posted by kurt.kluever on 2012-02-15 at 06:04 PM

(Re-assigning to gak@ since he's the primary author and the newest member of our team!)

FYI, this will get pushed out to HEAD tomorrow morning (and subsequently released in Guava 12). Please take a look at the API and give us any feedback once it hits the repo.

Owner: gak@google.com

@gissuebot
Copy link
Author

Original comment posted by gak@google.com on 2012-03-14 at 04:39 PM

I'm pretty sure that we've covered most of the feature requests in the original bug, so I'm going to mark this fixed. Let us know if anybody has an more issues/requests and keep in mind that this class is still @Beta, so things may change.

Status: Fixed

@gissuebot
Copy link
Author

Original comment posted by husayt on 2012-12-30 at 12:53 AM

It is good to have this class in guava, but I was surprised it is missing "application/xml" type. Is it by purpose? It's really strange that such an often used type has been left out.

@gissuebot
Copy link
Author

Original comment posted by gak@google.com on 2013-01-02 at 06:30 PM

It wasn't left out for any reason other than that we hadn't had any request for it. It seems that the majority of our users internally had been using text/xml over application/xml, so it just hadn't come up. I'll be happy to add it though.

@gissuebot
Copy link
Author

Original comment posted by husayt on 2013-01-03 at 12:47 AM

Will be very appreciated, since it is one of the required mime types in Google Cloud Storage:
https://developers.google.com/storage/docs/developer-guide#overview

Thanks

@cgdecker cgdecker modified the milestone: 12.0 Nov 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gk5885 gk5885

Labels
package=net status=fixed type=enhancement Make an existing feature better
Projects
None yet
Milestone
12.0
Development

No branches or pull requests
3 participants
@cgdecker @gk5885 @gissuebot