You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2020/09/04 14:32:41 UTC
[GitHub] [jena] afs commented on a change in pull request #792: JENA-1959 Added JsonLdReadContext
afs commented on a change in pull request #792:
URL: https://github.com/apache/jena/pull/792#discussion_r483640231
##########
File path: jena-arq/src/main/java/org/apache/jena/riot/JsonLDReadContext.java
##########
@@ -0,0 +1,74 @@
+/**
Review comment:
Should be `/*`. (Yes - some other files have `/**` and need fixing)
##########
File path: jena-arq/src/main/java/org/apache/jena/riot/JsonLDReadContext.java
##########
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.riot;
+
+import com.github.jsonldjava.core.JsonLdOptions;
+import org.apache.jena.atlas.web.ContentType;
+import org.apache.jena.riot.lang.JsonLDReader;
+import org.apache.jena.riot.system.StreamRDF;
+import org.apache.jena.sparql.util.Context;
+
+import java.io.InputStream;
+
+/**
+ * Set of parameters that can be used to control the reading of JSON-LD.
+ *
+ * This class provides setters to define a "Context" suitable to be passed as
+ * last argument to {@link ReaderRIOT#read(InputStream, String, ContentType, StreamRDF, Context)}
+ * when the ReaderRIOT has been created with one of the JSON-LD RDFFormat variants (that is, when it is
+ * an instance of {@link JsonLDReader})
+ *
+ * Parameters that are actually useful are ''documentLoader'' and ''produceGeneralizedRdf''.
+ *
+ */
+public class JsonLDReadContext extends Context {
Review comment:
OK - I see the intention here but subclassing isn't going to work. This is because `Context` is also used in places where it is a container of settings from different subsystems - e.g. `RIOT.getContext()` is a global system place to have context settings. A subclass doesn't because it can't be shared.
Instead, have a library class `JsonLD` with static functions.
```
public static void setOptions(Context cxt, JsonLdOptions opts) {
cxt.set(JsonLDReader.JSONLD_OPTIONS, opts);
}
```
##########
File path: jena-arq/src/test/java/org/apache/jena/riot/TestJsonLDReader.java
##########
@@ -1,4 +1,4 @@
-/*
+/**
Review comment:
`/*`
##########
File path: jena-arq/src/main/java/org/apache/jena/riot/RIOT.java
##########
@@ -109,13 +109,6 @@ public static String getBuildDate() {
// ---- Symbols
- /**
- * Symbol to use to pass (in a Context object) the "@context" to be used when reading jsonld
- * (overriding the actual @context in the jsonld)
- * Expected value: the value of the "@context",
- * as expected by the JSONLD-java API (a Map) */
- public static final Symbol JSONLD_CONTEXT = Symbol.create("http://jena.apache.org/riot/jsonld#JSONLD_CONTEXT");
-
Review comment:
RIOT holds the public constants so an app does not need to import class `JsonLDReader` (which is in a subpackage).
It is possibly used by existing Jena applications so there is a compatibility issue here.
While not a perfect way to achieve a tidy separate of public and implementation I think we ought to keep the current style (and for `JSONLD_OPTONS`) unless there is a specific reason for this change.
##########
File path: jena-arq/src/main/java/org/apache/jena/riot/lang/JsonLDReader.java
##########
@@ -112,8 +130,21 @@ public void read(InputStream in, String baseURI, ContentType ct, StreamRDF outpu
IO.exception(e) ;
}
}
-
- private void read$(Object jsonObject, String baseURI, ContentType ct, final StreamRDF output, Context context) {
+
+ private void readWithJsonLDCtxOptions(Object jsonObject, String baseURI, final StreamRDF output, Context context) throws JsonParseException, IOException {
+ JsonLdOptions options = getJsonLdOptions(baseURI, context) ;
+ Object jsonldCtx = getJsonLdContext(context);
+ if (jsonldCtx != null) {
+ if (jsonObject instanceof Map) {
+ ((Map) jsonObject).put("@context", jsonldCtx);
Review comment:
I'm getting warnings on lines 114, 139: these should be suppressed (it makes it easier to see when new warnings appear in the codebase).
```
Map is a raw type. References to generic type Map<K,V> should be parameterized
JsonLDReader.java /jena-arq/src/main/java/org/apache/jena/riot/lang line 139
Type safety: The method put(Object, Object) belongs to the raw type Map. References to generic type Map<K,V> should be parameterized
JsonLDReader.java /jena-arq/src/main/java/org/apache/jena/riot/lang line 139
Unnecessary @SuppressWarnings("rawtypes")
JsonLDReader.java /jena-arq/src/main/java/org/apache/jena/riot/lang line 114
Unnecessary @SuppressWarnings("unchecked")
JsonLDReader.java /jena-arq/src/main/java/org/apache/jena/riot/lang line 114
```
##########
File path: jena-arq/src/test/java/org/apache/jena/riot/TestJsonLDReader.java
##########
@@ -18,127 +18,123 @@
package org.apache.jena.riot;
-import static org.junit.Assert.assertTrue;
-
-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.nio.charset.StandardCharsets;
-
import com.fasterxml.jackson.core.JsonGenerationException;
+import com.github.jsonldjava.core.DocumentLoader;
+import com.github.jsonldjava.core.JsonLdOptions;
import com.github.jsonldjava.utils.JsonUtils;
-
-import org.apache.jena.query.Dataset;
-import org.apache.jena.query.DatasetFactory;
import org.apache.jena.rdf.model.Model;
import org.apache.jena.rdf.model.ModelFactory;
+import org.apache.jena.riot.lang.JsonLDReader;
import org.apache.jena.riot.system.ErrorHandlerFactory;
import org.apache.jena.sparql.util.Context;
import org.apache.jena.vocabulary.RDF;
import org.junit.Test;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.Assert.assertTrue;
+
public class TestJsonLDReader {
@Test
- public final void simpleReadTest() {
- try {
- String jsonld = someSchemaDorOrgJsonld();
- Model m = ModelFactory.createDefaultModel();
- RDFParser.create()
- .errorHandler(ErrorHandlerFactory.errorHandlerNoLogging)
- .fromString(jsonld)
- .lang(Lang.JSONLD)
- .parse(m);
- assertJohnDoeIsOK(m);
- } catch (RiotException e) {
- // cf. org.apache.jena.riot.RiotException: loading remote context failed: http://schema.org/
- // There's a line printed anyway
- // e.printStackTrace();
- }
+ public final void simpleReadTest() throws IOException {
+ String jsonld = someSchemaDorOrgJsonld();
+ Model m = jsonld2model(jsonld, null);
+ assertJohnDoeIsOK(m);
}
- /** Test using the jena Context mechanism to pass the jsonld "@context" */
- @Test public final void overrideAtContextTest() throws JsonGenerationException, IOException {
+ /**
+ * Test using the jena Context mechanism to pass the jsonld "@context"
+ */
+ @Test
+ public final void overrideAtContextTest() throws JsonGenerationException, IOException {
// some jsonld using schema.org's URI as "@context"
String jsonld = someSchemaDorOrgJsonld();
// a subset of schema.org that can be used as @context for jsonld
- String jsonldContext = "{\"name\":{\"@id\":\"http://schema.org/name\"},\"Person\": {\"@id\": \"http://schema.org/Person\"}}";
+ String jsonldContext = "{\"name\":{\"@id\":\"http://schema.org/name\", \"@type\": \"http://www.w3.org/2001/XMLSchema#other\"},\"Person\": {\"@id\": \"http://schema.org/Person\"}}";
// pass the jsonldContext to the read using a jena Context
Context jenaCtx = new Context();
Object jsonldContextAsMap = JsonUtils.fromInputStream(new ByteArrayInputStream(jsonldContext.getBytes(StandardCharsets.UTF_8)));
- jenaCtx.set(RIOT.JSONLD_CONTEXT, jsonldContextAsMap);
+ jenaCtx.set(JsonLDReader.JSONLD_CONTEXT, jsonldContextAsMap);
// read the jsonld, replacing its "@context"
- Dataset ds = jsonld2dataset(jsonld, jenaCtx);
+ Model m = jsonld2model(jsonld, jenaCtx);
// check ds is correct
- assertJohnDoeIsOK(ds.getDefaultModel());
+ assertJohnDoeIsOK(m);
}
- /** Not really useful, but one can replace the @context by a URI: in this case, this URI is used when expanding the json
- * (letting JSON-LD java API taking care of downloading the context. */
- // well, as of this writing, it doesn't work, as we get a "loading remote context failed"
- // But it is about the replacing URI, not the replaced one, showing that the mechanism does work
- @Test public final void overrideAtContextByURITest() throws JsonGenerationException, IOException {
+ /**
+ * Not really useful, but one can replace the @context by a URI: in this case, this URI is used when expanding the json
+ * (letting JSON-LD java API taking care of downloading the context.
+ */
+ @Test
+ public final void overrideJsonLdOptionsAndContextUri() throws JsonGenerationException, IOException {
// some jsonld using a (fake) pseudo.schema.org's URI as "@context"
String jsonld = "{\"@id\":\"_:b0\",\"@type\":\"Person\",\"name\":\"John Doe\",\"@context\":\"http://pseudo.schema.org/\"}";
// a subset of schema.org that can be used as @context for jsonld
String jsonldContext = "\"http://schema.org\"";
- // pass the jsonldContext to the read using a jena Context
- Context jenaCtx = new Context();
- Object jsonldContextAsObject = JsonUtils.fromInputStream(new ByteArrayInputStream(jsonldContext.getBytes(StandardCharsets.UTF_8)));
- jenaCtx.set(RIOT.JSONLD_CONTEXT, jsonldContextAsObject);
-
- try {
- // read the jsonld, replacing its "@context"
- Dataset ds = jsonld2dataset(jsonld, jenaCtx);
-
- // check ds is correct
- assertJohnDoeIsOK(ds.getDefaultModel());
- } catch (RiotException e) {
- // cf. org.apache.jena.riot.RiotException: loading remote context failed: http://schema.org/
- // There's a line printed anyway
- // e.printStackTrace();
- }
+ JsonLdOptions options = new JsonLdOptions();
+ DocumentLoader dl = new DocumentLoader();
+ dl.addInjectedDoc("http://schema.org", String.format("{%s}", schemaOrgContext()));
+ options.setDocumentLoader(dl);
+
+ // pass the jsonldContext and JsonLdOptions to the read using a jena Context
+ JsonLDReadContext jenaCtx = new JsonLDReadContext();
+ jenaCtx.setJsonLDContext(jsonldContext);
+ jenaCtx.setOptions(options);
+
+ // read the jsonld, replacing its "@context"
+ Model m = jsonld2model(jsonld, jenaCtx);
+
+ // check ds is correct
+ assertJohnDoeIsOK(m);
}
//
//
//
- /**
- * Reading some jsonld String, using a Context
- * @return a new Dataset
- * @throws IOException
- */
- private Dataset jsonld2dataset(String jsonld, Context jenaCtx) throws IOException {
Review comment:
JSON-LD is a dataset format and can encode datasets (quads) as well as a single graph.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org