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

Consider raising Java version requirement to 17 or 21 to allow for algebraic data types #1101

Open
julesjacobsen opened this issue May 3, 2023 · 5 comments

Comments

@julesjacobsen
Copy link

Hi, I know this is probably a long shot given the downstream dependencies but as you're probably aware Java 17 has been released for at least a year now and is the latest LTS release (soon to be superseded by 21). However, the benefits it brings to the OWLAPI are sealed classes and pattern matching for switch which allows some much cleaner code both internally and for consumers.

Maybe for owlapi version 6 or 7?

For example the OWLAnnotationValue would benefit from being sealed

/**
 * A marker interface for annotation values, which can either be an IRI (URI), Literal or Anonymous
 * Individual, with visitor methods.
 * 
 * @see org.semanticweb.owlapi.model.IRI
 * @see org.semanticweb.owlapi.model.OWLLiteral
 * @see org.semanticweb.owlapi.model.OWLAnonymousIndividual
 * @author Matthew Horridge, The University of Manchester, Information Management Group
 * @since 3.0.0
 */
public interface OWLAnnotationValue extends OWLAnnotationObject, OWLPrimitive {

    /**
     * @param visitor visitor to accept
     */
    void accept(@Nonnull OWLAnnotationValueVisitor visitor);

    /**
     * @param visitor visitor to accept
     * @param <O> visitor return type
     * @return visitor value
     */
    @Nonnull
    <O> O accept(@Nonnull OWLAnnotationValueVisitorEx<O> visitor);

    /**
     * @return if the value is a literal, return an optional containing it. Return Optional.absent
     *         otherwise.
     */
    @Nonnull
    Optional<OWLLiteral> asLiteral();

    /**
     * @return if the value is an IRI, return an optional containing it. Return Optional.absent
     *         otherwise.
     */
    @Nonnull
    Optional<IRI> asIRI();

    /**
     * @return if the value is an anonymous, return an optional containing it. Return
     *         Optional.absent otherwise.
     */
    @Nonnull
    Optional<OWLAnonymousIndividual> asAnonymousIndividual();

    /**
     * @return true if the annotation value is a literal
     */
    default boolean isLiteral() {
        return false;
    }
}

would become

/**
 * A marker interface for annotation values, which can either be an IRI (URI), Literal or Anonymous
 * Individual, with visitor methods.
 * 
 * @see org.semanticweb.owlapi.model.IRI
 * @see org.semanticweb.owlapi.model.OWLLiteral
 * @see org.semanticweb.owlapi.model.OWLAnonymousIndividual
 * @author Matthew Horridge, The University of Manchester, Information Management Group
 * @since 3.0.0
 */
public sealed interface OWLAnnotationValue permits IRI, OWLLiteral, OWLAnonymousIndividual extends OWLAnnotationObject, OWLPrimitive {

}

Where did the visitor pattern accept methods go? This isn't needed with sealed classes and pattern matching for switch. See NipaFX's nice explanation. Also no need to read the comments about what interfaces are permitted.

Similarly the OWLAnnotationSubject

/**
 * A marker interface for annotation subjects, which can either be IRIs or
 * anonymous individuals, with visitor methods.
 * 
 * @author Matthew Horridge, The University of Manchester, Information
 *         Management Group
 * @since 3.0.0
 */
public interface OWLAnnotationSubject extends OWLAnnotationObject, OWLPrimitive {

becomes sealed and permits IRI and OWLAnonymousIndividual

/**
 * A marker interface for annotation subjects, which can either be IRIs or
 * anonymous individuals, with visitor methods.
 * 
 * @author Matthew Horridge, The University of Manchester, Information
 *         Management Group
 * @since 3.0.0
 */
public sealed interface OWLAnnotationSubject permits IRI, OWLAnonymousIndividual extends OWLAnnotationObject, OWLPrimitive {

and so would OWLAnnotationAxiom (would permits OWLAnnotationAssertionAxiom, OWLAnnotationPropertyDomainAxiom, OWLAnnotationPropertyRangeAxiom , OWLSubAnnotationPropertyOfAxiom)
which would allow current code like this (taken from obographs):

if (ax instanceof OWLAnnotationAssertionAxiom) {
    OWLAnnotationAssertionAxiom aaa = (OWLAnnotationAssertionAxiom) ax;
    OWLAnnotationProperty p = aaa.getProperty();
    OWLAnnotationSubject s = aaa.getSubject();

    // non-blank nodes
    if (s instanceof IRI) {
        String subj = getNodeId((IRI) s);

        OWLAnnotationValue v = aaa.getValue();
        
       /// lots of other if tests
     else {
        String val;
        if (v instanceof IRI) {
            val = ((IRI) v).toString();
        } else if (v instanceof OWLLiteral) {
            val = ((OWLLiteral) v).getLiteral();
        } else if (v instanceof OWLAnonymousIndividual) {
            val = ((OWLAnonymousIndividual) v).getID().toString();
        } else {
            val = "";
        }

        BasicPropertyValue basicPropertyValue = new BasicPropertyValue.Builder()
                .pred(getPropertyId(p))
                .val(val)
                .build();

        oboGraphBuilder.addNodeBasicPropertyValue(subj, basicPropertyValue);
    }  else {
        // subject is anonymous
        oboGraphBuilder.addUntranslatedAxiom(aaa);
    }
}

to become a lot less noisy and more succinct:

// 'new' Java 16 - Pattern Matching for instanceof: https://openjdk.org/jeps/394
if (ax instanceof OWLAnnotationAssertionAxiom aaa) {
    OWLAnnotationProperty p = aaa.getProperty();
    OWLAnnotationSubject s = aaa.getSubject();

    // non-blank nodes, switch on sealed class OWLAnnotationSubject allows for compiler-checked cases instead of
    // unchecked if ... else
    // OK, maybe in Java 21 the next LTS release - Pattern Matching for switch: https://openjdk.org/jeps/441 
    switch (s) {
        case IRI sIRI -> {
                String subj = getNodeId(sIRI);
        
                OWLAnnotationValue v = aaa.getValue();
               
                /// lots of other if tests
                String val = switch (v) {
                        case IRI iri -> iri.toString();
                        case OWLLiteral owlLiteral -> owlLiteral.getLiteral();
                        case OWLAnonymousIndividual anon -> anon.getID().toString();
                } // no need for guard or default as these are the only allowed options for the sealed interface 
                // and the compiler will check this.
        
                BasicPropertyValue basicPropertyValue = new BasicPropertyValue.Builder()
                        .pred(getPropertyId(p))
                        .val(val)
                        .build();
        
                oboGraphBuilder.addNodeBasicPropertyValue(subj, basicPropertyValue);
        };
        case OWLAnonymousIndividual anon -> oboGraphBuilder.addUntranslatedAxiom(aaa);
    }
}
@ignazio1977
Copy link
Contributor

@julesjacobsen Indeed, version 6 will require Java 17. I've not started work on it yet, but that's the last important change before a release candidate for it.
Version 5.5.0 requires Java 11 now, which is a first step up.

@julesjacobsen
Copy link
Author

Glad to hear this is on the cards. It's unfortunate that pattern matching for switch isn't final in 17, but I'm guessing you'll be targeting a post-17 LTS (Java 21?) for owlapi version 7 where this could all come together?

@ignazio1977
Copy link
Contributor

That's more or less the plan, unless the version 6 release candidate ends up coming out when V21 becomes available.

@jamesamcl
Copy link

I can't actually get OWLAPI working with Java 17:

java.lang.reflect.InaccessibleObjectException: Unable to make protected final java.lang.Class java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int,java.security.ProtectionDomain) throws java.lang.ClassFormatError accessible: module java.base does not "opens java.lang" to unnamed module @47f37ef1
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354) ~[na:na]
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297) ~[na:na]
        at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199) ~[na:na]
        at java.base/java.lang.reflect.Method.setAccessible(Method.java:193) ~[na:na]
        at com.google.inject.internal.cglib.core.$ReflectUtils$1.run(ReflectUtils.java:52) ~[guice-4.1.0.jar!/:na]
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:318) ~[na:na]
        at com.google.inject.internal.cglib.core.$ReflectUtils.<clinit>(ReflectUtils.java:42) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.cglib.reflect.$FastClass$Generator.getProtectionDomain(FastClass.java:73) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.cglib.core.$AbstractClassGenerator.create(AbstractClassGenerator.java:206) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.cglib.reflect.$FastClass$Generator.create(FastClass.java:65) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.BytecodeGen.newFastClassForMember(BytecodeGen.java:252) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.BytecodeGen.newFastClassForMember(BytecodeGen.java:203) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.ProviderMethod.create(ProviderMethod.java:69) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.ProviderMethodsModule.createProviderMethod(ProviderMethodsModule.java:275) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.ProviderMethodsModule.getProviderMethods(ProviderMethodsModule.java:144) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.ProviderMethodsModule.configure(ProviderMethodsModule.java:123) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.spi.Elements$RecordingBinder.install(Elements.java:340) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.spi.Elements$RecordingBinder.install(Elements.java:349) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.spi.Elements.getElements(Elements.java:110) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.InjectorShell$Builder.build(InjectorShell.java:138) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:104) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.Guice.createInjector(Guice.java:99) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.Guice.createInjector(Guice.java:73) ~[guice-4.1.0.jar!/:na]
        at com.google.inject.Guice.createInjector(Guice.java:62) ~[guice-4.1.0.jar!/:na]
        at org.semanticweb.owlapi.apibinding.OWLManager.createInjector(OWLManager.java:104) ~[owlapi-distribution-5.1.0.jar!/:5.1.0.2017-03-29T23:31:42Z]
        at org.semanticweb.owlapi.apibinding.OWLManager.createOWLOntologyManager(OWLManager.java:44) ~[owlapi-distribution-5.1.0.jar!/:5.1.0.2017-03-29T23:31:42Z]

@ignazio1977
Copy link
Contributor

@udp Version 5.1.0 was still using Guice, that's not been the case for quite a while. I've not tried running version 5.1.0 with Java newer than 8. Version 5.5.0 is built with Java 11, including tests that exercise this code path. Some of the most recent 5.1.x versions should work as well.

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

No branches or pull requests

3 participants