-
Notifications
You must be signed in to change notification settings - Fork 9
[refactor] Centralize filesystem entry classification in inventory flow #276
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: master
Are you sure you want to change the base?
Changes from all commits
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 | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||
| using System.IO; | ||||||
| using ByteSync.Business.Inventories; | ||||||
| using ByteSync.Common.Business.Misc; | ||||||
| using ByteSync.Interfaces.Controls.Inventories; | ||||||
|
|
||||||
|
|
@@ -7,6 +8,48 @@ namespace ByteSync.Services.Inventories; | |||||
| public class FileSystemInspector : IFileSystemInspector | ||||||
| { | ||||||
| private const int FILE_ATTRIBUTE_RECALL_ON_DATA_ACCESS = 4194304; | ||||||
| private readonly IPosixFileTypeClassifier _posixFileTypeClassifier; | ||||||
|
|
||||||
| public FileSystemInspector(IPosixFileTypeClassifier? posixFileTypeClassifier = null) | ||||||
| { | ||||||
| _posixFileTypeClassifier = posixFileTypeClassifier ?? new PosixFileTypeClassifier(); | ||||||
| } | ||||||
|
|
||||||
| public FileSystemEntryKind ClassifyEntry(FileSystemInfo fsi) | ||||||
| { | ||||||
| if (fsi is null) | ||||||
| { | ||||||
| return FileSystemEntryKind.Unknown; | ||||||
| } | ||||||
|
|
||||||
| if (HasLinkTarget(fsi) || SafeIsReparsePoint(fsi)) | ||||||
| { | ||||||
| return FileSystemEntryKind.Symlink; | ||||||
| } | ||||||
|
|
||||||
| if (!OperatingSystem.IsWindows()) | ||||||
| { | ||||||
| try | ||||||
| { | ||||||
| var posixKind = _posixFileTypeClassifier.ClassifyPosixEntry(fsi.FullName); | ||||||
| if (posixKind != FileSystemEntryKind.Unknown) | ||||||
| { | ||||||
| return posixKind; | ||||||
| } | ||||||
| } | ||||||
| catch (Exception) | ||||||
| { | ||||||
| return FileSystemEntryKind.Unknown; | ||||||
|
||||||
| return FileSystemEntryKind.Unknown; | |
| // Ignore POSIX classification failures and fall back to type-based classification below. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| using ByteSync.Business.Inventories; | ||
| using ByteSync.Interfaces.Controls.Inventories; | ||
| using ByteSync.Services.Inventories; | ||
| using FluentAssertions; | ||
| using Moq; | ||
| using NUnit.Framework; | ||
|
|
||
| namespace ByteSync.Client.UnitTests.Services.Inventories; | ||
|
|
||
| public class FileSystemInspectorTests | ||
| { | ||
| [Test] | ||
| public void ClassifyEntry_ReturnsDirectory_ForDirectoryInfo() | ||
| { | ||
| var posix = new Mock<IPosixFileTypeClassifier>(MockBehavior.Strict); | ||
| posix.Setup(p => p.ClassifyPosixEntry(It.IsAny<string>())).Returns(FileSystemEntryKind.Unknown); | ||
| var inspector = new FileSystemInspector(posix.Object); | ||
| var tempDirectory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"))); | ||
|
|
||
| try | ||
| { | ||
| var result = inspector.ClassifyEntry(tempDirectory); | ||
|
|
||
| result.Should().Be(FileSystemEntryKind.Directory); | ||
| } | ||
| finally | ||
| { | ||
| Directory.Delete(tempDirectory.FullName, true); | ||
| } | ||
| } | ||
|
|
||
| [Test] | ||
| public void ClassifyEntry_ReturnsRegularFile_ForFileInfo() | ||
| { | ||
| var posix = new Mock<IPosixFileTypeClassifier>(MockBehavior.Strict); | ||
| posix.Setup(p => p.ClassifyPosixEntry(It.IsAny<string>())).Returns(FileSystemEntryKind.Unknown); | ||
| var inspector = new FileSystemInspector(posix.Object); | ||
| var tempDirectory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"))); | ||
| var tempFilePath = Path.Combine(tempDirectory.FullName, "file.txt"); | ||
| File.WriteAllText(tempFilePath, "x"); | ||
| var fileInfo = new FileInfo(tempFilePath); | ||
|
|
||
| try | ||
| { | ||
| var result = inspector.ClassifyEntry(fileInfo); | ||
|
|
||
| result.Should().Be(FileSystemEntryKind.RegularFile); | ||
| } | ||
| finally | ||
| { | ||
| Directory.Delete(tempDirectory.FullName, true); | ||
| } | ||
| } | ||
|
|
||
| [Test] | ||
| public void ClassifyEntry_ReturnsSymlink_WhenLinkTargetExists() | ||
| { | ||
| var posix = new Mock<IPosixFileTypeClassifier>(MockBehavior.Strict); | ||
| posix.Setup(p => p.ClassifyPosixEntry(It.IsAny<string>())).Returns(FileSystemEntryKind.Unknown); | ||
| var inspector = new FileSystemInspector(posix.Object); | ||
| var tempDirectory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"))); | ||
| var targetPath = Path.Combine(tempDirectory.FullName, "target.txt"); | ||
| File.WriteAllText(targetPath, "x"); | ||
| var linkPath = Path.Combine(tempDirectory.FullName, "link.txt"); | ||
|
|
||
| try | ||
| { | ||
| try | ||
| { | ||
| File.CreateSymbolicLink(linkPath, targetPath); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Assert.Ignore($"Symbolic link creation failed: {ex.GetType().Name}"); | ||
| } | ||
|
|
||
| var result = inspector.ClassifyEntry(new FileInfo(linkPath)); | ||
|
|
||
| result.Should().Be(FileSystemEntryKind.Symlink); | ||
| } | ||
| finally | ||
| { | ||
| Directory.Delete(tempDirectory.FullName, true); | ||
| } | ||
| } | ||
|
|
||
| [Test] | ||
| [Platform(Include = "Linux,MacOsX")] | ||
| public void ClassifyEntry_ReturnsPosixSpecialKind_WhenClassifierProvidesOne() | ||
| { | ||
| var posix = new Mock<IPosixFileTypeClassifier>(MockBehavior.Strict); | ||
| posix.Setup(p => p.ClassifyPosixEntry(It.IsAny<string>())).Returns(FileSystemEntryKind.Fifo); | ||
| var inspector = new FileSystemInspector(posix.Object); | ||
| var tempDirectory = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"))); | ||
| var tempFilePath = Path.Combine(tempDirectory.FullName, "file.txt"); | ||
| File.WriteAllText(tempFilePath, "x"); | ||
| var fileInfo = new FileInfo(tempFilePath); | ||
|
|
||
| try | ||
| { | ||
| var result = inspector.ClassifyEntry(fileInfo); | ||
|
|
||
| result.Should().Be(FileSystemEntryKind.Fifo); | ||
| } | ||
| finally | ||
| { | ||
| Directory.Delete(tempDirectory.FullName, true); | ||
| } | ||
| } | ||
|
|
||
| [Test] | ||
| public void ClassifyEntry_ReturnsUnknown_ForNullEntry() | ||
| { | ||
| var posix = new Mock<IPosixFileTypeClassifier>(MockBehavior.Strict); | ||
| posix.Setup(p => p.ClassifyPosixEntry(It.IsAny<string>())).Returns(FileSystemEntryKind.Unknown); | ||
| var inspector = new FileSystemInspector(posix.Object); | ||
|
|
||
| var result = inspector.ClassifyEntry(null!); | ||
|
|
||
| result.Should().Be(FileSystemEntryKind.Unknown); | ||
| } | ||
| } |
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.
ClassifyEntrydefensively handlesnullinputs, but the interface contract uses a non-nullableFileSystemInfoparameter. With nullable enabled, consider changing the signature toFileSystemInfo?to reflect the supported input and avoid callers needingnull!in tests.