-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[dotnet] Implement third-party Permissions module #16414
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
b595dfc
eaa3d5c
71a7489
1fbf4c6
9a26d1e
dbe8e0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
// <copyright file="PermissionDescriptor.cs" company="Selenium Committers"> | ||
// Licensed to the Software Freedom Conservancy (SFC) under one | ||
// or more contributor license agreements. See the NOTICE file | ||
// distributed with this work for additional information | ||
// regarding copyright ownership. The SFC licenses this file | ||
// to you under the Apache License, Version 2.0 (the | ||
// "License"); you may not use this file except in compliance | ||
// with the License. You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, | ||
// software distributed under the License is distributed on an | ||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
// KIND, either express or implied. See the License for the | ||
// specific language governing permissions and limitations | ||
// under the License. | ||
// </copyright> | ||
|
||
namespace OpenQA.Selenium.BiDi.Extensions.Permissions; | ||
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. I propose to simplify namespace to be "regular" module, but with 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. And given that we will have public |
||
|
||
internal record PermissionDescriptor(string Name); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
// <copyright file="PermissionState.cs" company="Selenium Committers"> | ||
// Licensed to the Software Freedom Conservancy (SFC) under one | ||
// or more contributor license agreements. See the NOTICE file | ||
// distributed with this work for additional information | ||
// regarding copyright ownership. The SFC licenses this file | ||
// to you under the Apache License, Version 2.0 (the | ||
// "License"); you may not use this file except in compliance | ||
// with the License. You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, | ||
// software distributed under the License is distributed on an | ||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
// KIND, either express or implied. See the License for the | ||
// specific language governing permissions and limitations | ||
// under the License. | ||
// </copyright> | ||
|
||
using OpenQA.Selenium.BiDi.Communication.Json.Converters; | ||
using System.Text.Json.Serialization; | ||
|
||
namespace OpenQA.Selenium.BiDi.Extensions.Permissions; | ||
|
||
[JsonConverter(typeof(CamelCaseEnumConverter<PermissionState>))] | ||
public enum PermissionState | ||
{ | ||
Granted, | ||
Denied, | ||
Prompt | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
using OpenQA.Selenium.BiDi.Browser; | ||
using OpenQA.Selenium.BiDi.Communication; | ||
using OpenQA.Selenium.BiDi.Communication.Json.Converters; | ||
using System.Text.Json; | ||
using System.Text.Json.Serialization; | ||
using System.Threading.Tasks; | ||
|
||
namespace OpenQA.Selenium.BiDi.Extensions.Permissions; | ||
|
||
public class PermissionsModule : Module | ||
{ | ||
private PermissionsJsonSerializerContext JsonContext => (PermissionsJsonSerializerContext)base.JsonContext; | ||
|
||
public async Task SetPermissionAsync(string permissionName, PermissionState state, string origin, UserContext? userContext, SetPermissionOptions? options = null) | ||
{ | ||
var @params = new SetPermissionCommandParameters(new PermissionDescriptor(permissionName), state, origin, userContext); | ||
|
||
await Broker.ExecuteCommandAsync(new SetPermissionCommand(@params), options, JsonContext.Permissions_SetPermissionCommand, JsonContext.Permissions_SetPermissionResult).ConfigureAwait(false); | ||
} | ||
|
||
protected override JsonSerializerContext Initialize(JsonSerializerOptions options) | ||
{ | ||
return new PermissionsJsonSerializerContext(options); | ||
} | ||
} | ||
|
||
[JsonSerializable(typeof(SetPermissionCommand), TypeInfoPropertyName = "Permissions_SetPermissionCommand")] | ||
[JsonSerializable(typeof(SetPermissionResult), TypeInfoPropertyName = "Permissions_SetPermissionResult")] | ||
internal partial class PermissionsJsonSerializerContext : JsonSerializerContext; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
using OpenQA.Selenium.BiDi.Browser; | ||
using OpenQA.Selenium.BiDi.Communication; | ||
using OpenQA.Selenium.BiDi.Communication.Json.Converters; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Text; | ||
using System.Text.Json.Serialization; | ||
|
||
namespace OpenQA.Selenium.BiDi.Extensions.Permissions; | ||
|
||
internal class SetPermissionCommand(SetPermissionCommandParameters @params) | ||
: Command<SetPermissionCommandParameters, SetPermissionResult>(@params, "permissions.setPermission"); | ||
|
||
public class SetPermissionOptions : CommandOptions; | ||
public sealed record SetPermissionResult : EmptyResult; | ||
|
||
internal record SetPermissionCommandParameters(PermissionDescriptor Descriptor, PermissionState State, string Origin, UserContext? UserContext) : Parameters; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,29 +18,28 @@ | |
// </copyright> | ||
|
||
using OpenQA.Selenium.BiDi.Communication; | ||
using OpenQA.Selenium.BiDi.Communication.Json; | ||
using System.Text.Json; | ||
using System.Text.Json.Serialization; | ||
|
||
namespace OpenQA.Selenium.BiDi; | ||
|
||
public abstract class Module | ||
{ | ||
protected Broker Broker { get; private set; } | ||
|
||
internal BiDiJsonSerializerContext JsonContext { get; private set; } | ||
internal JsonSerializerContext JsonContext { get; private set; } | ||
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. I am not ready to generalize it in this PR. Still not clear how to move on. 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. I have an example in this PR’s description, of how to move forward with these changes. 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. I though making 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. Observed it is theoretical changes. Let be practicable. 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. It is, except for the shared JSON context introduced in #16402 It is not theoretical changes. 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. I came to the following:
Then changes looks simple. 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. I feel introducing new
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. All right, |
||
|
||
protected virtual void Initialize(JsonSerializerOptions options) { } | ||
protected abstract JsonSerializerContext Initialize(JsonSerializerOptions options); | ||
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. We should choose between |
||
|
||
internal static TModule Create<TModule>(BiDi bidi, Broker broker, JsonSerializerOptions jsonOptions, BiDiJsonSerializerContext context) | ||
public static TModule Create<TModule>(BiDi bidi, JsonSerializerOptions jsonOptions, JsonSerializerContext? cachedContext = null) | ||
where TModule : Module, new() | ||
{ | ||
TModule module = new() | ||
{ | ||
Broker = broker, | ||
JsonContext = context | ||
Broker = bidi.Broker, | ||
}; | ||
|
||
module.Initialize(jsonOptions); | ||
module.JsonContext = cachedContext ?? module.Initialize(jsonOptions); | ||
|
||
return module; | ||
} | ||
|
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.
@RenderMichael this piece is strange, seems there is no reason to keep JsonContext in base class. If you have the same opinion, I can remake it to private field.