Skip to content

Fix min/maxOccurs validation for optional elements#1005

Merged
stevedlawrence merged 1 commit intoapache:mainfrom
stevedlawrence:daffodil-2808-optional-validation
Apr 17, 2023
Merged

Fix min/maxOccurs validation for optional elements#1005
stevedlawrence merged 1 commit intoapache:mainfrom
stevedlawrence:daffodil-2808-optional-validation

Conversation

@stevedlawrence
Copy link
Member

Commit dd60b60 made changes to validation so that it took into account absent elements that incremented the array iteration count but did not actually change the array size. It did this by just counting the actual elements in the infoset. Unfortunately, that change was very subtly broken for optional elements since it assumed maybeLastChild was always a DIArray, but it could actually be a DISimple/DIComplex for optional elements since they use very similar array logic.

We could modify this logic to take DISimple/DIComplex nodes into account, but later commit 16c738c added new state to differentiate between the occurs index and the iteration index. So this just modifies the array validation logic so it is very similar to the old logic, but uses the new state that accounts for absent elements.

DAFFODIL-2808

Commit dd60b60 made changes to validation so that it took into
account absent elements that incremented the array iteration count but
did not actually change the array size. It did this by just counting the
actual elements in the infoset. Unfortunately, that change was very
subtly broken for optional elements since it assumed maybeLastChild was
always a DIArray, but it could actually be a DISimple/DIComplex for
optional elements since they use very similar array logic.

We could modify this logic to take DISimple/DIComplex nodes into
account, but later commit 16c738c added new state to differentiate
between the occurs index and the iteration index. So this just modifies
the array validation logic so it is very similar to the old logic, but
uses the new state that accounts for absent elements.

DAFFODIL-2808
Copy link
Member

@mike-mcgann mike-mcgann left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@tuxji tuxji left a comment

Choose a reason for hiding this comment

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

+1

@stevedlawrence stevedlawrence merged commit c8e7059 into apache:main Apr 17, 2023
@stevedlawrence stevedlawrence deleted the daffodil-2808-optional-validation branch April 17, 2023 15:47
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.

3 participants