Skip to content
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

Merged
merged 1 commit into from
May 26, 2024

Conversation

sszuev
Copy link
Contributor

@sszuev sszuev commented May 9, 2024

GitHub issue resolved #2460

Pull request Description:

  • move org.apache.jena.ontapi.vocabulary (:jena-ontapi) -> org.apache.jena.vocabulary (:jena-core)
  • invite OWL1 voc
  • mark OWL voc as deprecated (now OWL extends OWL1)

  • Tests are included.
  • Documentation change and updates are provided for the Apache Jena website
  • Commits have been squashed to remove intermediate development commit messages.
  • Key commit messages start with the issue number (GH-xxxx)

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.

@sszuev sszuev changed the title GH-2460: move org.apache.jena.ontapi.vocabulary (:jena-ontapi) -> `… GH-2460: move vocabularies :jena-ontapi -> :jena-core, add OWL1, mark OWL as deprecated May 9, 2024
* <p>
* So for these cases, call this helper class: Init.function()
*/
public static class Init {
Copy link
Member

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.

Copy link
Contributor Author

@sszuev sszuev May 12, 2024

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 */
Copy link
Member

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.

Copy link
Contributor Author

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();
Copy link
Member

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.)

Copy link
Contributor Author

@sszuev sszuev May 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/** 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
Copy link
Member

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 {
Copy link
Member

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

@sszuev sszuev May 24, 2024

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)

Copy link
Contributor Author

@sszuev sszuev May 25, 2024

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:

image

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.

Copy link
Member

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).

Copy link
Contributor Author

@sszuev sszuev May 26, 2024

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).

Copy link
Contributor Author

@sszuev sszuev May 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added src/main/resources/META-INF/services/org.apache.jena.sys.JenaSubsystemLifecycle with content:

org.apache.jena.riot.system.InitRIOT
org.apache.jena.sparql.system.InitARQ
org.apache.jena.rdfs.sys.InitRDFS

image

Adding JenaSystem.init(); in additional to service init - resolves the issue.

@@ -0,0 +1,3 @@
org.apache.jena.riot.system.InitRIOT
org.apache.jena.sparql.system.InitARQ
org.apache.jena.rdfs.sys.InitRDFS
Copy link
Member

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.
    }
}

@afs
Copy link
Member

afs commented May 26, 2024

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?

@sszuev
Copy link
Contributor Author

sszuev commented May 26, 2024

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")
Copy link
Member

@afs afs May 26, 2024

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
@afs afs merged commit 1ddfdf1 into apache:main May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

moving/renaming :jena-ontapi vocabularies.
2 participants