IDS icon indicating copy to clipboard operation
IDS copied to clipboard

Reasoning behind maxOccurs="unbounded" on sequence in requirementsType

Open NiklasPor opened this issue 1 year ago • 8 comments

As many of you probably know, the order in which facets appears in applicability is predefined. On the first glance this is the same for the requirements, but it's not as easy as it appears.

// Excerpt from ids.xsd
  <xs:sequence maxOccurs="unbounded">
      <xs:element name="entity" minOccurs="0">...</xs:element>
      <xs:element name="partOf" minOccurs="0" maxOccurs="unbounded"></xs:element>
      <xs:element name="classification" minOccurs="0" maxOccurs="unbounded"></xs:element>
      <xs:element name="attribute" minOccurs="0" maxOccurs="unbounded">...</xs:element>
      <xs:element name="property" minOccurs="0" maxOccurs="unbounded">...</xs:element>
      <xs:element name="material" minOccurs="0" maxOccurs="unbounded">...</xs:element>
  </xs:sequence>

Notice that on the xs:sequence we have maxOccurs="unbounded" this enables the sequence to be repeated as many times as we want to. Combining this with optional elements inside the sequence, it's now possible to have ids files with orders which look different, but still are compliant.

This makes it not only possible to repeat the entity facet more than once inside the requirements, but also enables the user to ignore the ordering of the xs:sequence:

<ids:requirements>
	<ids:property>...</ids:property>
	<ids:material>...</ids:material>
	<ids:partOf>...</ids:partOf>
	<ids:material>...</ids:material>
	<ids:partOf>...</ids:partOf>
</ids:requirements>

Audit runs fine:

=== ids-tool - utility tool for buildingSMART IDS files.
info: idsTool.Program[0] Auditing: Ids structure, Ids content.
info: idsTool.Program[0] Auditing file: `/Users/niklaspor/Documents/GitHub/IDS-Audit-tool/ordering-test.ids`.
info: idsTool.Program[0] The file schema version is: Ids1_0
info: idsTool.Program[0] Completed reading 69 xml elements.
info: idsTool.Program[0] Completed with status: Ok.

ordering-test.ids.zip


I think this is rather confusing for the implementer / user. Inside the applicabilityType the order is required and enforced. Inside the requirementsType it look on the first glance like it's supposed to be ordered (xs:sequence) while in reality you can use any order.

As it was an explicit decision to use xs:sequence to "produce the text version of the content in a reliable and consistent way" (comment from @CBenghi ) I think it would be favorable to remove the maxOccurs="unbounded" from the requirementsType once the next breaking & major version of the schema is released.

If I'm missing any reason for the maxOccurs="unbounded" please tell me, I could not find anything while searching through issues & docs. Thanks for taking the time to read through this 👋

NiklasPor avatar Sep 14 '24 09:09 NiklasPor

Don't we need maxOccurs="unbounded" so we can have test multiple propeties, attributes etc in a single specification? For entityType is clearly makes no sense as an entity can only be one.

I personally feel we should be replacing all xs:sequences with xs:any. I can see no benefit in enforcing the sequencing of the nodes? My recollection is the decision related to 'implementor simplicity' but as this shows it's creating other issues. (not to mention folks who may edit IDS in an editor without XSD support)

andyward avatar Sep 14 '24 18:09 andyward

  1. I don't think we need maxOccurs="unbounded" on the sequence, because all elements inside the sequence have maxOccurs="unbounded" themselves. So they can already be repeated, but just have to be placed after each other. Check the excerpt from ids.xsd at the top. The only element without maxOccurs="unbounded" is the entity facet – so this seems intended. (This is also how it works inside the applicabilityType where the sequence does not have maxOccurs="unbounded".

  2. Yes I'd also favor xs:any. Take a look at https://github.com/buildingSMART/IDS/issues/175#issuecomment-1744634900 where this topic was already discussed. (Although I'd still favor xs:any over xs:sequence)

NiklasPor avatar Sep 14 '24 18:09 NiklasPor

Ah yes - I mis-read and thought you meant the maxOcurs on the elements. I agree the maxOccurs on the xs:sequence doesn't make sense - unless I'm missing something.

andyward avatar Sep 14 '24 20:09 andyward

We had this discussion before. My XSD validator (xmlspy) says 'any' is not valid XSD in that situation. Happy to take suggestions to change this, as long as XSD stays valid XSD

berlotti avatar Sep 16 '24 07:09 berlotti

@berlotti The xs:any is just a side note, what about the maxOccurs=unbounded on the requirementsType ?

// Update, I think if we wanted to replace the xs:sequence we should replace it with xs:all not xs:any

NiklasPor avatar Sep 16 '24 07:09 NiklasPor

The xs:all (and xs:any) is not suited, I think, because it only allows for one element of a kind (the maxOccurs can be only 0 or 1 and we need 'unbounded').

On the main topic, I think you're right, @NiklasPor. I don't see any reason for having maxOccurs=unbounded on sequence in requirementsType (L113). We want to be able to have for example: E-C-P-P, but not E-P-C-P. Right now, maxOccurs makes it possible.

Attaching sample file: SequenceTest.zip

atomczak avatar Sep 16 '24 14:09 atomczak

To solve this, we could we leave xs:sequence (because: https://github.com/buildingSMART/IDS/issues/175#issuecomment-1744634900), but remove: <xs:sequence ~~maxOccurs="unbounded"~~ > from https://github.com/buildingSMART/IDS/blob/eff6e5763c579011ca2f70dd574b7f1d6a144816/Schema/ids.xsd#L113C15-L113C37 to prevent multiple entity facets.

atomczak avatar Nov 06 '24 19:11 atomczak

How about using: <xs:choice maxOccurs="unbounded"> instead of all sequences (see: https://stackoverflow.com/questions/28523597/meaning-of-minoccurs-and-maxoccurs-for-xsdchoice). This would also resolve #205.

atomczak avatar Nov 13 '24 17:11 atomczak