Skip to content

Commit

Permalink
address feedback and cross plat tests
Browse files Browse the repository at this point in the history
  • Loading branch information
joem-msft committed Jun 21, 2024
1 parent ba40c6f commit 611ec60
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/Persistence.Tests/Persistence.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
<PackageReference Include="MSTest" Version="$(MSTest)" />
<PackageReference Include="coverlet.collector" Version="$(CoverletCollectorVersion)" />
<PackageReference Include="FluentAssertions" Version="$(FluentAssertionsVersion)" />
<PackageReference Include="Moq" Version="4.16.0" />
<PackageReference Include="Moq" Version="$(MoqVersion)" />
</ItemGroup>

<ItemGroup Label="Global usings">
Expand Down
11 changes: 5 additions & 6 deletions src/Persistence/MsApp/IMsappArchive.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,17 @@ public interface IMsappArchive : IDisposable
/// <summary>
/// Attempts to generate a unique entry path in the specified directory, based on a starter file name and extension.
/// </summary>
/// <param name="directory">The directory path for the entry; or null. Note: Directory path is expected to already be safe, as it is expected to not contain customer data.</param>
/// <param name="fileNameNoExtension">The name of the file, without an extension. This file name should already have been made 'safe' by the caller. If needed, use <see cref="MsappArchive.TryMakeSafeForEntryPathSegment"/> to make it safe.</param>
/// <param name="extension">The file extension to add for the file path or null for no extension. Note: This should already contain only safe chars, as it is expected to not contain customer data.</param>
/// <param name="directory">The directory path for the entry; or null. Note: Directory path is expected to already be safe, as it is expected to not contain customer data.</param>
/// <param name="uniqueSuffixSeparator">The string added just before the unique file number and extension. Default is empty string.</param>
/// <param name="entryPath">The entry path which is unique in the specified <paramref name="directory"/>.</param>
/// <returns>A value indicating whether a unique entry was able to be created. If true, then <paramref name="entryPath"/> will contain the unique path.</returns>
/// <returns>The entry path which is unique in the specified <paramref name="directory"/>.</returns>
/// <exception cref="ArgumentException">directory is empty or whitespace.</exception>
bool TryGenerateUniqueEntryPath(
string? directory,
/// <exception cref="InvalidOperationException">A unique filename entry could not be generated.</exception>
string GenerateUniqueEntryPath(
string fileNameNoExtension,
string? extension,
[NotNullWhen(true)] out string? entryPath,
string? directory = null,
string uniqueSuffixSeparator = "");

/// <summary>
Expand Down
48 changes: 32 additions & 16 deletions src/Persistence/MsApp/MsappArchive.cs
Original file line number Diff line number Diff line change
Expand Up @@ -458,46 +458,47 @@ private string GetSafeEntryPath(string directory, string name, string extension)
if (!TryMakeSafeForEntryPathSegment(name, out var safeName, unsafeCharReplacementText: ""))
throw new ArgumentException("Control name is not valid.", nameof(name));

if (!TryGenerateUniqueEntryPath(directory.WhiteSpaceToNull(), safeName, extension, out var entryPath))
{
throw new InvalidOperationException("Failed to find a unique name for the control.");
}

return entryPath;
return GenerateUniqueEntryPath(safeName, extension, directory: directory.WhiteSpaceToNull());
}

/// <inheritdoc/>
public bool TryGenerateUniqueEntryPath(
string? directory,
public string GenerateUniqueEntryPath(
string fileNameNoExtension,
string? extension,
[NotNullWhen(true)] out string? entryPath,
string? directory = null,
string uniqueSuffixSeparator = "")
{
_ = fileNameNoExtension ?? throw new ArgumentNullException(nameof(fileNameNoExtension));
if (!IsSafeForEntryPathSegment(fileNameNoExtension))
{
throw new ArgumentException($"The {nameof(fileNameNoExtension)} must be safe for use as an entry path segment. Prevalidate using {nameof(TryMakeSafeForEntryPathSegment)} first.", nameof(fileNameNoExtension));
}
if (extension != null && !IsSafeForEntryPathSegment(extension))
{
throw new ArgumentException("The extension can be null, but cannot be empty or whitespace only, and must be a valid entry path segment.", nameof(directory));
}
if (directory != null && string.IsNullOrWhiteSpace(directory))
{
throw new ArgumentException("The directory can be null, but cannot be empty or whitespace only.", nameof(directory));
}
_ = !string.IsNullOrEmpty(fileNameNoExtension) ? fileNameNoExtension : throw new ArgumentNullException(nameof(fileNameNoExtension));

var entryPathPrefix = directory == null ? fileNameNoExtension : Path.Combine(directory, fileNameNoExtension);
var entryPathPrefix = directory == null ? fileNameNoExtension : Path.Combine(directory, fileNameNoExtension).Replace('\\', '/');

// First see if we can use the name as is
entryPath = $"{entryPathPrefix}{extension}";
var entryPath = $"{entryPathPrefix}{extension}";
if (!DoesEntryExist(entryPath))
return true;
return entryPath;

// If file with the same name already exists, add a number to the end of the name
entryPathPrefix += uniqueSuffixSeparator;
for (var i = 1; i < int.MaxValue; i++)
{
entryPath = $"{entryPathPrefix}{i}{extension}";
if (!DoesEntryExist(entryPath))
return true;
return entryPath;
}

entryPath = null;
return false;
throw new InvalidOperationException("Failed to generate a unique name.");
}

public void Save()
Expand Down Expand Up @@ -560,6 +561,21 @@ public static bool TryMakeSafeForEntryPathSegment(
return safeName != null;
}

/// <summary>
/// Used to verify that a name is safe for use as a single path segment for an entry.
/// Directory separator chars are not allowed in a path segment.
/// </summary>
/// <param name="name">The proposed path segment name.</param>
/// <returns>false when <paramref name="name"/> is null, empty, whitespace only, has leading or trailing whitespace, contains path separator chars or contains any other invalid chars.</returns>
public static bool IsSafeForEntryPathSegment(string name)
{
_ = name ?? throw new ArgumentNullException(nameof(name));

return !string.IsNullOrWhiteSpace(name)
&& !UnsafeFileNameCharactersRegex().IsMatch(name)
&& name.Trim() == name; // No leading or trailing whitespace
}

/// <summary>
/// Regular expression that matches any characters that are unsafe for entry filenames.<br/>
/// Note: we don't allow any sort of directory separator chars for filenames to remove cross-platform issues.
Expand Down
3 changes: 3 additions & 0 deletions src/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,8 @@
<FluentAssertionsVersion>6.12.0</FluentAssertionsVersion>
<MinVerVersion>4.3.0</MinVerVersion>
<MicrosoftExtensionsLoggingVersion>7.0.0</MicrosoftExtensionsLoggingVersion>

<!-- WARNING: Do not update Moq library past 4.20.0 due to privacy issues: https://github.com/devlooped/moq/issues/1372 -->
<MoqVersion>4.16.0</MoqVersion>
</PropertyGroup>
</Project>

0 comments on commit 611ec60

Please sign in to comment.