search

filip26 / titanium-json-ld

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

Set the fallbackContentType for the DefaultHttpLoader to JSON (and/or make DefaultHttpLoader public) #350

Closed costas80 closed 4 months ago

costas80 commented 4 months ago

Is your feature request related to a problem? Please describe.

I have an issue loading a JSON-LD remote context over HTTP which is (incorrectly) being returned with a Content Type of text/plain. Checking the implementation of DefaultHttpLoader I see that the resulting error could easily be avoided if the fallbackContentType was set to MediaType.JSON, exactly as the FileLoader already does.

Describe the solution you'd like

It seems that this could be very easily resolved if the fallbackContentType for DefaultHttpLoader were set to MediaType.JSON. Alternatively this could be done when setting up your own DocumentLoaders whereby we could replace the default DefaultHttpLoader with a new instance on which the fallbackContentType is set. Frustratingly however, class DefaultHttpLoader is not public meaning that I cannot reuse it to create such an instance.

I would suggest two minor updates to resolve this:

Describe alternatives you've considered

The alternative to what I propose would be a complete rewrite (copy basically) of the DefaultHttpLoader which seems complete overkill to simply add one method call.

Additional context

If you confirm that doing these changes don't break anything I am unaware of, and don't go against any design principles I'd be happy to submit a PR making these changes.

filip26 commented 4 months ago

Hi, what about this https://github.com/filip26/titanium-json-ld/issues/132#issuecomment-826282856 ? Does it work for you?

costas80 commented 4 months ago

Yes it does! Don't know how I missed that... Many thanks @filip26!

filip26 commented 4 months ago

@costas80 if you have an idea on how to make the code more modular, configurable, then please don't hesitate to open PR. I would only recommend to contact me before making huge changes.

costas80 commented 4 months ago

No, I think that your solution covers this perfectly. I was trying to work directly with the DefaultHttpLoader when in fact using the HttpLoader was the way to go. In hindsight we can keep things as-is.