-
Notifications
You must be signed in to change notification settings - Fork 68
Remove or fix APIs that were ported with different signatures #510
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
65 changes: 52 additions & 13 deletions
65
src/Microsoft.AspNetCore.SystemWebAdapters/Generated/Ref.Standard.cs
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
109 changes: 109 additions & 0 deletions
109
src/Microsoft.AspNetCore.SystemWebAdapters/HttpApplicationStateBase.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Collections; | ||
| using System.Collections.Specialized; | ||
| using System.Diagnostics.CodeAnalysis; | ||
|
|
||
| namespace System.Web; | ||
|
|
||
| [SuppressMessage("Design", "CA1010:Generic interface should also be implemented", Justification = Constants.ApiFromAspNet)] | ||
| [SuppressMessage("Naming", "CA1710:Identifiers should have correct suffix", Justification = Constants.ApiFromAspNet)] | ||
| public abstract class HttpApplicationStateBase : NameObjectCollectionBase, ICollection | ||
| { | ||
| [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = Constants.ApiFromAspNet)] | ||
| [SuppressMessage("Performance", "CA1819:Properties should not return arrays", Justification = Constants.ApiFromAspNet)] | ||
| public virtual string?[]? AllKeys => throw new NotImplementedException(); | ||
|
|
||
| [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = Constants.ApiFromAspNet)] | ||
| public virtual HttpApplicationStateBase Contents => throw new NotImplementedException(); | ||
|
|
||
| [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = Constants.ApiFromAspNet)] | ||
| public override int Count => throw new NotImplementedException(); | ||
|
|
||
| [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = Constants.ApiFromAspNet)] | ||
| public virtual bool IsSynchronized => throw new NotImplementedException(); | ||
|
|
||
| [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = Constants.ApiFromAspNet)] | ||
| public virtual object SyncRoot => throw new NotImplementedException(); | ||
|
|
||
| [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = Constants.ApiFromAspNet)] | ||
| [DisallowNull] | ||
| public virtual object? this[int index] => throw new NotImplementedException(); | ||
|
|
||
| [SuppressMessage("Design", "CA1065:Do not raise exceptions in unexpected locations", Justification = Constants.ApiFromAspNet)] | ||
| [DisallowNull] | ||
| public virtual object? this[string name] | ||
| { | ||
| get => throw new NotImplementedException(); | ||
| set => throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public virtual void Add(string name, object value) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public virtual void Clear() | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public virtual void CopyTo(Array array, int index) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| [SuppressMessage("Naming", "CA1716:Identifiers should not match keywords", Justification = Constants.ApiFromAspNet)] | ||
| public virtual object? Get(int index) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| [SuppressMessage("Naming", "CA1716:Identifiers should not match keywords", Justification = Constants.ApiFromAspNet)] | ||
| public virtual object? Get(string name) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public override IEnumerator GetEnumerator() | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public virtual string? GetKey(int index) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public virtual void Lock() | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public virtual void Remove(string name) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public virtual void RemoveAll() | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public virtual void RemoveAt(int index) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| [SuppressMessage("Naming", "CA1716:Identifiers should not match keywords", Justification = Constants.ApiFromAspNet)] | ||
| public virtual void Set(string name, object value) | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
|
|
||
| public virtual void UnLock() | ||
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
| } |
108 changes: 108 additions & 0 deletions
108
src/Microsoft.AspNetCore.SystemWebAdapters/HttpApplicationStateWrapper.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Collections; | ||
| using System.Collections.Specialized; | ||
| using System.Diagnostics.CodeAnalysis; | ||
|
|
||
| namespace System.Web; | ||
|
|
||
| [SuppressMessage("Design", "CA1010:Generic interface should also be implemented", Justification = Constants.ApiFromAspNet)] | ||
| public class HttpApplicationStateWrapper : HttpApplicationStateBase | ||
| { | ||
| private readonly HttpApplicationState _application; | ||
|
|
||
| public HttpApplicationStateWrapper(HttpApplicationState httpApplicationState) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(httpApplicationState); | ||
|
|
||
| _application = httpApplicationState; | ||
| } | ||
|
|
||
| public override string?[]? AllKeys => _application.AllKeys; | ||
|
|
||
| public override HttpApplicationStateBase Contents => this; | ||
|
|
||
| public override int Count => _application.Count; | ||
|
|
||
| public override bool IsSynchronized => ((ICollection)_application).IsSynchronized; | ||
|
|
||
| public override NameObjectCollectionBase.KeysCollection Keys => _application.Keys; | ||
|
|
||
| public override object SyncRoot => ((ICollection)_application).SyncRoot; | ||
|
|
||
| [DisallowNull] | ||
| public override object? this[int index] => _application[index]!; | ||
|
|
||
| [DisallowNull] | ||
| public override object? this[string name] | ||
| { | ||
| get => _application[name]; | ||
| set => _application[name] = value; | ||
| } | ||
|
|
||
| public override void Add(string name, object value) | ||
| { | ||
| _application.Add(name, value); | ||
| } | ||
|
|
||
| public override void Clear() | ||
| { | ||
| _application.Clear(); | ||
| } | ||
|
|
||
| public override void CopyTo(Array array, int index) | ||
| { | ||
| ((ICollection)_application).CopyTo(array, index); | ||
| } | ||
|
|
||
| public override object? Get(int index) | ||
| { | ||
| return _application.Get(index); | ||
| } | ||
|
|
||
| public override object? Get(string name) | ||
| { | ||
| return _application.Get(name); | ||
| } | ||
|
|
||
| public override IEnumerator GetEnumerator() | ||
| { | ||
| return _application.GetEnumerator(); | ||
| } | ||
|
|
||
| public override string? GetKey(int index) | ||
| { | ||
| return _application.GetKey(index); | ||
| } | ||
|
|
||
| public override void Lock() | ||
| { | ||
| _application.Lock(); | ||
| } | ||
|
|
||
| public override void Remove(string name) | ||
| { | ||
| _application.Remove(name); | ||
| } | ||
|
|
||
| public override void RemoveAll() | ||
| { | ||
| _application.RemoveAll(); | ||
| } | ||
|
|
||
| public override void RemoveAt(int index) | ||
| { | ||
| _application.RemoveAt(index); | ||
| } | ||
|
|
||
| public override void Set(string name, object value) | ||
| { | ||
| _application.Set(name, value); | ||
| } | ||
|
|
||
| public override void UnLock() | ||
| { | ||
| _application.UnLock(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,14 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Net; | ||
| using System.IO; | ||
| using Microsoft.AspNetCore.Http; | ||
|
|
||
| namespace System.Web; | ||
|
|
||
| public class HttpException : SystemException | ||
| { | ||
| private readonly int _httpStatusCode = 500; | ||
| private readonly int _httpStatusCode; | ||
|
|
||
| public HttpException() | ||
| { | ||
|
|
@@ -17,50 +18,32 @@ public HttpException(string message) : base(message) | |
| { | ||
| } | ||
|
|
||
| public HttpException(string message, Exception innerException) | ||
| public HttpException(String message, Exception innerException) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Super nit-pick but this change seems unnecessary. We use |
||
| : base(message, innerException) | ||
| { | ||
| } | ||
|
|
||
| public HttpException(int httpStatusCode) | ||
| { | ||
| _httpStatusCode = httpStatusCode; | ||
| } | ||
|
|
||
| public HttpException(HttpStatusCode httpStatusCode) | ||
| { | ||
| _httpStatusCode = (int)httpStatusCode; | ||
| } | ||
|
|
||
| public HttpException(int httpStatusCode, string message) | ||
| : base(message) | ||
| { | ||
| _httpStatusCode = httpStatusCode; | ||
| } | ||
|
|
||
| public HttpException(HttpStatusCode httpStatusCode, string message) | ||
| : base(message) | ||
| { | ||
| _httpStatusCode = (int)httpStatusCode; | ||
| } | ||
|
|
||
| public HttpException(int httpStatusCode, string message, Exception innerException) | ||
| : base(message, innerException) | ||
| { | ||
| _httpStatusCode = httpStatusCode; | ||
| } | ||
|
|
||
| public HttpException(HttpStatusCode httpStatusCode, string message, Exception innerException) | ||
| : base(message, innerException) | ||
| { | ||
| _httpStatusCode = (int)httpStatusCode; | ||
| } | ||
|
|
||
| [Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1024:Use properties where appropriate", Justification = Constants.ApiFromAspNet)] | ||
| public int GetHttpCode() | ||
| { | ||
| return StatusCode; | ||
| } | ||
| public int GetHttpCode() => GetHttpCodeForException(this); | ||
|
|
||
| public int StatusCode => _httpStatusCode; | ||
| internal static int GetHttpCodeForException(Exception e) => e switch | ||
| { | ||
| HttpException { _httpStatusCode: { } code } when code > 0 => code, | ||
| UnauthorizedAccessException => StatusCodes.Status401Unauthorized, | ||
| PathTooLongException => StatusCodes.Status414UriTooLong, | ||
| { InnerException: { } inner } => GetHttpCodeForException(inner), | ||
| _ => 500 | ||
| }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you've removed the SetCacheability overload here... Was that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - it wasn't implemented on HttpCachePolicy itself so we couldn't forward it in the wrapper correctly. Happy to take a fix that implements it on all three as a follow up :) I've added tests to validate that the three (i.e. HttpContext/HttpContextBase/HttpContextWrapper) are all kept in sync when adding APIs and this case was causing failure