epimorphics / elda

Epimorphics implementation of the Linked Data API
Other
53 stars 27 forks source link

Hyphenated prefix'es generate illegal SPARQL variables. #132

Open skwlilac opened 9 years ago

skwlilac commented 9 years ago

The 'shortname' def-bwp_pollutionRiskForecastStatement from the following URI reference:

http://ea-rbwd-staging.epimorphics.net/doc/bathing-water-profile?exists-def-bwp_pollutionRiskForecastStatement=true

propagates into an illegal SPARQL variable name ?___def-bwp_pollutionRiskForecastStatement_0 in the following elda generated SPARQL query.

I suspect this is a widespread problem wrt to the generation of SPARQL variable names.

PREFIX def-bw: <http://environment.data.gov.uk/def/bathing-water/>
PREFIX def-bwp: <http://environment.data.gov.uk/def/bathing-water-profile/>
PREFIX interval: <http://reference.data.gov.uk/def/intervals/>
PREFIX skos: <http://www.w3.org/2004/02/skos/core#>
PREFIX version: <http://environment.data.gov.uk/def/version/>
SELECT DISTINCT ?item
WHERE {

        ?item def-bw:bathingWater ?bw .
        ?item a def-bwp:BathingWaterProfile .
        ?bw skos:notation ?eubwid .
        ?item def-bwp:applicableYear ?year .
        ?year interval:ordinalYear ?oYear .
        ?item version:versionString ?vs .
    ?item def-bwp:pollutionRiskForecastStatement ?___def-bwp_pollutionRiskForecastStatement_0 .
}  ORDER BY STR(?eubwid) ?oYear ?vs OFFSET 0 LIMIT 10
ehedgehog commented 9 years ago

def-bwp_pollutionRiskForecastStatement is not a legal shortname, so the problem really is that Elda doesn't check that alleged property shortnames in URIs are legal.

A quick look at the code suggests that the Param constructor should check that the name is has been handed is a legal shortname and, if not, throw a Bad Request exception with a suitable message.

skwlilac commented 9 years ago

@prefix def-bwp: <http://environment.data.gov.uk/def/bathing-water-profile>

is a fine prefix to use in the preparation of data and an elda config. That it fall foul of the problem that it has a '-' within it is flaw in the design of {prefix}_{localName} as a way pass in property references when an explicit shortname setting is missing. Incidentally, well more than incidentally, the shortname makes it through correctly in-terms of property selection in the resulting SPARQL query. What is broken is the 'invented' variable name ?___def-bwp_pollutionRiskForecastStatement_0.

So I disagree with the course of action you propose. It is NOT a bad request. There are other places where hypenated short names are used (for the value part of filter matches) that are also (currently) unproblematic.

So... please just fix the SPARQL variable name generation.