Restriction visitor rewrite supports most complex restrictions. Change most lists to sets. Some mapper updates to deal with those changes

[cweedall]

This PR will be very difficult to review. Apologies in advance for this!

I was looking through other issues and noticed issue #93 Range with complex objects (using other schemas) makes OBA not recognize all classes. I noticed the range for asociadaA was a unionOf two classes and it was being used at least once with exact cardinality. This got me started looking to see if I could further fix/improve complex restrictions.

The closer I got to it working correctly, the more I realized I was reusing/condensing a lot of the existing RestrictionVisitor code. I ended up with 3 helper methods to store the properties/restrictions in the Set and Map variables. Several visit types (e.g. OWLNaryBooleanClassExpression versus OWLObjectUnionOf and OWLObjectIntersectionOf) were virtually identical, except for the restriction values key. For all of these, I made helper methods and inferred the key based on the subinterface.

Also added comments and fixes where it became easier to identify issues due to code cleanup. For this re-write, it also eventually became clear that enums are basically class restrictions (rather than properties) and can be handled here also. The caveat is that the property is an empty string (because it's not a property!). This is treated as a special case check in MapperSchema, which builds the enum schema upon encountering this restriction.

There were some adjustments that needed to be made in Mapper and MapperSchema due to the RestrictionVisitor changes. Perhaps the biggest issue that I ran into when using the ontology from issue #93 Range with complex objects (using other schemas) makes OBA not recognize all classes, is that it didn't generate endpoints or models for classes based on the ontology's IRI. This may require some discussion.

Throughout, I updated most of the lists to sets. I did this partly by comparing examples from owlapi TutorialSnippetsTestCase and partly by realizing this allowed us to only pass the OWLClass object around and determine which ontology it was part of. (There may be a better way overall, but I was trying to make incremental improvements, including readability and code usability/write-ability).

Lastly, I encountered several things the appeared to be bugs. For example, data type nonPositiveInteger would map to the integer type in the spec - but if you have nonPositiveInteger and nonNegativeInteger then you'd have two (generic) integer entries in the items list; this completely misses the fact that it is basically saying "only zero is allowed".

"oneOf" references to enum values should just use the individual's short form (in my opinion) like the enum classes and not use format: uri and type: string, in order to align with the enums. But, if this is necessary, I am OK with using a configuration flag to flip between the two methods.

Unions and intersections appear to be working correctly now for me, but I was seeing bugs (i.e. missing OpenAPI spec details) in some cases, such as StudyMaterial from the example.owl file. Inheritance was also not working in some cases. For example, hasDegree was missing on Student, despite it existing on its superclass Person.

The only two issues that I am aware of from restrictions and schema models now:

1. Property nullability and requiredness need to be reviewed (to make sure nothing is wrong or missing).
2. Complex unions/intersections (e.g. the testObjectUnionOf in example.owl) create anyOf AND allOf arrays under items - each having all of the range values conflated together. (To fix this will require a re-write of RestrictionVisitor and the mapper classes and be a big undertaking.)

[dgarijo]

Thanks, @cweedall impressive work! I think we can just acknowledge the need for complex unions/intersections and leave it as a future issue. Nice catch about integers. In https://owl-to-oas.readthedocs.io/en/latest/mapping we wrote the whole owl 2 oas specification (even things that are currently not supported in OBA). I think we did not write some of these considerations you are discussing (e.g., zero when nonnegative and non positive integer restrictions apply).

[cweedall]

@dgarijo Thanks for reviewing.

I know you mentioned putting off complex unions/intersections, but I ended up finding a way to do it after posting this. I think I will orphan this branch, but leave it available for some time. I have new changes that are heavily based on this, but also include wrapping the MapperSchema class into the Visitor class. There was a lot of stumbling, but I found ways to make the code more readable and minimize repeated steps/loops.

Regarding the link, I had come across that previously. As I was comparing the coding work with ontologies, I came across instances that I was unsure about. There may be some others, but from what I can recall at the moment, examples included:

• why are hasValue and oneOf values the full URI, rather than just the short form name and listed as enums?
• similar question for the default value.
• why is inheritance handled in the repo by copying all properties to the sub/inheriting class? But it is done in the link via the following on the sub/inheriting class:

  allOf:
  - $ref: '#/components/schemas/SCHEMANAME'
  - type: object

I would like to better understand some of these details. And, if have made any mistakes it is definitely good to fix these! Some/all of these answer may be listed there or elsewhere in the literature(?) Unfortunately, I do not have a lot time to familiarize myself thoroughly with the literature and would greatly appreciate direction on where to look.

After I have time to wrap up some of my changes, mentioned at the top, I will post a new draft PR. Hopefully I can incorporate any problems/confusions that I had in this branch, and the questions here, into the new one. Thank you again for your patience with helping and reviewing!