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

Fix issue #51778 and add more test coverage for xs:restrictions. #51779

Merged
merged 5 commits into from
Apr 28, 2021
Merged

Fix issue #51778 and add more test coverage for xs:restrictions. #51779

merged 5 commits into from
Apr 28, 2021

Conversation

lovettchris
Copy link
Contributor

@lovettchris lovettchris commented Apr 24, 2021

Fixes #51778

Implemented a simple fix which is to remove the check below:

 || derivdSequence.Items.Count > baseChoice.Items.Count)

from the following method as it is actually bogus. In the xsd:restriction case the number of immediate child particles in the sequence or in the choice cannot be compared like this since GetMappingParticle is actually recursive.

        private bool IsSequenceFromChoice(XmlSchemaSequence derivedSequence, XmlSchemaChoice baseChoice)
        {
            decimal minOccurs, maxOccurs;
            minOccurs = derivedSequence.MinOccurs * derivedSequence.Items.Count;
            if (derivedSequence.MaxOccurs == decimal.MaxValue)
            {
                maxOccurs = decimal.MaxValue;
            }
            else
            {
                maxOccurs = derivedSequence.MaxOccurs * derivedSequence.Items.Count;
            }
            if (!IsValidOccurrenceRangeRestriction(minOccurs, maxOccurs, baseChoice.MinOccurs, baseChoice.MaxOccurs)
                 || derivdSequence.Items.Count > baseChoice.Items.Count)
            {
                return false;
            }
            for (int i = 0; i < derivedSequence.Items.Count; ++i)
            {
                if (GetMappingParticle((XmlSchemaParticle)derivedSequence.Items[i], baseChoice.Items) < 0)
                    return false;
            }
            return true;
        }

@ghost
Copy link

ghost commented Apr 24, 2021

Tagging subscribers to this area: @buyaa-n, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

Implemented a simple fix which is to remove the check below:

 || derivdSequence.Items.Count > baseChoice.Items.Count)

from the following method as it is actually bogus. In the xsd:restriction case the number of immediate child particles in the sequence or in the choice cannot be compared like this since GetMappingParticle is actually recursive.

        private bool IsSequenceFromChoice(XmlSchemaSequence derivedSequence, XmlSchemaChoice baseChoice)
        {
            decimal minOccurs, maxOccurs;
            minOccurs = derivedSequence.MinOccurs * derivedSequence.Items.Count;
            if (derivedSequence.MaxOccurs == decimal.MaxValue)
            {
                maxOccurs = decimal.MaxValue;
            }
            else
            {
                maxOccurs = derivedSequence.MaxOccurs * derivedSequence.Items.Count;
            }
            if (!IsValidOccurrenceRangeRestriction(minOccurs, maxOccurs, baseChoice.MinOccurs, baseChoice.MaxOccurs)
                 || derivdSequence.Items.Count > baseChoice.Items.Count)
            {
                return false;
            }
            for (int i = 0; i < derivedSequence.Items.Count; ++i)
            {
                if (GetMappingParticle((XmlSchemaParticle)derivedSequence.Items[i], baseChoice.Items) < 0)
                    return false;
            }
            return true;
        }
Author: lovettchris
Assignees: -
Labels:

area-System.Xml

Milestone: -

@danmoseley danmoseley requested a review from krwq April 24, 2021 16:34
@danmoseley
Copy link
Member

Thanks @lovettchris !

@danmoseley
Copy link
Member

Some tests failed eg


10:29:14] info: System.Xml.Schema.XmlSchemaException : Invalid particle derivation by restriction - 'The derived sequence particle at (22, 6) is not a valid restriction of the base choice particle at (7, 4) according to Sequence:Choice -- MapAndSum.'.

https://dev.azure.com/dnceng/public/_build/results?buildId=1105728&view=ms.vss-test-web.build-test-results-tab

@lovettchris
Copy link
Contributor Author

lovettchris commented Apr 24, 2021 via email

@lovettchris
Copy link
Contributor Author

lovettchris commented Apr 27, 2021

Hmmm, I seem to be hitting some unrelated tests failures, see above, complaining about System.IO.FileSystem.Watcher.Tests in job 550598f7-547a-4211-b618-e4799eb70970 has failed? And an infrustructure issue saying "##[error].packages/microsoft.dotnet.helix.sdk/6.0.0-beta.21222.1/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(78,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.IO.FileSystem.Watcher.Tests in job 550598f7-547a-4211-b618-e4799eb70970 has failed." ?

@danmoseley
Copy link
Member

That test failure is #30056 which @carlossanlop is working on a fix for. So your PR validation is clean. Just need @krwq to sign off.

@krwq krwq merged commit 2223bab into dotnet:main Apr 28, 2021
@krwq
Copy link
Member

krwq commented Apr 28, 2021

Thank you @lovettchris!

@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XmlSchema reports error on a valid XML schema regarding <xs:restriction>
4 participants