search

sweetrdf / sparqlClient

MIT License
0 stars 0 forks source link

StandardConnection: make properties "connection" and "requestFactory" protected #6

Open k00ni opened 1 year ago

k00ni commented 1 year ago

StandardConnection uses private properties connection and requestFactory inside functions which are public, like query. When I create a subclass, I can't access connection and requestFactory. I would like to change their visibility from private to protected.

Why? I want to query Wikidata , but run into errors when using StandardConnection. I think its because they return data as XML and not JSON. Making a sub class which solves that is hard, because I have to code "around" these limitations above. Maybe I overlooked something, if so please give me a hint. Thanks!

@zozlak What do you think?

zozlak commented 1 year ago

Could you provide examples of failing code?

In regard to your question I'm generally sceptical about protected properties because they introduce a blurry and slippery API contract. If the same can be achieved with e.g. dependency injection, I would rather stick to the DI. Anyway I need to understand the problem better first and that's why I'm asking for code samples.

k00ni commented 1 year ago

After using the right SPARL endpoint URL, the script didn't throw errors anymore. Currently I don't receive any data/objects, even for simple queries and I don't know why. Will investigate and should not be of your concern.

What I saw is, in Connection class you "force" the header Accept to be of a certain value: https://github.com/sweetrdf/sparqlClient/blob/master/src/sparqlClient/Connection.php#L41 That might be problematic for some endpoints. My initial question is solved, but we could use this issue to brainstorm if a more flexible approach makes sense.

WDYT?

zozlak commented 1 year ago

you "force" the header Accept to be of a certain value

As the Statement class can only deal with JSON now, it makes a lot of sense to me :-)

Although this should rather include also application/sparql-results+json ending up with something like Accept: application/sparql-results+json, application/json;q=0.9.

And of course it would be nice to add support for the CSV/TSV and XML, especially all of them can be pretty easily parsed as a stream.

zozlak commented 1 year ago

83139302ee967592baca74f532b06653b7a6fbcd (release 0.6.0) add support for all known SPARQL result formats and unified the API for the ASK queries.

k00ni commented 1 year ago

Cool!