-
Notifications
You must be signed in to change notification settings - Fork 641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-2460: move vocabularies :jena-ontapi -> :jena-core, add OWL1, mark OWL as deprecated #2461
Conversation
org.apache.jena.ontapi.vocabulary
(:jena-ontapi) -> `…* <p> | ||
* So for these cases, call this helper class: Init.function() | ||
*/ | ||
public static class Init { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pattern (Init inner class) should be unnecessary because initialization is more controlled by ServiceLoader
and InitJenaCore
.
I tried making Init
, class and functions, all private static
and it didn't seem to cause any problems.
Maybe it would be a good idea to either make this change or at least deprecate all uses of Init.*
so any user code using them is warned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made a separate commit with fixed OWL1, RDF, XSD vocabularies (after review I will squash commits)
@@ -178,6 +180,36 @@ public class XSD { | |||
/** Resource URI for xsd:gMonthDay */ | |||
public static Resource gMonthDay; | |||
|
|||
/** Property URI for xsd:length */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "Property" the right word here? These are constraining facets, which must be an IRI, so maybe Property is forced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant org.apache.jena.rdf.model.Property
, it is used as a predicate in facet restrictions
* | ||
* @see <a href="https://www.w3.org/TR/rdf-plain-literal">rdf:PlainLiteral: A Datatype for RDF Plain Literals (Second Edition)</a> | ||
*/ | ||
public final static Resource PlainLiteral = Init.PlainLiteral(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rdf:PlainLiteral should never appear in RDF data!
RDF plain literals should not be necessary for RDF 1.1 (all literals have a datatype).
RDF 1.1 does not have RDF 1.0 "plain literals" (strings + lang tag strings).
Probably worth adding this to the javadoc.
(I would describe it as "archaic" these days.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rdf:PlainLiteral
is used by jena-ontapi (https://github.com/apache/jena/blob/main/jena-ontapi/src/main/java/org/apache/jena/ontapi/common/OntVocabulary.java#L361), since it is present in the specification: https://www.w3.org/TR/owl2-quick-reference/#Built-in_Datatypes ( OWL 2 Web Ontology Language Quick Reference Guide )
Also, it is used by OWLAPI (and ONTAPI) https://github.com/owlcs/owlapi/blob/version5/impl/src/main/java/uk/ac/manchester/cs/owl/owlapi/InternalizedEntities.java#L44
/** RDF constants are used during Jena initialization. | ||
* <p> | ||
* If that initialization is triggered by touching the RDF class, | ||
* then the constants are null. | ||
* <p> | ||
* So for these cases, call this helper class: Init.function() | ||
*/ | ||
@Deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
public static Resource Nothing() { return resource( "Nothing" ); } | ||
} | ||
@Deprecated | ||
public class OWL extends OWL1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cases 424 warnings on the Jena code base!
200 are in jena-core:src/main/java:org.apache.jena.ontology
50 are in jena-core:src/main/java:org.apache.jena.reasoner
155 are in jena-core tests
Shouldn't these all be OWL1?
As vocabularies, is there anything OWL2 that conflicts with OWL1?
Would it work if we change all uses in Jena to be OWL1 where it must be OWL1 (legacy ontology, reasoner) then have public class OWL extends OWL2
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are 4 constants in OWL1 that are not in OWL2: NAMESPACE
, LITE_LANG
, DL_LANG
, FULL_LANG
we can do like this:
class OWL {
public static final Resource NAMESPACE = ResourceFactory.createResource(NS);
public static final Resource OWL1_FULL_LANG = ResourceFactory.createResource(getURI());
public static final Resource OWL1_DL_LANG = ResourceFactory.createResource("http://www.w3.org/TR/owl-features/#term_OWLDL");
public static final Resource OWL1_LITE_LANG = ResourceFactory.createResource("http://www.w3.org/TR/owl-features/#term_OWLLite");
@Deprecated
public static final Resource FULL_LANG = OWL1_FULL_LANG;
@Deprecated
public static final Resource DL_LANG = OWL1_DL_LANG;
@Deprecated
public static final Resource LITE_LANG = OWL1_LITE_LANG;
...
And make OWL2 deprecated to avoid confusion.
in this case we do not need OWL1 vocabulary at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This time:
- rename OWL => OWL1
- make new (alias) OWL extends OWL2
- no depreciation (so no new warnings should appear)
- jena-ontapi uses OWL2, jena-core & jena-arq uses OWL1
- change initialization of RDF, XSD, OWL, add some new constants (see above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this new initialization I observe error when run particular test through IDE:
java.lang.ExceptionInInitializerError
at org.apache.jena.riot.RDFParserRegistry.initStandard(RDFParserRegistry.java:74)
at org.apache.jena.riot.RDFParserRegistry.init(RDFParserRegistry.java:57)
at org.apache.jena.riot.RDFParserRegistry.<clinit>(RDFParserRegistry.java:52)
at org.apache.jena.riot.RIOT.init(RIOT.java:83)
at org.apache.jena.riot.system.InitRIOT.start(InitRIOT.java:29)
at org.apache.jena.base.module.Subsystem.lambda$initialize$1(Subsystem.java:117)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at org.apache.jena.base.module.Subsystem.forEach(Subsystem.java:193)
at org.apache.jena.base.module.Subsystem.forEach(Subsystem.java:169)
at org.apache.jena.base.module.Subsystem.initialize(Subsystem.java:115)
at org.apache.jena.sys.JenaSystem.init(JenaSystem.java:89)
at org.apache.jena.graph.NodeFactory.<clinit>(NodeFactory.java:39)
at org.apache.jena.rdf.model.impl.ResourceImpl.fresh(ResourceImpl.java:150)
at org.apache.jena.rdf.model.impl.ResourceImpl.<init>(ResourceImpl.java:86)
at org.apache.jena.rdf.model.ResourceFactory$Impl.createResource(ResourceFactory.java:308)
at org.apache.jena.rdf.model.ResourceFactory.createResource(ResourceFactory.java:94)
at org.apache.jena.vocabulary.RDF.resource(RDF.java:54)
at org.apache.jena.vocabulary.RDF.<clinit>(RDF.java:65)
at org.apache.jena.vocabulary.RDF$Nodes.<clinit>(RDF.java:193)
at org.apache.jena.ontapi.utils.Graphs.listOntologyNodes(Graphs.java:659)
at org.apache.jena.ontapi.utils.Graphs.ontologyNode(Graphs.java:626)
at org.apache.jena.ontapi.utils.Graphs.ontologyNode(Graphs.java:613)
at org.apache.jena.ontapi.utils.Graphs.findOntologyNameNode(Graphs.java:561)
at org.apache.jena.ontapi.utils.Graphs.findOntologyNameNode(Graphs.java:544)
at org.apache.jena.ontapi.impl.repositories.PersistentGraphRepository.lambda$baseOntGraphs$2(PersistentGraphRepository.java:64)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1707)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
at org.apache.jena.ontapi.impl.repositories.PersistentGraphRepository.collectUnions(PersistentGraphRepository.java:53)
at org.apache.jena.ontapi.impl.repositories.PersistentGraphRepository.<init>(PersistentGraphRepository.java:49)
at org.apache.jena.ontapi.PersistentGraphRepositoryTest.testSeveralRepositories(PersistentGraphRepositoryTest.java:66)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.lang.NullPointerException: Cannot invoke "org.apache.jena.rdf.model.Resource.getURI()" because "org.apache.jena.vocabulary.RDF.xmlLiteral" is null
at org.apache.jena.riot.lang.ReaderTriX.<clinit>(ReaderTriX.java:119)
... 38 more
It can be fixed by removing static { JenaSystem.init(); }
in NodeFactory
:
So should I remove this line or return back that sophisticated initialization? I think the better solution is to remove JenaSystem.init()
, as it is not required for NodeFactory to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see PersistentGraphRepositoryTest
in the PR code base - I have this pulled PR as of commit 648852b. The warnings count looks the same as before.
1/
The situation in the stacktrace usually means the code is already inside JenaSystem.init
.
There is call to JenaSystem.init
here because code may use NodeFactory
as a first call into Jena.
tests suite can sometimes need their own JenaSystem.init
because they do tend to go straight the non-public code they are testing.
PersistentGraphRepository
looks like a entry point (a place where a first call happens) - maybe it needs static {
JenaSystem.init(): }`
Calling JenaSystem.init
should always be safe. I can't see from at the stacktrace how the code got to org.apache.jena.riot.lang.ReaderTriX.<clinit>(ReaderTriX.java:119)
.
PersistentGraphRepositoryTest
may need a call to JenaSystem.init
. If that does not help, then, yes, this may need to go back to the old way of initializing. which was for this this case but IIRC before ServiceLoader came along.
2/
This isn't the cause - it may help generally.
jena-ontapi does not have a src/main/resources/META-INF/services/org.apache.jena.sys.JenaSubsystemLifecycle
file.
See jena-core
for an example. Having one of these, controls the initlialization order in Jena so it can go after jena-core before jena-arq at level 15 (see JenaSystem
for details).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PersistentGraphRepositoryTest is here #2496 - it is another independent PR
Can you please give me an example of warning. This is strange, since in core and arq there are only renaming (now).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1d3a9f2
to
648852b
Compare
34515a7
to
0d7a033
Compare
@@ -0,0 +1,3 @@ | |||
org.apache.jena.riot.system.InitRIOT | |||
org.apache.jena.sparql.system.InitARQ | |||
org.apache.jena.rdfs.sys.InitRDFS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should not be here. They are in other ServiceLoader files and wil be run from there. That might be the initialization problem. Putting them here makes them get called twice.
Here, give a class in this module:
org.apache.jena.ontapi.sys.InitOntAPI
then new package and class:
org.apache.jena.ontapi.sys.InitOntAPI
public class InitOntAPI implements JenaSubsystemLifecycle {
@Override
public void start() {
// Any initialization for this module. This may be nothing.
}
@Override
public void stop() {}
@Override
public int level() {
return 15 ; -- this is between jena-core and jena-arq RIOT.
}
}
If you have no preference a to PR order, let's get them in 2461 then 2496 (including any changes to the code in 2461 that 2496 shows are needed. Is this branch ready now? |
I think it is ready |
public static final Node nodeOwlSameAs = OWL.sameAs.asNode(); // uri("http://www.w3.org/2002/07/owl#sameAs") | ||
public static final Node nodeOwlImports = OWL1.imports.asNode(); // uri("http://www.w3.org/2002/07/owl#imports") | ||
public static final Node nodeOwlOntology = OWL1.Ontology.asNode(); // uri("http://www.w3.org/2002/07/owl#Ontology") | ||
public static final Node nodeOwlSameAs = OWL1.sameAs.asNode(); // uri("http://www.w3.org/2002/07/owl#sameAs") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these be OWL
- they are OWL1/OWL2 neutral.
This happens quite a lot in this PR.
If that's the case, then OWL1 where it iued for constants can be converted to OWL as a separate later PR.
Otherwise, it looks good to go. I mainly checked the code outside jena-ontapi.
…) -> `org.apache.jena.vocabulary` (:jena-core) & invite OWL1 voc; change initialization of OWL1, RDF, XSD vocabularies
GitHub issue resolved #2460
Pull request Description:
org.apache.jena.ontapi.vocabulary
(:jena-ontapi) ->org.apache.jena.vocabulary
(:jena-core)OWL extends OWL1
)By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.
See the Apache Jena "Contributing" guide.