Fixing PhysicalFilesWatcher path maintenance (Issue #71386)

[turnyanszki]

#if NET5_0_OR_GREATER is needed beacuse the project is targeting .net framwork for some reason (analyzers maybe).
All project should be refactored at this lib some point in my opinion.
fixes #71386

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[radical]

@turnyanszki In the PR description, could you please describe the issue that you are fixing here? Also, add a short title for the PR. And reference any link for the issue, if there is one.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

---

#if NET5_0_OR_GREATER is needed beacuse the project is targeting .net framwork for some reason (analyzers maybe).
All project should be refactored at this lib some point in my opinion.

Author:	turnyanszki
Assignees:	-
Labels:	area-System.Configuration, community-contribution
Milestone:	-

[turnyanszki]

@turnyanszki In the PR description, could you please describe the issue that you are fixing here? Also, add a short title for the PR. And reference any link for the issue, if there is one.

There is a link in the commit to the issue.

[ghost]

Tagging subscribers to this area: @dotnet/area-extensions-filesystem
See info in area-owners.md if you want to be subscribed.

Issue Details

---

#if NET5_0_OR_GREATER is needed beacuse the project is targeting .net framwork for some reason (analyzers maybe).
All project should be refactored at this lib some point in my opinion.
fixes #71386

Author:	turnyanszki
Assignees:	-
Labels:	area-Extensions-FileSystem, community-contribution
Milestone:	-

[danmoseley]

@jozkee could you please give an idea when you will be able to review? This change seems small.

[eerhardt]

@turnyanszki - looks like the new test you added to verify the bug fix is failing on .NET Framework. This is because you are only adding code for #if NET5_0_OR_GREATER. You need to also fix the bug for .NET Framework.

[turnyanszki]

@turnyanszki - looks like the new test you added to verify the bug fix is failing on .NET Framework. This is because you are only adding code for #if NET5_0_OR_GREATER. You need to also fix the bug for .NET Framework.

I dont want to implement the whole Path.GetFullPath method myself, and it does not exists in .NET Framework.
Any suggestions?

[eerhardt]

I dont want to implement the whole Path.GetFullPath method myself, and it does not exists in .NET Framework.
Any suggestions?

Looking at Path.GetFullPath, the trickiest part is the if (IsPathFullyQualified(path)) method:

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs

Lines 36 to 51 in 878354a

	public static string GetFullPath(string path, string basePath)
	{
	ArgumentNullException.ThrowIfNull(path);
	ArgumentNullException.ThrowIfNull(basePath);
	if (!IsPathFullyQualified(basePath))
	throw new ArgumentException(SR.Arg_BasePathNotFullyQualified, nameof(basePath));
	if (basePath.Contains('\0') || path.Contains('\0'))
	throw new ArgumentException(SR.Argument_NullCharInPath);
	if (IsPathFullyQualified(path))
	return GetFullPathInternal(path);
	return GetFullPathInternal(CombineInternal(basePath, path));
	}

This method is available on netstandard2.0 or .NET Framework. However, looking at the callers of this method, it call comes from CreateFileChildToken here:

runtime/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFilesWatcher.cs

Lines 134 to 146 in 878354a

	public IChangeToken CreateFileChangeToken(string filter)
	{
	ThrowHelper.ThrowIfNull(filter);
	filter = NormalizePath(filter);
	// Absolute paths and paths traversing above root not permitted.
	if (Path.IsPathRooted(filter) || PathUtils.PathNavigatesAboveRoot(filter))
	{
	return NullChangeToken.Singleton;
	}
	IChangeToken changeToken = GetOrAddChangeToken(filter);

Which verifies that the filter string isn't a rooted path. So I believe you can just Combine the two paths, and call GetFullPath on the result. That should be the same on .NET Framework.

[jozkee]

@eerhardt suggestion, this should work for .NET Framework too:

Suggested change

	#if NETCOREAPP
	var absolutePath = Path.GetFullPath(filePath, _root);
	filePath = absolutePath.Substring(_root.Length);
	#endif
	#if NETCOREAPP
	string absolutePath = Path.GetFullPath(filePath, _root);
	#else
	string absolutePath = Path.GetFullPath(Path.Combine(_root, filePath));
	#endif
	filePath = absolutePath.Substring(_root.Length);

[jozkee]

This test should be a Theory and you should put ./reload.json and reload.json as InlineData.

[ghost]

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.