filip26 / titanium-json-ld

A JSON-LD 1.1 Processor & API
https://apicatalog.com
Apache License 2.0
134 stars 33 forks source link

API MediaType missing of(String type, String subtype, Map<String, List<String>> parameters) #355

Closed vorburger closed 3 months ago

vorburger commented 3 months ago

In JsonLdDocumentLoader.java in https://github.com/enola-dev/enola/pull/796 (WIP) I am creating a new no.hasmac.jsonld.http.media.MediaType, from a Guava com.google.common.net.MediaType.

With the API as-is (with lots of final and protected and what not), the only way to do this without loosing parameters seems to be:

MediaType titaniumMediaType = MediaType.of(guavaMediaType.toString());

This "works" (of course), but toString() seems a little bit like a shame; it could be cool if MediaType had an:

MediaType.of(String type, String subtype, Map<String, List<String>> parameters) 
filip26 commented 3 months ago

@vorburger agreed MediaType.of(type, subtype, params) would be useful. How to align it with MediaTypeParams ?

Feel free to open PR.

FYI final and protected keywords help with backward compatibility. In order to keep Titanium maintainable, the strategy is to keep public API as minimal as possible, unless there is a good reason, e.g. a use-case like yours.

vorburger commented 3 months ago

How to align it with MediaTypeParams ?

MediaTypeParams is public final but its constructor is protected (a combination which doesn't really make sense I guess).

With the API design as it is, it seems simplest to me to consider MediaTypeParams to be (effectively) internal (package private) and simply add an of() with an Map<String, List<String>> parametersargument. (BTW The com.google.common.net.MediaType uses an com.google.common.collect.ImmutableListMultimap for its parameters.)

I'll see if I can send you a PR for this some time.

filip26 commented 3 months ago

MediaTypeParams is public final but its constructor is protected (a combination which doesn't really make sense I guess

Yeah, feel free change the scope to private. I guess I made that protected with some future goal on mind, to unfinal those classes when the implementation gets completed, i.e. nice complete API and tests.