Consider altChunk parts in Flat OPC transformation

[ThomasBarnekow]

This commit enhances the

XDocument OpenXmlPackage.ToFlatOpcDocument(XProcessingInstruction)

method to consider "altChunk parts" (AlternativeFormatInputParts),
which must have their content rendered in binary form even if it is XML
content (e.g., XML, XHTML).

Fixes #525.

[ThomasBarnekow]

@twsouthwick, how can I check why this failed? I don't have access to the Azure DevOps project.

All unit tests passed on my local machine, so I don't know what might have happened.

[ThomasBarnekow]

I might have found another "bug" in Directory.Build.props, which still lists net35.

<ProductTargetFrameworks>netstandard1.3;netstandard2.0;net35;net40;net46</ProductTargetFrameworks>

After setting ProjectLoadStyle to All, the build failed also on my machine. This was due to the fact that net35 does not provide the ISet<T> interface.

[ThomasBarnekow]

So I added a fix for net35 and the solution was built successfully and all unit tests passed. What goes wrong? I don't have access to the details.

[ThomasBarnekow]

@rominator1983, do you have access to those details which might tell us why the check failed?

[twsouthwick]

Good point about access. We're building it on the office dev ops instance so we can sign stuff... but we can't give access. I'll look into how we can expose that.

[twsouthwick]

As for .NET 3.5, I do want to continue to support it, but it does limit some things. What I end up doing is adding stubs for the types to get things working. You can see under the System folder, a number of these, such as IReadOnlyLIst and others.

[twsouthwick]

Thanks for taking a look at this. A few comments about some of the structure. I tend to avoid too much LINQ as recent profiling I've done has identified it as hot spots. I'm fine with starting off with it for getting correctness, but keeping them in separate methods will help during profiling to identify any specific queries that are problematic.

Also, I don't see any tests being added? Can you add some of those? Preferably more unit test style without needing a full file; just build up the components needed and the expected outcome.

[ThomasBarnekow]

@twsouthwick, thanks for your feedback.

Please have a look at the unit test in the OpenXmlPackageTests class in the DocumentFormat.OpenXml.Packaging.Tests project.

[twsouthwick]

Must have missed the test - sorry about that. You can disregard that comment.

[ThomasBarnekow]

Made the changes requested by @twsouthwick. Rebased on master and fixed up some commits.

[ThomasBarnekow]

@twsouthwick, can you tell me what went wrong here? Setting ProjectLoadStyle to All, the solution is successfully built and all unit tests pass on my machine.

[twsouthwick]

I have warnings as errors turned on. I recently added some FxCop rules to enforce StringComparer, so that's what's being raised:

src\DocumentFormat.OpenXml\Packaging\OpenXmlPackage.FlatOpc.cs(89,17): Error CA1307: The behavior of 'string.EndsWith(string)' could vary based on the current user's locale settings. Replace this call in 'DocumentFormat.OpenXml.Packaging.OpenXmlPackage.GetContentsAsXml(System.IO.Packaging.PackagePart, System.Collections.Generic.HashSet<System.Uri>)' with a call to 'string.EndsWith(string, System.StringComparison)'.

Not having access to the build is not good. I may create a separate devops instance for CI builds that I can make public. I'll look into that.

[twsouthwick]

Looking much better. A couple more fixes and it should be good to merge in. Do you mind also updating the Changelog with an entry to this (referencing the PR)? I'm hoping to get a full release of 2.10 soon and would like this to be included

[ThomasBarnekow]

@twsouthwick, is it possible that you turned those FxCop rules on after my previous build? That would explain it because I did not change that line of code with the warning.

[twsouthwick]

It was sometime yesterday (Pacific time) in the afternoon. Highly possible

[twsouthwick]

I opened #666 as a tracking item for seeing build failures

[ThomasBarnekow]

@twsouthwick, unless you have further input, this would be it for now. I've updated CHANGELOG.md and also beautified the Flat OPC-related partial classes (where I also removed the redundant base type references).

[twsouthwick]

LGTM! Thanks for responding to the feedback :)