ruby-rdf / sparql-client

SPARQL client for Ruby.
http://rubygems.org/gems/sparql-client
The Unlicense
112 stars 58 forks source link

raise error when no suitable rdf reader is found #66

Closed nvdk closed 8 years ago

nvdk commented 8 years ago

Currently sparql-client will silently fail with a nil if no suitable rdf reader is found. This can lead to the awkward situation where a nil is returned for the query and the user has to find out what is causing it. This pull request tries to quicken this debugging proces by at least indicating the point of failure by raising a descriptive error.

[1] pry(main)> require 'sparql/client'
=> true
[2] pry(main)> s = SPARQL::Client.new('http://localhost:8890/sparql')
=> #<SPARQL::Client:0x8fc(http://localhost:8890/sparql)>
[3] pry(main)> s.query("CONSTRUCT  {?s a ?type}  WHERE {?s a ?type}").first
NoMethodError: undefined method `first' for nil:NilClass
from (pry):3:in `<eval>'
[4] pry(main)> require 'rdf/turtle'
=> true
[5] pry(main)> s = SPARQL::Client.new('http://localhost:8890/sparql')
=> #<SPARQL::Client:0x902(http://localhost:8890/sparql)>
[6] pry(main)> s.query("CONSTRUCT  {?s a ?type}  WHERE {?s a ?type}").first
=> [#<RDF::URI:0x906 URI:http://www.openlinksw.com/virtrdf-data-formats#default-iid>, #<RDF::URI:0x908 URI:http://www.w3.org/1999/02/22-rdf-syntax-ns#type>, #<RDF::URI:0x90a URI:http://www.openlinksw.com/schemas/virtrdf#QuadMapFormat>]
coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.008%) to 94.437% when pulling a16c0ec0a30a3fc900d16f8ca54bdcdc8090fc99 on nvdk:patch-1 into ddf951d03c8af2c9ce589a5667d041e5b137e529 on ruby-rdf:develop.

gkellogg commented 8 years ago

Several specs fail as a result of this change. Generally, a pull request should include spec updates to test that the feature works correctly and that code coverage is adequate.

nvdk commented 8 years ago

Understood, if you agree with approach I will update the required specs accordingly. On 7 Jun 2016 17:03, "Gregg Kellogg" notifications@github.com wrote:

Several specs fail as a result of this change. Generally, a pull request should include spec updates to test that the feature works correctly and that code coverage is adequate.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ruby-rdf/sparql-client/pull/66#issuecomment-224309300, or mute the thread https://github.com/notifications/unsubscribe/AA1oXvzZgyiBHSuP13-BRvmOBIkFeF-gks5qJYgngaJpZM4IvrKo .

gkellogg commented 8 years ago

I think it's reasonable to fail with an exception if now reader is found, although this would seem to be avoidable, as the request is made with the set of mime-types available to the client, which should be used by the server when encoding the response, but it may not, in which case failing with an exception would be good. It might be a form of RDF::ReaderError, rather than ArgumentError.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.008%) to 94.437% when pulling 893d795afe30e24f9ce01630ffe6d596b4a38cbe on nvdk:patch-1 into ddf951d03c8af2c9ce589a5667d041e5b137e529 on ruby-rdf:develop.

nvdk commented 8 years ago

I've updated the specs where appropriate, for the two remaining failing tests I believe I'm running into a bug in parse_rdf_serialization.

when querying a remote endpoint should handle successful response with overridden plain header

Tries to load an Rdf::Reader for turtle, which is not present as sparql/client currently does not depend on the rdf-turtle gem.

However I think it should actually use a reader based on the returned content-type and not the requested one.

when querying a remote endpoint should handle successful response with custom headers

Similar issue, RDF::Reader.for({:headers=>{"Authorization"=>"Basic XXX=="}}) actually returns nil, again the returned content type should be passed to RDF::Reader.for and not the configured options.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.008%) to 94.437% when pulling 436bcb6c4865e75353e5d1c307faf1c4f891dd57 on nvdk:patch-1 into ddf951d03c8af2c9ce589a5667d041e5b137e529 on ruby-rdf:develop.

nvdk commented 8 years ago

as this has become a bit of a mess I split it up in two new pull requests, please review #67 and #68.