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.

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

Add type checking when importing types from recipes #15979 #15980

Merged
merged 24 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
19889be
add typepart check #15979
hyzx86 May 5, 2024
34d55c0
Update ContentDefinitionService.cs
hyzx86 May 5, 2024
bd2ff8b
Merge branch 'main' into recipetypecheck
hyzx86 May 5, 2024
0f14c07
Update ContentDefinitionStep.cs
hyzx86 May 5, 2024
5018c0f
Update src/OrchardCore.Modules/OrchardCore.ContentTypes/RecipeSteps/C…
hyzx86 May 6, 2024
34063ce
Update test/OrchardCore.Tests/Recipes/RecipeFiles/recipe6.json
hyzx86 May 6, 2024
9f8809c
Cleaning up recipes and removing unwanted elements
May 6, 2024
a3b7a3c
use Assert.ThrowsAsync
May 6, 2024
0c2c3f3
Merge branch 'main' into recipetypecheck
hyzx86 May 6, 2024
3bc6cf2
Update src/OrchardCore.Modules/OrchardCore.ContentTypes/RecipeSteps/C…
hyzx86 May 6, 2024
d7d3851
Update test/OrchardCore.Tests/Recipes/RecipeExecutorTests.cs
hyzx86 May 6, 2024
fcf0ee7
Update test/OrchardCore.Tests/Recipes/RecipeFiles/recipe6.json
hyzx86 May 6, 2024
6483f83
Update test/OrchardCore.Tests/Recipes/RecipeFiles/recipe6.json
hyzx86 May 6, 2024
23aafad
Update test/OrchardCore.Tests/Recipes/RecipeExecutorTests.cs
hyzx86 May 6, 2024
f252446
Fix test
MikeAlhayek May 6, 2024
683b05a
Merge branch 'main' into recipetypecheck
hyzx86 May 6, 2024
f7cbf66
add notify
May 7, 2024
68d9b66
Merge branch 'main' into recipetypecheck
hyzx86 May 7, 2024
05387fd
update notify
hyzx86 May 7, 2024
e2e7a9e
ad log
hyzx86 May 7, 2024
174ec9c
Merge branch 'main' into recipetypecheck
hyzx86 May 8, 2024
bdd313d
restore Changes
hyzx86 May 8, 2024
75b42b6
Merge branch 'recipetypecheck' of https://github.com/hyzx86/OrchardCo…
hyzx86 May 8, 2024
13c32f1
remove localize
hyzx86 May 8, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ private Task UpdateContentTypeAsync(ContentTypeDefinition type, ContentTypeDefin

foreach (var part in record.ContentTypePartDefinitionRecords)
{
if (string.IsNullOrEmpty(part.PartName))
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
{
throw new Exception(string.Format("Failed to add part to ContentType '{0}', The PartName property is not allowed to be empty", type.Name));
hyzx86 marked this conversation as resolved.
Show resolved Hide resolved
}

builder.WithPart(part.Name, part.PartName, partBuilder => partBuilder.MergeSettings(part.Settings));
}
});
Expand Down
2 changes: 2 additions & 0 deletions test/OrchardCore.Tests/OrchardCore.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<None Remove="Recipes\RecipeFiles\recipe3.json" />
<None Remove="Recipes\RecipeFiles\recipe4.json" />
<None Remove="Recipes\RecipeFiles\recipe5.json" />
<None Remove="Recipes\RecipeFiles\recipe6.json" />
</ItemGroup>

<ItemGroup>
Expand All @@ -44,6 +45,7 @@
<EmbeddedResource Include="Recipes\RecipeFiles\recipe3.json" />
<EmbeddedResource Include="Recipes\RecipeFiles\recipe4.json" />
<EmbeddedResource Include="Recipes\RecipeFiles\recipe5.json" />
<EmbeddedResource Include="Recipes\RecipeFiles\recipe6.json" />
</ItemGroup>

<ItemGroup>
Expand Down
23 changes: 23 additions & 0 deletions test/OrchardCore.Tests/Recipes/RecipeExecutorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using OrchardCore.Recipes.Models;
using OrchardCore.Recipes.Services;
using OrchardCore.Scripting;
using OrchardCore.Tests.Apis.Context;

namespace OrchardCore.Recipes
{
Expand Down Expand Up @@ -50,6 +51,28 @@ public async Task ShouldTrimValidScriptExpression(string recipeName, string expe
});
}

[Fact]
public async Task IncorrectTypeDefinitionImportsShouldBeBlocked()
hyzx86 marked this conversation as resolved.
Show resolved Hide resolved
{
var context = new BlogContext();
await context.InitializeAsync();
await context.UsingTenantScopeAsync(async scope =>
{
var recipeExecutor = scope.ServiceProvider.GetRequiredService<IRecipeExecutor>();
// Act
var executionId = Guid.NewGuid().ToString("n");
var recipeDescriptor = new RecipeDescriptor { RecipeFileInfo = GetRecipeFileInfo("recipe6") };
try
{
await recipeExecutor.ExecuteAsync(executionId, recipeDescriptor, new Dictionary<string, object>(), CancellationToken.None);
}
catch (Exception ex)
{
Assert.Equal("Failed to add part to ContentType 'Message', The PartName property is not allowed to be empty", ex.Message);
}
Copy link
Member

Choose a reason for hiding this comment

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

Use Assert.ThrowsAsync() instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, always learning something new from you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DONE

Copy link
Member

Choose a reason for hiding this comment

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

I'm glad :).

});
}

private static Task<ShellScope> GetScopeAsync() => ShellScope.Context.CreateScopeAsync();

private static ShellContext CreateShellContext() => new()
Expand Down
58 changes: 58 additions & 0 deletions test/OrchardCore.Tests/Recipes/RecipeFiles/recipe6.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
{
"name": "Recipe6",
"displayName": "Recipe 6",
"description": "This is a recipe for a bug to verify that the type-checking function works as expected",
hyzx86 marked this conversation as resolved.
Show resolved Hide resolved
"author": "Tony Han",
"website": "",
"version": "1.0.0",
"issetuprecipe": false,
"categories": [ "test" ],
"tags": [],
"variables": {
"messageContentItemId": "[js:uuid()]",
"now": "[js: new Date().toISOString()]"
},
"steps": [
{
"name": "feature",
"enable": [
"OrchardCore.Contents",
"OrchardCore.ContentTypes",
"OrchardCore.Title"
]
},
{
"name": "ContentDefinition",
"ContentTypes": [
{
"Name": "Message",
"DisplayName": "Message",
"Settings": {
"ContentTypeSettings": {
"Creatable": true,
"Draftable": true,
"Versionable": true,
"Listable": true,
"Securable": true
}
},
"ContentTypePartDefinitionRecords": [
{
"PartName": "Message",
"Name": "Message",
"Settings": {
"ContentTypePartSettings": {
"Position": "5"
}
}
},
Copy link
Member

Choose a reason for hiding this comment

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

Are these really needed?

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'll try it, I'm not sure, but I think this recipe is closer to the real scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

{
//"PartName": "TitlePart",//test part validate
hyzx86 marked this conversation as resolved.
Show resolved Hide resolved
"Name": "TitlePart"
}
]
}
]
}
]
}