Skip to content

Commit 153fddb

Browse files
BornToBeRootBornToBeRoot
committed
Fix: Code review fixes
1 parent e6ef255 commit 153fddb

12 files changed

Lines changed: 111 additions & 33 deletions

File tree

‎Source/NETworkManager.Models/RunCommand/RunCommandTarget.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,15 @@ namespace NETworkManager.Models.RunCommand;
1414
public static class RunCommandTarget
1515
{
1616
/// <summary>
17-
/// Checks if the ports of an entry of the port lookup (e.g. "22", "80-90/tcp") are valid: from 1 to 65535, a range
18-
/// from a lower to a higher port.
17+
/// Checks if the ports of an entry of the port lookup (e.g. "22", "80-90/tcp") are valid, like the port lookup.
1918
/// </summary>
2019
private static bool IsValidPortLookupEntry(string entry)
2120
{
2221
var ports = entry.Split('/')[0].Split('-').Select(x => int.TryParse(x, out var port) ? port : 0).ToArray();
2322

24-
return ports.All(x => x is > 0 and < 65536) && (ports.Length == 1 || ports[0] < ports[1]);
23+
return ports.Length == 1
24+
? PortRangeHelper.IsValidPort(ports[0])
25+
: PortRangeHelper.IsValidPortRange(ports[0], ports[1]);
2526
}
2627

2728
/// <summary>

‎Source/NETworkManager.Tests/Utilities/PortRangeHelperTests.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
using NETworkManager.Utilities;
2+
using Xunit;
3+
4+
namespace NETworkManager.Tests.Utilities;
5+
6+
public class PortRangeHelperTests
7+
{
8+
[Theory]
9+
[InlineData(1, true)]
10+
[InlineData(22, true)]
11+
[InlineData(65535, true)]
12+
[InlineData(0, false)]
13+
[InlineData(65536, false)]
14+
[InlineData(-1, false)]
15+
public void IsValidPort(int port, bool isValid)
16+
{
17+
Assert.Equal(isValid, PortRangeHelper.IsValidPort(port));
18+
}
19+
20+
[Theory]
21+
[InlineData(80, 90, true)]
22+
[InlineData(1, 65535, true)]
23+
[InlineData(90, 80, false)]
24+
[InlineData(80, 80, false)]
25+
[InlineData(0, 80, false)]
26+
[InlineData(80, 65536, false)]
27+
public void IsValidPortRange(int startPort, int endPort, bool isValid)
28+
{
29+
Assert.Equal(isValid, PortRangeHelper.IsValidPortRange(startPort, endPort));
30+
}
31+
}

‎Source/NETworkManager.Utilities/PortRangeHelper.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,27 @@ namespace NETworkManager.Utilities;
55

66
public static class PortRangeHelper
77
{
8+
/// <summary>
9+
/// Checks if the port is valid (1 to 65535).
10+
/// </summary>
11+
/// <param name="port">Port.</param>
12+
/// <returns>True if the port is valid.</returns>
13+
public static bool IsValidPort(int port)
14+
{
15+
return port is > 0 and < 65536;
16+
}
17+
18+
/// <summary>
19+
/// Checks if the port range is valid: both ports are valid and the range goes from a lower to a higher port.
20+
/// </summary>
21+
/// <param name="startPort">First port.</param>
22+
/// <param name="endPort">Last port.</param>
23+
/// <returns>True if the range is valid.</returns>
24+
public static bool IsValidPortRange(int startPort, int endPort)
25+
{
26+
return IsValidPort(startPort) && IsValidPort(endPort) && startPort < endPort;
27+
}
28+
829
public static Task<int[]> ConvertPortRangeToIntArrayAsync(string portRange)
930
{
1031
return Task.Run(() => ConvertPortRangeToIntArray(portRange));

‎Source/NETworkManager.Validators/EmptyOrPortRangeValidator.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Globalization;
22
using System.Windows.Controls;
33
using NETworkManager.Localization.Resources;
4+
using NETworkManager.Utilities;
45

56
namespace NETworkManager.Validators;
67

@@ -20,8 +21,7 @@ public override ValidationResult Validate(object value, CultureInfo cultureInfo)
2021

2122
if (int.TryParse(portRange[0], out var startPort) && int.TryParse(portRange[1], out var endPort))
2223
{
23-
if (!(startPort > 0 && startPort < 65536 && endPort > 0 && endPort < 65536 &&
24-
startPort < endPort))
24+
if (!PortRangeHelper.IsValidPortRange(startPort, endPort))
2525
isValid = false;
2626
}
2727
else
@@ -33,7 +33,7 @@ public override ValidationResult Validate(object value, CultureInfo cultureInfo)
3333
{
3434
if (int.TryParse(portOrRange, out var portNumber))
3535
{
36-
if (!(portNumber > 0 && portNumber < 65536))
36+
if (!PortRangeHelper.IsValidPort(portNumber))
3737
isValid = false;
3838
}
3939
else

‎Source/NETworkManager.Validators/PortRangeValidator.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Globalization;
22
using System.Windows.Controls;
33
using NETworkManager.Localization.Resources;
4+
using NETworkManager.Utilities;
45

56
namespace NETworkManager.Validators;
67

@@ -20,8 +21,7 @@ public override ValidationResult Validate(object value, CultureInfo cultureInfo)
2021

2122
if (int.TryParse(portRange[0], out var startPort) && int.TryParse(portRange[1], out var endPort))
2223
{
23-
if (!(startPort > 0 && startPort < 65536 && endPort > 0 && endPort < 65536 &&
24-
startPort < endPort))
24+
if (!PortRangeHelper.IsValidPortRange(startPort, endPort))
2525
isValid = false;
2626
}
2727
else
@@ -33,7 +33,7 @@ public override ValidationResult Validate(object value, CultureInfo cultureInfo)
3333
{
3434
if (int.TryParse(portOrRange, out var portNumber))
3535
{
36-
if (!(portNumber > 0 && portNumber < 65536))
36+
if (!PortRangeHelper.IsValidPort(portNumber))
3737
isValid = false;
3838
}
3939
else

‎Source/NETworkManager.Validators/PortValidator.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Globalization;
22
using System.Windows.Controls;
33
using NETworkManager.Localization.Resources;
4+
using NETworkManager.Utilities;
45

56
namespace NETworkManager.Validators;
67

@@ -11,7 +12,7 @@ public override ValidationResult Validate(object value, CultureInfo cultureInfo)
1112
if (!int.TryParse(value as string, out var portNumber))
1213
return new ValidationResult(false, Strings.EnterValidPort);
1314

14-
if (portNumber > 0 && portNumber < 65536)
15+
if (PortRangeHelper.IsValidPort(portNumber))
1516
return ValidationResult.ValidResult;
1617

1718
return new ValidationResult(false, Strings.EnterValidPort);

‎Source/NETworkManager/DialogHelper.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ namespace NETworkManager
1717
/// </summary>
1818
public static class DialogHelper
1919
{
20+
/// <summary>
21+
/// Occurs before a message or confirmation dialog is shown, e.g. so a dialog that stays open (the run command
22+
/// with Ctrl+Enter) can close itself first instead of the dialogs being shown on top of each other.
23+
/// </summary>
24+
public static event EventHandler DialogOpening;
25+
2026
/// <summary>
2127
/// Displays a modal message dialog with an OK button, allowing the user to acknowledge the message before
2228
/// continuing.
@@ -33,6 +39,8 @@ public static Task ShowMessageAsync(Window parentWindow, string title, string me
3339
{
3440
confirmButtonText ??= Strings.OK;
3541

42+
DialogOpening?.Invoke(null, EventArgs.Empty);
43+
3644
var childWindow = new MessageChildWindow();
3745

3846
var childWindowViewModel = new MessageViewModel(_ =>
@@ -72,6 +80,8 @@ public static async Task<bool> ShowConfirmationMessageAsync(Window parentWindow,
7280

7381
var result = false;
7482

83+
DialogOpening?.Invoke(null, EventArgs.Empty);
84+
7585
var childWindow = new MessageConfirmationChildWindow();
7686

7787
var childWindowViewModel = new MessageConfirmationViewModel(_ =>

‎Source/NETworkManager/MainWindow.xaml.cs‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,14 +1102,22 @@ private void OpenRunAction()
11021102

11031103
var childWindow = new RunCommandChildWindow(this);
11041104

1105+
// A message of an application that was run with Ctrl+Enter (the dialog stays open, e.g. an error of the Ping
1106+
// Monitor) closes the dialog first, so the dialogs are not shown on top of each other.
1107+
void DialogHelper_DialogOpening(object sender, EventArgs e) => CloseDialog();
1108+
11051109
void CloseDialog()
11061110
{
1111+
DialogHelper.DialogOpening -= DialogHelper_DialogOpening;
1112+
11071113
childWindow.IsOpen = false;
11081114
ConfigurationManager.Current.IsChildWindowOpen = false;
11091115

11101116
ConfigurationManager.OnDialogClose();
11111117
}
11121118

1119+
DialogHelper.DialogOpening += DialogHelper_DialogOpening;
1120+
11131121
var childWindowViewModel = new RunCommandViewModel(instance =>
11141122
{
11151123
// Ctrl+Enter: the dialog stays open to run another command.
@@ -1464,18 +1472,24 @@ private void UpdateOnWindowResize()
14641472
private void RegisterHotKeys()
14651473
{
14661474
if (SettingsManager.Current.HotKey_ShowWindowEnabled)
1467-
{
1468-
RegisterHotKey(new WindowInteropHelper(this).Handle, 1, SettingsManager.Current.HotKey_ShowWindowModifier,
1475+
RegisterHotKey(1, SettingsManager.Current.HotKey_ShowWindowModifier,
14691476
SettingsManager.Current.HotKey_ShowWindowKey);
1470-
_registeredHotKeys.Add(1);
1471-
}
14721477

14731478
if (SettingsManager.Current.HotKey_RunCommandEnabled)
1474-
{
1475-
RegisterHotKey(new WindowInteropHelper(this).Handle, 2, SettingsManager.Current.HotKey_RunCommandModifier,
1479+
RegisterHotKey(2, SettingsManager.Current.HotKey_RunCommandModifier,
14761480
SettingsManager.Current.HotKey_RunCommandKey);
1477-
_registeredHotKeys.Add(2);
1478-
}
1481+
}
1482+
1483+
/// <summary>
1484+
/// Registers a hotkey. It fails if the combination is already used (e.g. by the other hotkey or another
1485+
/// application), then it is logged and not unregistered later.
1486+
/// </summary>
1487+
private void RegisterHotKey(int id, int modifier, int key)
1488+
{
1489+
if (RegisterHotKey(new WindowInteropHelper(this).Handle, id, modifier, key))
1490+
_registeredHotKeys.Add(id);
1491+
else
1492+
Log.Warn($"Hotkey {id} could not be registered, the key combination is already in use.");
14791493
}
14801494

14811495
private void UnregisterHotKeys()

‎Source/NETworkManager/RunCommandManager.cs‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -192,21 +192,16 @@ public static IEnumerable<ProfileInfo> GetProfiles(RunCommandDefinition command)
192192
}
193193

194194
/// <summary>
195-
/// Checks if the request opens (or can open) a dialog, so the run command dialog must be closed: the settings,
196-
/// the connect dialog of a remote tool, or the Subnetting, which asks for confirmation for a large calculation
197-
/// and shows an error for an invalid subnet mask.
195+
/// Checks if the request opens a dialog, so the run command dialog must be closed: the settings or the connect
196+
/// dialog of a remote tool. Messages of the applications (e.g. an error or a confirmation) close the run command
197+
/// dialog when they are shown (<see cref="DialogHelper.DialogOpening" />).
198198
/// </summary>
199199
/// <param name="request">Request, or null for the settings.</param>
200200
/// <returns>True if the request opens a dialog.</returns>
201201
public static bool OpensDialog(ApplicationRequest request)
202202
{
203-
return request switch
204-
{
205-
null => true,
206-
SubnetCalculatorRequest { NewSubnetmask: not null } => true,
207-
_ => request.Profile == null && !string.IsNullOrEmpty(request.Target) &&
208-
ConnectDialogApplications.Contains(request.Application)
209-
};
203+
return request == null || request.Profile == null && !string.IsNullOrEmpty(request.Target) &&
204+
ConnectDialogApplications.Contains(request.Application);
210205
}
211206

212207
/// <summary>

‎Source/NETworkManager/ViewModels/LookupPortViewModel.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ private async Task PortLookupAction()
231231
// Check if the search is a port number like 22
232232
if (RegexHelper.PortLookupPortRegex().IsMatch(searchTrim))
233233
if (int.TryParse(searchTrim, out var port))
234-
if (port is > 0 and < 65536)
234+
if (PortRangeHelper.IsValidPort(port))
235235
{
236236
ports.Add(port);
237237
continue;
@@ -243,7 +243,7 @@ private async Task PortLookupAction()
243243
var portAndProtocol = searchTrim.Split('/');
244244

245245
if (int.TryParse(portAndProtocol[0], out var port))
246-
if (port is > 0 and < 65536)
246+
if (PortRangeHelper.IsValidPort(port))
247247
{
248248
portsAndProtocols.Add(new Tuple<int, string>(port, portAndProtocol[1]));
249249
continue;
@@ -256,7 +256,7 @@ private async Task PortLookupAction()
256256
var portRange = searchTrim.Split('-');
257257

258258
if (int.TryParse(portRange[0], out var startPort) && int.TryParse(portRange[1], out var endPort))
259-
if (startPort is > 0 and < 65536 && endPort is > 0 and < 65536 && startPort < endPort)
259+
if (PortRangeHelper.IsValidPortRange(startPort, endPort))
260260
{
261261
for (var i = startPort; i < endPort + 1; i++)
262262
ports.Add(i);
@@ -273,7 +273,7 @@ private async Task PortLookupAction()
273273
var portRange = portRangeAndProtocol[0].Split('-');
274274

275275
if (int.TryParse(portRange[0], out var startPort) && int.TryParse(portRange[1], out var endPort))
276-
if (startPort is > 0 and < 65536 && endPort is > 0 and < 65536 && startPort < endPort)
276+
if (PortRangeHelper.IsValidPortRange(startPort, endPort))
277277
{
278278
for (var i = startPort; i < endPort + 1; i++)
279279
portsAndProtocols.Add(new Tuple<int, string>(i, portRangeAndProtocol[1]));

0 commit comments

Comments
 (0)