Skip to content

Commit 3600e29

Browse files
committed
Refactor RedirectFile initialization and enhance IRepositoryTracker for improved redirect handling
- Simplified `RedirectFile` creation logic and constructor overloading. - Changed `IRepositoryTracker` to return a `IReadOnlyCollection` for consistency. - Enhanced path normalization across repository trackers. - Adjusted dependent tests and updated related YAML documentation.
1 parent 709a53b commit 3600e29

File tree

8 files changed

+68
-57
lines changed

8 files changed

+68
-57
lines changed

docs/contribute/redirects.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ This example strips all anchors from the source page.
8686
Any remaining links resolving to anchors on `7th-page.md` will fail link validation.
8787

8888
```yaml
89-
redirects
89+
redirects:
9090
'testing/redirects/7th-page.md':
9191
to: 'testing/redirects/5th-page.md'
9292
anchors: '!'

src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,10 @@ public ConfigurationFile(IDocumentationSetContext context, VersionsConfiguration
7373
return;
7474
}
7575

76-
var sourceFile = context.ConfigurationPath;
77-
var redirectFileName = sourceFile.Name.StartsWith('_') ? "_redirects.yml" : "redirects.yml";
78-
var redirectFileInfo = sourceFile.FileSystem.FileInfo.New(Path.Combine(sourceFile.Directory!.FullName, redirectFileName));
79-
var redirectFile = new RedirectFile(redirectFileInfo, _context);
76+
var redirectFile = new RedirectFile(_context);
8077
Redirects = redirectFile.Redirects;
8178

79+
var sourceFile = context.ConfigurationPath;
8280
var reader = new YamlStreamReader(sourceFile, _context.Collector);
8381
try
8482
{

src/Elastic.Documentation.Configuration/Builder/RedirectFile.cs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,19 @@ namespace Elastic.Documentation.Configuration.Builder;
1010

1111
public record RedirectFile
1212
{
13-
public Dictionary<string, LinkRedirect>? Redirects { get; set; }
14-
private IFileInfo Source { get; init; }
15-
private IDocumentationSetContext Context { get; init; }
13+
public Dictionary<string, LinkRedirect>? Redirects { get; }
14+
public IFileInfo Source { get; }
15+
private IDocumentationSetContext Context { get; }
1616

17-
public RedirectFile(IFileInfo source, IDocumentationSetContext context)
17+
public RedirectFile(IDocumentationSetContext context, IFileInfo? source = null)
1818
{
19-
Source = source;
19+
var docsetConfigurationPath = context.ConfigurationPath;
20+
var redirectFileName = docsetConfigurationPath.Name.StartsWith('_') ? "_redirects.yml" : "redirects.yml";
21+
var redirectFileInfo = docsetConfigurationPath.FileSystem.FileInfo.New(Path.Combine(docsetConfigurationPath.Directory!.FullName, redirectFileName));
22+
Source = source ?? redirectFileInfo;
2023
Context = context;
2124

22-
if (!source.Exists)
25+
if (!Source.Exists)
2326
return;
2427

2528
var reader = new YamlStreamReader(Source, Context.Collector);

src/tooling/docs-builder/Cli/DiffCommands.cs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,51 +40,55 @@ public async Task<int> ValidateRedirects(string? path = null, Cancel ctx = defau
4040
var fs = new FileSystem();
4141

4242
var buildContext = new BuildContext(collector, fs, fs, configurationContext, ExportOptions.MetadataOnly, path, null);
43-
var sourceFile = buildContext.ConfigurationPath;
44-
var redirectFileName = sourceFile.Name.StartsWith('_') ? "_redirects.yml" : "redirects.yml";
45-
var redirectFileInfo = sourceFile.FileSystem.FileInfo.New(Path.Combine(sourceFile.Directory!.FullName, redirectFileName));
46-
if (!redirectFileInfo.Exists)
43+
var redirectFile = new RedirectFile(buildContext);
44+
if (!redirectFile.Source.Exists)
4745
{
4846
await collector.StopAsync(ctx);
4947
return 0;
5048
}
5149

52-
var redirectFileParser = new RedirectFile(redirectFileInfo, buildContext);
53-
var redirects = redirectFileParser.Redirects;
50+
var redirects = redirectFile.Redirects;
5451

5552
if (redirects is null)
5653
{
57-
collector.EmitError(redirectFileInfo, "It was not possible to parse the redirects file.");
54+
collector.EmitError(redirectFile.Source, "It was not possible to parse the redirects file.");
5855
await collector.StopAsync(ctx);
5956
return collector.Errors;
6057
}
6158

6259
var root = Paths.DetermineSourceDirectoryRoot(buildContext.DocumentationSourceDirectory);
6360
if (root is null)
6461
{
65-
collector.EmitError(redirectFileInfo, $"Unable to determine the root of the source directory {buildContext.DocumentationSourceDirectory}.");
62+
collector.EmitError(redirectFile.Source, $"Unable to determine the root of the source directory {buildContext.DocumentationSourceDirectory}.");
6663
await collector.StopAsync(ctx);
6764
return collector.Errors;
6865
}
6966
var relativePath = Path.GetRelativePath(root.FullName, buildContext.DocumentationSourceDirectory.FullName);
7067
_log.LogInformation("Using relative path {RelativePath} for validating changes", relativePath);
7168
IRepositoryTracker tracker = runningOnCi ? new IntegrationGitRepositoryTracker(relativePath) : new LocalGitRepositoryTracker(collector, root, relativePath);
72-
var changed = tracker.GetChangedFiles().ToArray();
69+
var changed = tracker.GetChangedFiles();
7370

74-
if (changed.Length != 0)
75-
_log.LogInformation("Found {Count} changes to files related to documentation in the current branch.", changed.Length);
71+
if (changed.Count != 0)
72+
_log.LogInformation("Found {Count} changes to files related to documentation in the current branch.", changed.Count);
7673

77-
foreach (var notFound in changed.DistinctBy(c => c.FilePath).Where(c => c.ChangeType is GitChangeType.Deleted or GitChangeType.Renamed
78-
&& !redirects.ContainsKey(c is RenamedGitChange renamed ? renamed.OldFilePath : c.FilePath)))
74+
var missingRedirects = changed
75+
.DistinctBy(c => c.FilePath)
76+
.Where(c =>
77+
c.ChangeType is GitChangeType.Deleted or GitChangeType.Renamed
78+
&& !redirects.ContainsKey(c is RenamedGitChange renamed ? renamed.OldFilePath : c.FilePath)
79+
)
80+
.ToArray();
81+
82+
foreach (var notFound in missingRedirects)
7983
{
8084
if (notFound is RenamedGitChange renamed)
8185
{
82-
collector.EmitError(redirectFileInfo.Name,
86+
collector.EmitError(redirectFile.Source,
8387
$"File '{renamed.OldFilePath}' was renamed to '{renamed.NewFilePath}' but it has no redirect configuration set.");
8488
}
8589
else if (notFound.ChangeType is GitChangeType.Deleted)
8690
{
87-
collector.EmitError(redirectFileInfo.Name,
91+
collector.EmitError(redirectFile.Source,
8892
$"File '{notFound.FilePath}' was deleted but it has no redirect targets. This will lead to broken links.");
8993
}
9094
}

src/tooling/docs-builder/Tracking/IRepositoryTracker.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,5 @@ public record RenamedGitChange(string OldFilePath, string NewFilePath, GitChange
1919

2020
public interface IRepositoryTracker
2121
{
22-
IEnumerable<GitChange> GetChangedFiles();
22+
IReadOnlyCollection<GitChange> GetChangedFiles();
2323
}

src/tooling/docs-builder/Tracking/IntegrationGitRepositoryTracker.cs

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,38 +7,44 @@ namespace Documentation.Builder.Tracking;
77
public class IntegrationGitRepositoryTracker(string lookupPath) : IRepositoryTracker
88
{
99
private string LookupPath { get; } = $"{lookupPath.Trim(['/', '\\'])}/";
10-
public IEnumerable<GitChange> GetChangedFiles()
10+
public IReadOnlyCollection<GitChange> GetChangedFiles()
1111
{
12-
var deletedFiles = Environment.GetEnvironmentVariable("DELETED_FILES") ?? string.Empty;
13-
if (!string.IsNullOrEmpty(deletedFiles))
14-
{
15-
foreach (var file in deletedFiles.Split(' ', StringSplitOptions.RemoveEmptyEntries).Where(f => f.StartsWith(LookupPath)))
16-
yield return new GitChange(file, GitChangeType.Deleted);
17-
}
12+
return GetChanges().ToArray();
1813

19-
var addedFiles = Environment.GetEnvironmentVariable("ADDED_FILES");
20-
if (!string.IsNullOrEmpty(addedFiles))
14+
IEnumerable<GitChange> GetChanges()
2115
{
22-
foreach (var file in addedFiles.Split(' ', StringSplitOptions.RemoveEmptyEntries).Where(f => f.StartsWith(LookupPath)))
23-
yield return new GitChange(file, GitChangeType.Added);
24-
}
16+
var deletedFiles = Environment.GetEnvironmentVariable("DELETED_FILES") ?? string.Empty;
17+
if (!string.IsNullOrEmpty(deletedFiles))
18+
{
19+
foreach (var file in deletedFiles.Split(' ', StringSplitOptions.RemoveEmptyEntries).Where(f => f.StartsWith(LookupPath)))
20+
yield return new GitChange(file, GitChangeType.Deleted);
21+
}
2522

26-
var modifiedFiles = Environment.GetEnvironmentVariable("MODIFIED_FILES");
27-
if (!string.IsNullOrEmpty(modifiedFiles))
28-
{
29-
foreach (var file in modifiedFiles.Split(' ', StringSplitOptions.RemoveEmptyEntries).Where(f => f.StartsWith(LookupPath)))
30-
yield return new GitChange(file, GitChangeType.Modified);
31-
}
23+
var addedFiles = Environment.GetEnvironmentVariable("ADDED_FILES");
24+
if (!string.IsNullOrEmpty(addedFiles))
25+
{
26+
foreach (var file in addedFiles.Split(' ', StringSplitOptions.RemoveEmptyEntries).Where(f => f.StartsWith(LookupPath)))
27+
yield return new GitChange(file, GitChangeType.Added);
28+
}
3229

33-
var renamedFiles = Environment.GetEnvironmentVariable("RENAMED_FILES");
34-
if (!string.IsNullOrEmpty(renamedFiles))
35-
{
36-
foreach (var pair in renamedFiles.Split(' ', StringSplitOptions.RemoveEmptyEntries).Where(f => f.StartsWith(LookupPath)))
30+
var modifiedFiles = Environment.GetEnvironmentVariable("MODIFIED_FILES");
31+
if (!string.IsNullOrEmpty(modifiedFiles))
3732
{
38-
var parts = pair.Split(':');
39-
if (parts.Length == 2)
40-
yield return new RenamedGitChange(parts[0], parts[1], GitChangeType.Renamed);
33+
foreach (var file in modifiedFiles.Split(' ', StringSplitOptions.RemoveEmptyEntries).Where(f => f.StartsWith(LookupPath)))
34+
yield return new GitChange(file, GitChangeType.Modified);
4135
}
36+
37+
var renamedFiles = Environment.GetEnvironmentVariable("RENAMED_FILES");
38+
if (!string.IsNullOrEmpty(renamedFiles))
39+
{
40+
foreach (var pair in renamedFiles.Split(' ', StringSplitOptions.RemoveEmptyEntries).Where(f => f.StartsWith(LookupPath)))
41+
{
42+
var parts = pair.Split(':');
43+
if (parts.Length == 2)
44+
yield return new RenamedGitChange(parts[0], parts[1], GitChangeType.Renamed);
45+
}
46+
}
47+
4248
}
4349
}
4450
}

src/tooling/docs-builder/Tracking/LocalGitRepositoryTracker.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,16 @@ namespace Documentation.Builder.Tracking;
1010

1111
public class LocalGitRepositoryTracker(DiagnosticsCollector collector, IDirectoryInfo workingDirectory, string lookupPath) : ExternalCommandExecutor(collector, workingDirectory), IRepositoryTracker
1212
{
13-
private string LookupPath { get; } = lookupPath;
13+
private string LookupPath { get; } = lookupPath.Trim('\\', '/');
1414

15-
public IEnumerable<GitChange> GetChangedFiles()
15+
public IReadOnlyCollection<GitChange> GetChangedFiles()
1616
{
1717
var defaultBranch = GetDefaultBranch();
1818
var commitChanges = CaptureMultiple("git", "diff", "--name-status", $"{defaultBranch}...HEAD", "--", $"./{LookupPath}");
1919
var localChanges = CaptureMultiple("git", "status", "--porcelain");
20-
var captured = Capture("git", "stash", "push", "--", $"./{LookupPath}");
20+
var gitStashDocsFolder = Capture("git", "stash", "push", "--", $"./{LookupPath}");
2121
var localUnstagedChanges = Array.Empty<string>();
22-
if (captured != "No local changes to save")
22+
if (gitStashDocsFolder != "No local changes to save")
2323
{
2424
localUnstagedChanges = CaptureMultiple("git", "stash", "show", "--name-status", "-u");
2525
ExecInSilent([], "git", "stash", "pop");

tests/authoring/Framework/CrossLinkResolverAssertions.fs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ module CrossLinkResolverAssertions =
3535
member _.ConfigurationPath = mockFileSystem.FileInfo.New("mock_docset.yml")
3636
member _.OutputDirectory = mockFileSystem.DirectoryInfo.New(".artifacts")
3737
}
38-
let redirectFileParser = RedirectFile(mockRedirectsFile, docContext)
38+
let redirectFileParser = RedirectFile(docContext, mockRedirectsFile)
3939
redirectFileParser.Redirects
4040

4141
let private createFetchedCrossLinks (redirectsYamlSnippet: string) (linksData: IDictionary<string, LinkMetadata>) repoName =

0 commit comments

Comments
 (0)