-
Notifications
You must be signed in to change notification settings - Fork 39
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
Required IdType fixes #471
base: master
Are you sure you want to change the base?
Conversation
Failing examples must be fixed. Already checked with the cardinality in the documentation. |
</xsd:annotation> | ||
<!-- TODO: Matthias |
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 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.
What we try to achieve here is that the attribute id gets overridden later with a specific ObjectIdType for that specific element. Theoretically we could add a regular expression then that would guard that the object is named like "codespace:ScheduledStopPoint:something". Not my intention now, but building the relationship between Ref and Id is.
…TEx-CEN/NeTEx into fix_attribute_id_use_required
@@ -483,7 +483,7 @@ Rail transport, Roads and Road transport | |||
</xsd:extension> | |||
</xsd:complexContent> | |||
</xsd:complexType> | |||
<xsd:element name="VehicleEntrance" abstract="true" substitutionGroup="Entrance"> | |||
<xsd:element name="VehicleEntrance" abstract="true" substitutionGroup="SiteComponent"> |
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 kills the method resolution order (MRO) of the schema. Hence VehicleEntrance will be at the same level as Entrance, while what we want is having the inheritance.
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 think we have a problem already, because Entrance comes from SITE and VehicleEntrance probably should not be a SITE. Or do I read this wrong?
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.
It also might be one of the cases where we need an Entrance_ . Because having to restrictions in a row is not allowed in the inheritance. Should I try to model an Entrance_? According to one of the examples?
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.
Or we should do the inheritance through the VersionStructure elements. I think that only in the "leaves" the id stuff can be done once.
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 think this is a case where Entrance_ would make sense. In this situation I also want to figure out why (or not) Entrance_ may (or may not) be abstract.
…TableColumn has a required order element
fixed all assigned tasks |
@@ -37,7 +37,7 @@ | |||
<xsd:element name="points" minOccurs="0"> | |||
<xsd:complexType> | |||
<xsd:sequence> | |||
<xsd:element ref="Point" maxOccurs="unbounded"/> | |||
<xsd:element ref="Point_" maxOccurs="unbounded"/> |
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.
Are you sure about these changes? It feels like you are referencing to 'abstract' datatypes / 'containers'.
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 am an engineer (not true): It works... And it seems to me that they did it that way on other xxx_ elements. What it does it references the "concrete" element that is used in the substitutionGroup. Otherwise it wouldn't work. But to be sure we would need a comment from Nick or Christophe.
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 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 is a bit different from the topic of the PR ;-) but technically should be Ok: I think that this is replacing a ref to anything inheriting from Point to anything being of in the Point_ substitution group. I didn't check the possible semantic impact (yet).
I'm not sure that we can be so systematic ! A commented in other PR, I agree that any standalone object that can be referenced should be enforced to have an Id, but when it's an object that is embedded in the XML hierarchy and that will never be referenced, this constraint does not really makes sense (also it is harmless ... except that it brakes existing data). |
@ue71603 in essence this is the problem with order in skinkie/reference#65 so if we on one hand have defined it as key element and thus compound key, and in the XML Schema it is optional. That is never going to work. |
In my view objects have id. One does not know if it will be referenced. They also may not be generated at the same time in the same process. |
This pull request tries to address the missing required "id" attributes. This is selected by:
I may have added too many, which I certainly will review. For the following objects there are IdTypes defined, but there is no individual id attribute modelled, this should still be done.
PassingTimeView and PassengerCarryingRequirementsView are likely a bug in the schema that introduces them in the GeneralFrame. For PassingTimeView a DataManagedObjectView is first introduced, and never used after. For PassengerCarryingRequirementsView in my guess invalidly a DataManagedObject is used (see: #474)