Skip to content

Commit

Permalink
Another path rework (#345)
Browse files Browse the repository at this point in the history
* Add Debug.Assert to dangerous methods

* Update AbsolutePath API

* Cleanup

* Add DangerousGetReferenceAt for Span<T>

* Update namespaces

* Make ThrowHelpers static

* Update namespaces

* Add OSInformation.ToString

* Update RelativePath

* Add benchmark

* Update Extension

* Rework AbsolutePath and RelativePath

* Update docs

* Remove CombineChecked

* Update usages and tests

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Merge fixes

* Fix tests

* Fix tests

* Fix tests

* Fix tests on Windows
  • Loading branch information
erri120 authored Jun 13, 2023
1 parent 464cf70 commit dc12a50
Show file tree
Hide file tree
Showing 122 changed files with 2,807 additions and 2,330 deletions.
1 change: 1 addition & 0 deletions NexusMods.App.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@
<s:Boolean x:Key="/Default/UserDictionary/Words/=skyrim/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Stardew/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Unpersisted/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=unsanitized/@EntryIndexedValue">True</s:Boolean>
</wpf:ResourceDictionary>
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
using BenchmarkDotNet.Attributes;
using NexusMods.Benchmarks.Interfaces;
using NexusMods.Paths.HighPerformance.Backports.System.Globalization;

namespace NexusMods.Benchmarks.Benchmarks;

[MemoryDiagnoser]
[BenchmarkInfo("Change Case + Ordinal vs OrdinalIgnoreCase", "Compares Change Case + Ordinal vs OrdinalIgnoreCase")]
public class ChangeCaseVsOrdinalIgnoreCase : IBenchmark
{
[Params(4, 16, 64, 256, 1024)]
// ReSharper disable once UnusedAutoPropertyAccessor.Global
public int StringLength { get; set; }

private string _value1 = null!;
private string _value2 = null!;

[GlobalSetup]
public void Setup()
{
_value1 = RandomStringUpper(StringLength);
_value2 = _value1 + "_";
}

[Benchmark(Baseline = true)]
public bool WithOrdinalIgnoreCase()
{
return string.Equals(_value1, _value2, StringComparison.OrdinalIgnoreCase);
}

[Benchmark]
public bool WithChangeCaseOrdinal()
{
var span1 = _value1.Length > 512
? GC.AllocateUninitializedArray<char>(_value1.Length)
: stackalloc char[_value1.Length];
TextInfo.ChangeCase<TextInfo.ToLowerConversion>(_value1, span1);

var span2 = _value1.Length > 512
? GC.AllocateUninitializedArray<char>(_value2.Length)
: stackalloc char[_value2.Length];
TextInfo.ChangeCase<TextInfo.ToLowerConversion>(_value2, span2);

return span1.SequenceEqual(span2);
}

private static string RandomString(int length, string charSet)
{
return new string(Enumerable.Repeat(charSet, length)
.Select(s => s[Random.Shared.Next(s.Length)]).ToArray());
}

private static string RandomStringUpper(int length) => RandomString(length, "ABCDEFGHIJKLMNOPQRSTUVWXYZ");
}
8 changes: 3 additions & 5 deletions benchmarks/NexusMods.Benchmarks/Benchmarks/EnumerateFiles.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using BenchmarkDotNet.Attributes;
using NexusMods.Benchmarks.Interfaces;
using NexusMods.Paths;
using NexusMods.Paths.Extensions;

namespace NexusMods.Benchmarks.Benchmarks;

Expand All @@ -16,7 +15,7 @@ public EnumerateFiles()
_fileSystem = FileSystem.Shared;
FilePath = _fileSystem.GetKnownPath(KnownPath.ApplicationDataDirectory);
}

// Placeholder path that'll probably work for people using Windows.
// Point this at your games' directory.
public AbsolutePath FilePath { get; }
Expand Down Expand Up @@ -94,7 +93,6 @@ public AbsolutePath EnumerateDirectories_Old()

private struct OriginalImplementation
{

public static IEnumerable<AbsolutePath> EnumerateFiles(AbsolutePath path, string pattern = "*", bool recursive = true)
{
return Directory.EnumerateFiles(path.GetFullPath(), pattern,
Expand All @@ -104,7 +102,7 @@ public static IEnumerable<AbsolutePath> EnumerateFiles(AbsolutePath path, string
RecurseSubdirectories = recursive,
MatchType = MatchType.Win32
})
.Select(file => file.ToAbsolutePath(FileSystem.Shared));
.Select(file => FileSystem.Shared.FromUnsanitizedFullPath(file));
}

public static IEnumerable<AbsolutePath> EnumerateDirectories(AbsolutePath path, bool recursive = true)
Expand All @@ -119,7 +117,7 @@ public static IEnumerable<AbsolutePath> EnumerateDirectories(AbsolutePath path,
RecurseSubdirectories = recursive,
MatchType = MatchType.Win32
})
.Select(p => p.ToAbsolutePath(FileSystem.Shared));
.Select(file => FileSystem.Shared.FromUnsanitizedFullPath(file));
}

// public static IEnumerable<FileEntry> EnumerateFileEntries(AbsolutePath path, string pattern = "*",
Expand Down
8 changes: 4 additions & 4 deletions benchmarks/NexusMods.Benchmarks/Benchmarks/Paths.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ public Paths()
var longPath = @"c:\" + string.Join(@"\", Enumerable.Range(0, 20).Select(x => $"path_{x}"));
var shortestPath = @"c:\foo";
_shortPath = @"c:\foo\bar\baz";
_nexusShortPath = _shortPath.ToAbsolutePath(FileSystem.Shared);
_nexusShortPath = FileSystem.Shared.FromUnsanitizedFullPath(_shortPath);
var cPath = @"c:\";

_paths = new[] { longPath, _shortPath, shortestPath, cPath };
}

public IEnumerable<(string StringPath, AbsolutePath AbsolutePath)> AllPaths =>
_paths.Select(p => (p, p.ToAbsolutePath(FileSystem.Shared)));
_paths.Select(p => (p, FileSystem.Shared.FromUnsanitizedFullPath(p)));

[ParamsSource(nameof(AllPaths))]
// ReSharper disable once UnusedAutoPropertyAccessor.Global
Expand Down Expand Up @@ -91,7 +91,7 @@ public AbsolutePath NexusJoinSmall()
{
// Not a fair test here since one is a string concat; other includes
// normalization to OS path.
return CurrentPath.AbsolutePath.CombineUnchecked("foo");
return CurrentPath.AbsolutePath.Combine("foo");
}

[Benchmark]
Expand All @@ -103,7 +103,7 @@ public string SystemJoinLarge()
[Benchmark]
public AbsolutePath NexusJoinLarge()
{
return CurrentPath.AbsolutePath.CombineUnchecked("foo/bar/baz/quz/qax");
return CurrentPath.AbsolutePath.Combine("foo/bar/baz/quz/qax");
}

[Benchmark]
Expand Down
51 changes: 0 additions & 51 deletions docs/decisions/backend/0003-case-sensitivity.md

This file was deleted.

88 changes: 88 additions & 0 deletions docs/decisions/backend/0003-paths.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Paths

## Context and Problem Statement

1) The .NET runtime uses `string` to represent a path. This goes against [nominal typing](../project/0004-use-nominal-typing.md).
2) Different platforms support different path separators. This makes path creation difficult and prevents using hardcoded paths.
3) [Case sensitivity](https://github.com/dotnet/runtime/blob/main/src/libraries/Common/src/System/IO/PathInternal.CaseSensitivity.cs) is dependent on the platform and file system.

| OS | [Separator Character] | [Alternative Separator Character] |
|------------|-----------------------|-----------------------------------|
| Windows | `\\` | `/` |
| Unix-based | `/` | `/` |

[Separator Character]: https://learn.microsoft.com/en-us/dotnet/api/system.io.path.directoryseparatorchar#remarks
[Alternative Separator Character]: https://learn.microsoft.com/en-us/dotnet/api/system.io.path.altdirectoryseparatorchar#remarks

| OS | File System | Case-sensitive |
|---------|----------------|----------------------------------------------------|
| Windows | [NTFS] | No when using the Win32 API, Yes when using POSIX. |
| Linux | [NTFS3] | No by default, can be changed using mount options. |
| Linux | ext4 and other | Yes, most POSIX file systems are case-sensitive. |
| macOS | [APFS] | No by default, can be changed with Disk Utility. |

[NTFS]: https://en.wikipedia.org/wiki/NTFS
[NTFS3]: https://www.kernel.org/doc/html/latest/filesystems/ntfs.html
[APFS]: https://support.apple.com/guide/disk-utility/file-system-formats-dsku19ed921c/mac

## Key Observations

The forward slash `/` can be used across all platforms, while the backwards slash `\\` can only be used on Windows. Microsoft [recommends](https://learn.microsoft.com/en-us/dotnet/api/system.io.path.directoryseparatorchar#remarks) using the forward slash `/` character for cross-platforms applications.

Aside from some edge-cases with POSIX compliant file systems, paths can be considered case-insensitive. Having two files with the _same name_ in the _same folder_ that differ only in capitalization is an anti-pattern and should not be supported. Archive formats are the biggest culprits of this, and we should recommend that the Nexus Web APIs be updated to reject archives with those files.

## Decision Outcome

### Directory Separator Character

Paths **must** use the forward slash `/` as the **only** directory separator character.

### Root Directories

Root directories are the only directories that are allowed to end with a [directory separator character](#directory-separator-character):

- Windows: `C:/`
- Unix-based: `/`

The parent directory of a root directory is **always** the root directory itself, signaling to any consumer that this is the top of the path hierarchy.

Only classic Windows paths that start with a drive letter are supported. UNC paths or anything else is **not supported**.

### Path Sanitization

Raw paths that haven't been created programatically but were provided by the OS or by the User, are considered _unsanitized_ and **must** be sanitized before they can be used. Sanitization involves the following process:

1) Remove trailing whitespaces.
2) Remove trailing [directory separator characters](#directory-separator-character).
3) On Windows: replace backwards slashes with forwards slashes.

Paths containing relative dots (`..`) are **always** considered unsanitized and **shall not** be resolved in code.

### Value Objects

Value objects **shall** be used for paths.

#### `AbsolutePath`

`AbsolutePath` represents a fully rooted path, meaning it starts with a [root directory](#root-directories) and contains a **required** `Directory` part and an **optional** `FileName` part. Examples:

| Full Path | `Directory` | `FileName` |
|--------------|-------------|------------------|
| `/` | `/` | `<empty string>` |
| `/foo` | `/` | `foo` |
| `/foo/bar` | `/foo` | `bar` |
| `C:/` | `C:/` | `<empty string>` |
| `C:/foo` | `C:/` | `foo` |
| `C:/foo/bar` | `C:/foo` | `bar` |

The `Directory` part **must not** end with a [directory separator character](#directory-separator-character), **unless** it's a root directory.

#### `RelativePath`

`RelativePath` represents a non-rooted part of a path. Examples:

- `<empty string>`
- `foo`
- `foo/bar`

A `RelativePath` can be nested multiple directories deep (eg: `foo/bar/baz`) and can be hardcoded.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class SevenZipExtractor : IExtractor
private readonly ILogger<SevenZipExtractor> _logger;
private readonly IResource<IExtractor, Size> _limiter;

private static readonly IOSInformation OSInformation = Common.OSInformation.Shared;
private static readonly IOSInformation OSInformation = Paths.OSInformation.Shared;

private static readonly FileType[] SupportedTypesCached = { FileType._7Z, FileType.RAR_NEW, FileType.RAR_OLD, FileType.ZIP };
private static readonly Extension[] SupportedExtensionsCached = { KnownExtensions._7z, KnownExtensions.Rar, KnownExtensions.Zip, KnownExtensions._7zip };
Expand All @@ -51,8 +51,7 @@ public SevenZipExtractor(ILogger<SevenZipExtractor> logger, TemporaryFileManager
_logger = logger;
_manager = fileManager;
_limiter = limiter;
_exePath = fileSystem.GetKnownPath(KnownPath.EntryDirectory)
.CombineChecked(GetExeLocation().ToRelativePath()).ToString();
_exePath = fileSystem.GetKnownPath(KnownPath.EntryDirectory).Combine(GetExeLocation().ToRelativePath()).ToString();
}

/// <inheritdoc />
Expand Down Expand Up @@ -167,8 +166,8 @@ private static string GetExeLocation()
throw new NotSupportedException($"{nameof(NexusMods.FileExtractor)}'s {nameof(SevenZipExtractor)} only supports x64 processors.");

return OSInformation.MatchPlatform(
onWindows: () => @"runtimes\win-x64\native\7z.exe",
onLinux: () => @"runtimes/linux-x64/native/7zz",
onWindows: () => "runtimes/win-x64/native/7z.exe",
onLinux: () => "runtimes/linux-x64/native/7zz",
onOSX: () => "runtimes/osx-x64/native/7zz"
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public FileExtractorSettings(IFileSystem fileSystem)
{
_fileSystem = fileSystem;
TempFolderLocation = new ConfigurationPath(_fileSystem
.GetKnownPath(KnownPath.EntryDirectory).CombineUnchecked("Temp"));
.GetKnownPath(KnownPath.EntryDirectory).Combine("Temp"));
}

/// <inheritdoc />
Expand Down
Loading

0 comments on commit dc12a50

Please sign in to comment.