dotnet-api-docs icon indicating copy to clipboard operation
dotnet-api-docs copied to clipboard

Port System.ServiceModel new docs

Open carlossanlop opened this issue 1 year ago • 11 comments

@HongGit @mconnew @directhex @gewarren PTAL

carlossanlop avatar Jul 25 '24 21:07 carlossanlop

Learn Build status updates of commit d6e3337:

:warning: Validation status: warnings

File Status Preview URL Details
xml/System.ServiceModel.Channels/TransportDuplexSessionChannel.xml :warning:Warning View Details
xml/System.ServiceModel.Federation/WSTrustChannel.xml :warning:Warning View Details
xml/System.ServiceModel.Federation/WSTrustChannelSecurityTokenProvider.xml :warning:Warning View Details
xml/System.ServiceModel.Channels/OutputChannel.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Channels/RequestChannel.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Channels/SecurityCapabilities.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Channels/ServiceChannelProxy.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Channels/TransportDuplexSessionChannel+ConnectionDuplexSession.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Dispatcher/OperationInvokerBehavior.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Dispatcher/SyncMethodInvoker.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Dispatcher/TaskMethodInvoker.xml :white_check_mark:Succeeded View
xml/System.ServiceModel/ChannelFactory.xml :white_check_mark:Succeeded View
xml/System.ServiceModel/ClientBase`1.xml :white_check_mark:Succeeded View

xml/System.ServiceModel.Channels/TransportDuplexSessionChannel.xml

  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'System.ServiceModel.Channels.IAsyncDuplexSession'.
  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'System.ServiceModel.Channels.IAsyncDuplexSession'.

xml/System.ServiceModel.Federation/WSTrustChannel.xml

  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'Microsoft.IdentityModel.Protocols.WsTrust.WsTrustRequest'.
  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'Microsoft.IdentityModel.Protocols.WsTrust.WsTrustRequest'.
  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'Microsoft.IdentityModel.Protocols.WsTrust.WsTrustRequest'.

xml/System.ServiceModel.Federation/WSTrustChannelSecurityTokenProvider.xml

  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'Microsoft.IdentityModel.Protocols.WsTrust.WsTrustRequest'.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

Something has gone very wrong with the documentation generator here. The class TransportDuplexSessionChannel is internal, but documentation has been generated for it. There's an error about a cross reference to System.ServiceModel.Channels.IAsyncDuplexSession not being found, but that's an internal interface implemented on an internal class.

It looks like there's some bugs in the generator picking up api's it shouldn't.

mconnew avatar Jul 25 '24 22:07 mconnew

I think I see the problem, looking at the preview page, it lists the location of that class as being System.Private.ServiceModel.dll in v4.10.3. It might have been declared as public in that assembly in that version, but that assembly should never have been scanned for public api's as the nuget package isn't intended to be directly referenced. It has an empty ref folder so that if it is referenced, you don't get to compile against any api's.

mconnew avatar Jul 25 '24 22:07 mconnew

@mconnew the last preview that was ingested into dotnet-api-docs was preview6, and the TransportDuplexSessionChannel type did not get modified there: https://github.com/dotnet/dotnet-api-docs/pull/10114

So it means we have been incorrectly scanning that DLL you mention since way before.

As you said, it must be getting detected as public in System.Private.ServiceModel.dll, because that's how we show it here: https://github.com/dotnet/dotnet-api-docs/blob/d6e3337eaa73627c4c6b09a41618abdd808f900f/xml/System.ServiceModel.Channels/TransportDuplexSessionChannel.xml#L2

@gewarren what can we do to remove anything that came from that DLL?

carlossanlop avatar Jul 25 '24 22:07 carlossanlop

Are we incorrectly including that DLL in the transport packages you use to update the CI here, @gewarren?

carlossanlop avatar Jul 25 '24 22:07 carlossanlop

I checked, it is public in the private package for 4.10 as seen here. That type got moved to the System.ServiceModel.NetFramingBase package as part of removing the old private package, and was made internal as part of the process.

mconnew avatar Jul 25 '24 22:07 mconnew

The tool that grabs the NuGet packages looks for any packages published by aspnet or dotnetframework that start with System, Microsoft.Bcl, Microsoft.Extensions, or Microsoft.Win32. I'll add a special case to exclude System.Private.ServiceModel and rerun the CI, so those APIs should go away.

gewarren avatar Jul 26 '24 04:07 gewarren

Thank you @gewarren!

Let's wait for the CI update so I can rebase this PR and we determine if there are still any comments that need porting.

carlossanlop avatar Jul 26 '24 05:07 carlossanlop

CI update: https://github.com/dotnet/dotnet-api-docs/pull/10164

carlossanlop avatar Jul 26 '24 18:07 carlossanlop

@mconnew the CI update has been merged and this PR has been rebased. Would you mind taking another look?

carlossanlop avatar Jul 26 '24 19:07 carlossanlop

Learn Build status updates of commit b7edbe8:

:warning: Validation status: warnings

File Status Preview URL Details
xml/System.IdentityModel.Claims/WindowsClaimSet.xml :warning:Warning View Details
xml/System.Runtime.Diagnostics/EventTraceActivity.xml :warning:Warning Details
xml/System.Runtime/TimeoutHelper.xml :warning:Warning Details
xml/System.ServiceModel.Channels/ClientWebSocketFactory.xml :warning:Warning View Details
xml/System.ServiceModel.Channels/CommunicationPool`2.xml :warning:Warning Details
xml/System.ServiceModel.Channels/CommunicationPool`2+EndpointConnectionPool.xml :warning:Warning Details
xml/System.ServiceModel.Channels/CommunicationPool`2+EndpointConnectionPool+PoolIdleConnectionPool.xml :warning:Warning Details
xml/System.ServiceModel.Channels/CommunicationPool`2+IdleConnectionPool.xml :warning:Warning Details
xml/System.ServiceModel.Channels/ConnectionPool.xml :warning:Warning Details
xml/System.ServiceModel.Channels/ConnectionPoolRegistry.xml :warning:Warning Details
xml/System.ServiceModel.Channels/IAsyncRequest.xml :warning:Warning Details
xml/System.ServiceModel.Channels/IConnection.xml :warning:Warning View Details
xml/System.ServiceModel.Channels/IConnectionInitiator.xml :warning:Warning View Details
xml/System.ServiceModel.Channels/IConnectionOrientedConnectionSettings.xml :warning:Warning Details
xml/System.ServiceModel.Channels/IConnectionOrientedTransportChannelFactorySettings.xml :warning:Warning Details
xml/System.ServiceModel.Channels/IConnectionOrientedTransportFactorySettings.xml :warning:Warning Details
xml/System.ServiceModel.Channels/IdlingCommunicationPool`2.xml :warning:Warning Details
xml/System.ServiceModel.Channels/IdlingCommunicationPool`2+IdleTimeoutEndpointConnectionPool.xml :warning:Warning Details
xml/System.ServiceModel.Channels/IdlingCommunicationPool`2+IdleTimeoutEndpointConnectionPool+IdleTimeoutIdleConnectionPool.xml :warning:Warning Details
xml/System.ServiceModel.Channels/IMessageSource.xml :warning:Warning Details
xml/System.ServiceModel.Channels/IRequestBase.xml :warning:Warning Details
xml/System.ServiceModel.Channels/ITcpChannelFactorySettings.xml :warning:Warning Details
xml/System.ServiceModel.Channels/ITransportFactorySettings.xml :warning:Warning Details
xml/System.ServiceModel.Channels/MaxMessageSizeStream.xml :warning:Warning Details
xml/System.ServiceModel.Channels/RequestChannel.xml :warning:Warning Details

This comment lists only the first 25 files in the pull request.

xml/System.IdentityModel.Claims/WindowsClaimSet.xml

  • [Warning: file-not-redirected - See documentation] File xml/System.IdentityModel.Claims/WindowsClaimSet.xml with URL /dotnet/api/system.identitymodel.claims.windowsclaimset.trycreatewindowssidclaim was deleted without redirection. To avoid broken links, add a redirection.

xml/System.Runtime.Diagnostics/EventTraceActivity.xml

  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime.Diagnostics/EventTraceActivity.xml with URL /dotnet/api/system.runtime.diagnostics.eventtraceactivity was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime.Diagnostics/EventTraceActivity.xml with URL /dotnet/api/system.runtime.diagnostics.eventtraceactivity.activityid was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime.Diagnostics/EventTraceActivity.xml with URL /dotnet/api/system.runtime.diagnostics.eventtraceactivity.-ctor was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime.Diagnostics/EventTraceActivity.xml with URL /dotnet/api/system.runtime.diagnostics.eventtraceactivity.empty was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime.Diagnostics/EventTraceActivity.xml with URL /dotnet/api/system.runtime.diagnostics.eventtraceactivity.getactivityidfromthread was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime.Diagnostics/EventTraceActivity.xml with URL /dotnet/api/system.runtime.diagnostics.eventtraceactivity.getfromthreadorcreate was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime.Diagnostics/EventTraceActivity.xml with URL /dotnet/api/system.runtime.diagnostics.eventtraceactivity.name was deleted without redirection. To avoid broken links, add a redirection.

xml/System.Runtime/TimeoutHelper.xml

  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.add was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.-ctor was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.divide was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.elapsedtime was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.frommilliseconds was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.getcancellationtoken was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.getcancellationtokenasync was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.istoolarge was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.maxwait was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.min was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.originaltimeout was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.remainingtime was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.subtract was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.throwifnegativeargument was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.throwifnonpositiveargument was deleted without redirection. To avoid broken links, add a redirection.
  • [Warning: file-not-redirected - See documentation] File xml/System.Runtime/TimeoutHelper.xml with URL /dotnet/api/system.runtime.timeouthelper.tomilliseconds was deleted without redirection. To avoid broken links, add a redirection.

This comment lists only the first 25 errors (including error/warning/suggestion) in the pull request. For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

Learn Build status updates of commit 7c5c082:

:warning: Validation status: warnings

File Status Preview URL Details
xml/System.ServiceModel.Federation/WSTrustChannel.xml :warning:Warning View Details
xml/System.ServiceModel.Federation/WSTrustChannelSecurityTokenProvider.xml :warning:Warning View Details
xml/System.ServiceModel.Channels/OutputChannel.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Channels/ServiceChannelProxy.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Dispatcher/OperationInvokerBehavior.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Dispatcher/SyncMethodInvoker.xml :white_check_mark:Succeeded View
xml/System.ServiceModel.Dispatcher/TaskMethodInvoker.xml :white_check_mark:Succeeded View
xml/System.ServiceModel/ChannelFactory.xml :white_check_mark:Succeeded View
xml/System.ServiceModel/ClientBase`1.xml :white_check_mark:Succeeded View

xml/System.ServiceModel.Federation/WSTrustChannel.xml

  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'Microsoft.IdentityModel.Protocols.WsTrust.WsTrustRequest'.
  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'Microsoft.IdentityModel.Protocols.WsTrust.WsTrustRequest'.
  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'Microsoft.IdentityModel.Protocols.WsTrust.WsTrustRequest'.

xml/System.ServiceModel.Federation/WSTrustChannelSecurityTokenProvider.xml

  • Line 0, Column 0: [Warning: xref-not-found - See documentation] Cross reference not found: 'Microsoft.IdentityModel.Protocols.WsTrust.WsTrustRequest'.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

There's still a few problems. SyncMethodInvoker, TaskMethodInvoker, ServiceChannelProxy, and OutputChannel are public in our implementation assembly, but are purposefully absent from our ref assembly.

I don't fully understand the schema for these files so can't tell if it's correct or not, but ClientBase<T> only implements IAsyncDisposable on .NET6+.

I also see another problem, looking at the CLientBase<T> details for System.IAsyncDisposable.DisposeAsync, under AssemblyInfo, it's listing System.Private.ServiceModel, assembly version 4.10.3.0. That api wasn't in 4.10.3, but if it was, the assembly info should be pointing to System.ServiceModel.Primitives because that's where the ref assembly (with matching name).

mconnew avatar Jul 30 '24 00:07 mconnew

There's still a few problems. SyncMethodInvoker, TaskMethodInvoker, ServiceChannelProxy, and OutputChannel are public in our implementation assembly, but are purposefully absent from our ref assembly.

There's a special case for System.ServiceModel.Primitives where we use the reference assemblies instead of the implementation assemblies - but the behavior you're describing seems to be the opposite of that. Should I remove this special behavior? If I recall correctly, it was necessary for the ingestion logic to avoid hitting an error.

I don't fully understand the schema for these files so can't tell if it's correct or not, but ClientBase<T> only implements IAsyncDisposable on .NET6+.

It currently shows as only applicable to .NET 8+ (and .NET Core 1.0/1.1 and .NET Framework) - https://review.learn.microsoft.com/en-us/dotnet/api/system.servicemodel.clientbase-1?view=net-8.0&branch=pr-en-us-10157#applies-to.

I also see another problem, looking at the CLientBase<T> details for System.IAsyncDisposable.DisposeAsync, under AssemblyInfo, it's listing System.Private.ServiceModel, assembly version 4.10.3.0. That api wasn't in 4.10.3, but if it was, the assembly info should be pointing to System.ServiceModel.Primitives because that's where the ref assembly (with matching name).

I don't think that info is displayed anywhere, so this might just be some leftover info that wasn't cleaned up when System.Private.ServiceModel was removed.

gewarren avatar Jul 30 '24 00:07 gewarren

Should I remove this special behavior?

@mconnew this is a question for you. Do you mind answering it so we can advance this PR?

carlossanlop avatar Aug 28 '24 21:08 carlossanlop

@gewarren, the reference assemblies are what is supposed to be used. The situation used to be that the implementation was in the System.Private.ServiceModel package in a lib folder. We created a ref folder with a dummy empty file (otherwise nuget doesn't see the folder or removes it, something bad happened anyway) so that if someone added the package, the compiler wouldn't see any of the implementation. Then in the System.ServiceModel.* packages (e.g. Primitives) we had a ref folder which had all the api's. In the lib folder, we had a type forwarding assembly which just redirected the runtime to the System.Private.ServiceModel.dll implementation to find the types. It just had type forwarding attributes and no implementation.

Now we've got rid of System.Private.Servicmodel. How things are right now is slightly different from how they will be in a few months. Currently every package except Primitives only has an implementation assembly which will then double for being the reference assembly. For Primitives we have a ref and a lib folder. The ref folder contains the reference assembly and has all the public types. In the lib folder, we have the implementation assembly which has extra types which are public. Some are unintentional (but it never mattered before), some are intentional so that the other packages can use them. It avoids having to use InternalsVisibleTo. SyncMethodInvoker and TaskMethodInvoker are unintentionally public but do not appear in the reference assembly.

I'm currently working on changes where NetTcp and Http will also have reference assemblies. Because we already did the work to make sure nothing was unintentionally public, the public api surface of the reference assembly and the implementation assembly will be the same for .NET. We're bringing back netstandard2.0 which will have some api's excluded as they aren't available on .NET Framework. And we'll have type forwarding facades for .NET Framework.

mconnew avatar Aug 29 '24 20:08 mconnew

@mconnew What do you need me to do to resolve the blocker for this PR? Is that remove SyncMethodInvoker, TaskMethodInvoker, ServiceChannelProxy, and OutputChannel?

gewarren avatar Sep 04 '24 17:09 gewarren

@gewarren, these are just a few types which I noticed. Basically you should be scanning for types in the ref folders first and only looking in the lib folder if there isn't a ref folder. We keep getting inquiries about various types or methods which are documented but aren't available because of this issue. We just had one inquiring about a method on SecurityBindingElement which isn't exposed but is documented as such. A few days ago we had someone inquiring about the enum type InstanceContextMode which isn't exposed by the ref assembly but is documented as available too. I don't want to play whack a mole with types and methods, the correct assembly should be scanned.

mconnew avatar Sep 06 '24 21:09 mconnew

@gewarren, these are just a few types which I noticed. Basically you should be scanning for types in the ref folders first and only looking in the lib folder if there isn't a ref folder.

@huangmin-ms Is this 👆 something that mdoc is capable of?

gewarren avatar Sep 06 '24 23:09 gewarren

@gewarren, these are just a few types which I noticed. Basically you should be scanning for types in the ref folders first and only looking in the lib folder if there isn't a ref folder.

@huangmin-ms Is this 👆 something that mdoc is capable of?

@gewarren Thanks for reporting. We already support this and I can also see the configuration is correct.

https://apidrop.visualstudio.com/_git/binaries?path=/dotnet/net-8.0/net-8.0.csv&version=GBmaster&line=230&lineEnd=230&lineStartColumn=1&lineEndColumn=79&lineStyle=plain&_a=contents

pac230,[tfm=net8.0;includeXml=False;libpath=ref]System.ServiceModel.Primitives,8.0.0

However this is impacted by the source link change, we will fix it in https://ceapex.visualstudio.com/Engineering/_workitems/edit/996190

huangmin-ms avatar Sep 09 '24 09:09 huangmin-ms

@gewarren, these are just a few types which I noticed. Basically you should be scanning for types in the ref folders first and only looking in the lib folder if there isn't a ref folder.

@huangmin-ms Is this 👆 something that mdoc is capable of?

@gewarren The fix is done. Please help to review the latest CI update: https://github.com/dotnet/dotnet-api-docs/pull/10385 Thanks.

huangmin-ms avatar Sep 10 '24 14:09 huangmin-ms

@mconnew Just to clarify, you want the docs to use the reference assemblies for all System.ServiceModel.* packages?

gewarren avatar Sep 10 '24 20:09 gewarren

@gewarren, not exactly. Not all packages have reference assemblies. Currently only Primitives has a reference assembly, but that will be changing at some time in the future. Does the docs tooling require knowing ahead of time whether to look at a ref assembly of the lib?

mconnew avatar Sep 10 '24 20:09 mconnew

@gewarren, not exactly. Not all packages have reference assemblies. Currently only Primitives has a reference assembly, but that will be changing at some time in the future. Does the docs tooling require knowing ahead of time whether to look at a ref assembly of the lib?

Yes. Currently only System.ServiceModel.Primitives is special cased that way.

gewarren avatar Sep 10 '24 20:09 gewarren

That works for now, and when we release new package versions with ref assemblies that should be scanned instead, I'll let you know.

mconnew avatar Sep 10 '24 23:09 mconnew

What should I do about this PR?

carlossanlop avatar Sep 12 '24 20:09 carlossanlop

Closing. Will resubmit after the RC1 update.

carlossanlop avatar Sep 13 '24 17:09 carlossanlop