Skip to content
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.

Sign up for GitHub

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

RevEng: Canonicalizing output paths #3959

Closed
wants to merge 5 commits into from
Closed

RevEng: Canonicalizing output paths #3959

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a1f6e50
Canonicalizing output paths to avoid VS misinterpreting . and ..
lajones Dec 2, 2015
7e7a205
Updated to tidy up.
lajones Dec 3, 2015
c698808
Fixng test on Linux.
lajones Dec 3, 2015
515cc95
Adding Check.NotEmpty().
lajones Dec 3, 2015
ac135ae
Updating NamespaceAndOutputPaths to be internal. Plus moving tests to…
lajones Dec 8, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 66 additions & 28 deletions src/EntityFramework.Commands/Scaffolding/Internal/ReverseEngineeringGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,11 @@ public virtual Task<ReverseEngineerFiles> GenerateAsync(
configuration.CheckValidity();

var metadataModel = GetMetadataModel(configuration);

var outputPaths = ConstructCanonicalizedPaths(configuration.ProjectPath, configuration.OutputPath);

var @namespace = ConstructNamespace(configuration.ProjectRootNamespace,
configuration.ProjectPath, configuration.OutputPath);
outputPaths.CanonicalizedRelativeOutputPath);

var customConfiguration = _configurationFactory
.CreateCustomConfiguration(
Expand All @@ -67,16 +70,12 @@ public virtual Task<ReverseEngineerFiles> GenerateAsync(
? modelConfiguration.ClassName()
: customConfiguration.ContextClassName;

var outputPath = string.IsNullOrEmpty(configuration.OutputPath)
? configuration.ProjectPath
: (Path.IsPathRooted(configuration.OutputPath)
? configuration.OutputPath
: Path.Combine(configuration.ProjectPath, configuration.OutputPath));

CheckOutputFiles(outputPath, dbContextClassName, metadataModel, configuration.OverwriteFiles);
CheckOutputFiles(outputPaths.CanonicalizedFullOutputPath,
dbContextClassName, metadataModel, configuration.OverwriteFiles);

return CodeWriter.WriteCodeAsync(
modelConfiguration, outputPath, dbContextClassName, cancellationToken);
modelConfiguration, outputPaths.CanonicalizedFullOutputPath,
dbContextClassName, cancellationToken);
}

public virtual IModel GetMetadataModel([NotNull] ReverseEngineeringConfiguration configuration)
Expand Down Expand Up @@ -134,18 +133,41 @@ public virtual void CheckOutputFiles(
private static char[] directorySeparatorChars = new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar };

public static string ConstructNamespace(
[NotNull] string rootNamespace, [NotNull] string projectPath, [CanBeNull] string outputPath)
[NotNull] string rootNamespace, [CanBeNull] string canonicalizedRelativeOutputPath)
{
Check.NotEmpty(rootNamespace, nameof(rootNamespace));
Check.NotEmpty(projectPath, nameof(projectPath));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this check intentionally removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope - well spotted. Will put back in.

if (string.IsNullOrEmpty(outputPath)
|| Path.IsPathRooted(outputPath))
if (string.IsNullOrEmpty(canonicalizedRelativeOutputPath))
{
// outputPath is empty or is not relative - so just use root namespace
// canonicalized output path is outside of or is the same
// as the project dir - so just use root namespace
return rootNamespace;
}

var @namespace = rootNamespace;
foreach (var pathPart in canonicalizedRelativeOutputPath
.Split(directorySeparatorChars, StringSplitOptions.RemoveEmptyEntries))
{
@namespace += "." + CSharpUtilities.Instance.GenerateCSharpIdentifier(pathPart, null);
}

return @namespace;
}

/// <summary>
/// Construct canonicalized paths from the project path and output path.
/// </summary>
/// <param name="projectPath"> path to the project, must not be empty, can be absolute or relative </param>
/// <param name="outputPath"> path to output directory, can be null or empty, can be absolute or relative (to the project path) </param>
/// <returns>
/// a <see cref="CanonicalizedOutputPaths"> object containing the canonicalized full output path
/// and the canonicalized relative output path
/// </returns>
public static CanonicalizedOutputPaths ConstructCanonicalizedPaths(
[NotNull] string projectPath, [CanBeNull] string outputPath)
{
Check.NotEmpty(projectPath, nameof(projectPath));

// strip off any directory separator chars at end of project path
for (var projectPathLastChar = projectPath.Last();
directorySeparatorChars.Contains(projectPathLastChar);
Expand All @@ -154,24 +176,40 @@ public static string ConstructNamespace(
projectPath = projectPath.Substring(0, projectPath.Length - 1);
}

var canonicalizedProjectPath = Path.GetFullPath(projectPath);
var canonicalizedOutputPath = Path.GetFullPath(Path.Combine(projectPath, outputPath));
if (!canonicalizedOutputPath.StartsWith(canonicalizedProjectPath))
{
// canonicalizedOutputPath is outside of project - so just use root namespace
return rootNamespace;
}
var canonicalizedFullProjectPath = Path.GetFullPath(projectPath);
var canonicalizedFullOutputPath =
string.IsNullOrEmpty(outputPath)
? canonicalizedFullProjectPath
: Path.IsPathRooted(outputPath)
? Path.GetFullPath(outputPath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that Path.Combine() doesn't do this check already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked and you're right. Will fix.

: Path.GetFullPath(Path.Combine(projectPath, outputPath));

var canonicalizedRelativeOutputPath =
canonicalizedFullOutputPath == canonicalizedFullProjectPath
? string.Empty
: canonicalizedFullOutputPath.StartsWith(canonicalizedFullProjectPath)
? canonicalizedFullOutputPath
.Substring(canonicalizedFullProjectPath.Count() + 1)
: null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not completely sure if this is the same, but this looks almost the same as Uri.MakeRelativeUri

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried Uri stuff a while back. Don't remember exactly what the problem was now but would rather keep full control over what we are doing.


return new CanonicalizedOutputPaths(
canonicalizedFullOutputPath, canonicalizedRelativeOutputPath);
}

var @namespace = rootNamespace;
var canonicalizedRelativePath = canonicalizedOutputPath
.Substring(canonicalizedProjectPath.Count() + 1);
foreach (var pathPart in canonicalizedRelativePath
.Split(directorySeparatorChars, StringSplitOptions.RemoveEmptyEntries))
public class CanonicalizedOutputPaths
{
public CanonicalizedOutputPaths(
[NotNull] string canonicalizedFullOutputPath,
[CanBeNull] string canonicalizedRelativeOutputPath)
{
@namespace += "." + CSharpUtilities.Instance.GenerateCSharpIdentifier(pathPart, null);
Check.NotEmpty(canonicalizedFullOutputPath, nameof(canonicalizedFullOutputPath));

CanonicalizedFullOutputPath = canonicalizedFullOutputPath;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why repeat the words of the name of the class in each property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will change them.

CanonicalizedRelativeOutputPath = canonicalizedRelativeOutputPath;
}

return @namespace;
public virtual string CanonicalizedFullOutputPath { get; }
public virtual string CanonicalizedRelativeOutputPath { get; }
}
}
}
42 changes: 21 additions & 21 deletions src/EntityFramework.Commands/Scaffolding/Internal/StringBuilderCodeWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,54 +10,54 @@

namespace Microsoft.Data.Entity.Scaffolding.Internal
{
public class StringBuilderCodeWriter : CodeWriter
public class StringBuilderCodeWriter : CodeWriter
{
public virtual DbContextWriter DbContextWriter { get; }
public virtual EntityTypeWriter EntityTypeWriter { get; }
public virtual DbContextWriter DbContextWriter { get; }
public virtual EntityTypeWriter EntityTypeWriter { get; }

public StringBuilderCodeWriter(
[NotNull] IFileService fileService,
[NotNull] DbContextWriter dbContextWriter,
[NotNull] EntityTypeWriter entityTypeWriter)
: base(fileService)
{
Check.NotNull(dbContextWriter, nameof(dbContextWriter));
Check.NotNull(entityTypeWriter, nameof(entityTypeWriter));
Check.NotNull(dbContextWriter, nameof(dbContextWriter));
Check.NotNull(entityTypeWriter, nameof(entityTypeWriter));

DbContextWriter = dbContextWriter;
EntityTypeWriter = entityTypeWriter;
}

public override Task<ReverseEngineerFiles> WriteCodeAsync(
public override Task<ReverseEngineerFiles> WriteCodeAsync(
[NotNull] ModelConfiguration modelConfiguration,
[NotNull] string outputPath,
[NotNull] string dbContextClassName,
CancellationToken cancellationToken = default(CancellationToken))
{
Check.NotNull(modelConfiguration, nameof(modelConfiguration));
Check.NotEmpty(outputPath, nameof(outputPath));
Check.NotEmpty(dbContextClassName, nameof(dbContextClassName));
Check.NotNull(modelConfiguration, nameof(modelConfiguration));
Check.NotEmpty(outputPath, nameof(outputPath));
Check.NotEmpty(dbContextClassName, nameof(dbContextClassName));

cancellationToken.ThrowIfCancellationRequested();
cancellationToken.ThrowIfCancellationRequested();

var resultingFiles = new ReverseEngineerFiles();
var resultingFiles = new ReverseEngineerFiles();

var generatedCode = DbContextWriter.WriteCode(modelConfiguration);
var generatedCode = DbContextWriter.WriteCode(modelConfiguration);

// output DbContext .cs file
var dbContextFileName = dbContextClassName + FileExtension;
// output DbContext .cs file
var dbContextFileName = dbContextClassName + FileExtension;
var dbContextFileFullPath = FileService.OutputFile(
outputPath, dbContextFileName, generatedCode);
resultingFiles.ContextFile = dbContextFileFullPath;
outputPath, dbContextFileName, generatedCode);
resultingFiles.ContextFile = dbContextFileFullPath;

foreach (var entityConfig in modelConfiguration.EntityConfigurations)
foreach (var entityConfig in modelConfiguration.EntityConfigurations)
{
generatedCode = EntityTypeWriter.WriteCode(entityConfig);
generatedCode = EntityTypeWriter.WriteCode(entityConfig);

// output EntityType poco .cs file
var entityTypeFileName = entityConfig.EntityType.DisplayName() + FileExtension;
// output EntityType poco .cs file
var entityTypeFileName = entityConfig.EntityType.DisplayName() + FileExtension;
var entityTypeFileFullPath = FileService.OutputFile(
outputPath, entityTypeFileName, generatedCode);
outputPath, entityTypeFileName, generatedCode);
resultingFiles.EntityTypeFiles.Add(entityTypeFileFullPath);
}

Expand Down
4 changes: 2 additions & 2 deletions ...amework.MicrosoftSqlServer.Design.FunctionalTests/ReverseEngineering/SqlServerE2ETests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public void E2ETest_UseAttributesInsteadOfFluentApi()

var filePaths = Generator.GenerateAsync(configuration).GetAwaiter().GetResult();

var actualFileSet = new FileSet(InMemoryFiles, Path.Combine(TestProjectDir, TestSubDir))
var actualFileSet = new FileSet(InMemoryFiles, Path.GetFullPath(Path.Combine(TestProjectDir, TestSubDir)))
{
Files = Enumerable.Repeat(filePaths.ContextFile, 1).Concat(filePaths.EntityTypeFiles).Select(Path.GetFileName).ToList()
};
Expand Down Expand Up @@ -169,7 +169,7 @@ public void E2ETest_AllFluentApi()

var filePaths = Generator.GenerateAsync(configuration).GetAwaiter().GetResult();

var actualFileSet = new FileSet(InMemoryFiles, TestProjectDir)
var actualFileSet = new FileSet(InMemoryFiles, Path.GetFullPath(TestProjectDir))
{
Files = Enumerable.Repeat(filePaths.ContextFile, 1).Concat(filePaths.EntityTypeFiles).Select(Path.GetFileName).ToList()
};
Expand Down
47 changes: 46 additions & 1 deletion ...yFramework.Relational.Design.Tests/ReverseEngineering/ReverseEngineeringGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.IO;
using Microsoft.Data.Entity.Scaffolding.Internal;
using Xunit;
Expand All @@ -14,8 +15,20 @@ public class ReverseEngineeringGeneratorTests
public void Constructs_correct_namespace(
string rootNamespace, string projectPath, string outputPath, string resultingNamespace)
{
var outputPaths = ReverseEngineeringGenerator.ConstructCanonicalizedPaths(projectPath, outputPath);
Assert.Equal(resultingNamespace,
ReverseEngineeringGenerator.ConstructNamespace(rootNamespace, projectPath, outputPath));
ReverseEngineeringGenerator.ConstructNamespace(rootNamespace, outputPaths.CanonicalizedRelativeOutputPath));
}

[Theory]
[MemberData(nameof(PathOptions))]
public void Constructs_correct_canonical_paths(
string projectPath, string outputPath,
string canonicalizedFullOutputPath, string canonicalizedRelativeOutputPath)
{
var outputPaths = ReverseEngineeringGenerator.ConstructCanonicalizedPaths(projectPath, outputPath);
Assert.Equal(canonicalizedFullOutputPath, outputPaths.CanonicalizedFullOutputPath);
Assert.Equal(canonicalizedRelativeOutputPath, outputPaths.CanonicalizedRelativeOutputPath);
}

public static TheoryData NamespaceOptions
Expand Down Expand Up @@ -45,5 +58,37 @@ public static TheoryData NamespaceOptions
return data;
}
}

public static TheoryData PathOptions
{
get
{
var data = new TheoryData<string, string, string, string>();

if (Path.DirectorySeparatorChar == '\\'
|| Path.AltDirectorySeparatorChar == '\\')
{
data.Add(@"X:\project\Path", null, @"X:\project\Path", string.Empty);
data.Add(@"X:\project\Path", string.Empty, @"X:\project\Path", string.Empty);
data.Add(@"X:\project\Path", @"X:\Absolute\Output\Path", @"X:\Absolute\Output\Path", null);
data.Add(@"X:\project\Path", @"..\..\Path\Outside\Project", @"X:\Path\Outside\Project", null);
data.Add(@"X:\project\Path", @"Path\Inside\Project", @"X:\project\Path\Path\Inside\Project", @"Path\Inside\Project");
data.Add(@"X:\project\Path", @"Path\.\Inside\Project", @"X:\project\Path\Path\Inside\Project", @"Path\Inside\Project");
data.Add(@"X:\project\Path", @"FirstDir\IgnoreThisDir\..\AnotherDir", @"X:\project\Path\FirstDir\AnotherDir", @"FirstDir\AnotherDir");
}
else
{
data.Add("/project/Path", null, "/project/Path", string.Empty);
data.Add("/project/Path", string.Empty, "project/Path", string.Empty);
data.Add("/project/Path", "/Absolute/Output/Path", "/Absolute/Output/Path", null);
data.Add("/project/Path", "../../Path/Outside/Project", "/Path/Outside/Project", null);
data.Add("/project/Path", "Path/Inside/Project", "/project/Path/Path/Inside/Project", "Path/Inside/Project");
data.Add("/project/Path", "Path/./Inside/Project", "/project/Path/Path/Inside/Project", "Path/Inside/Project");
data.Add("/project/Path", "FirstDir/IgnoreThisDir/../AnotherDir", "/project/Path/FirstDir/AnotherDir", "FirstDir/AnotherDir");
}

return data;
}
}
}
}
Loading