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

Change exception type/message when project item does not exist #391

Merged
merged 2 commits into from
Nov 17, 2020

Conversation

maxbanas
Copy link
Contributor

This is related to the discussion in #378
RazorSourceGenerator.GenerateCodeAsync(RazorLightProjectItem projectItem) and RazorSourceGenerator.CreateCodeDocumentAsync(RazorLightProjectItem projectItem) both accept a RazorLightProjectItem with the precondition that the .Exists property is true. Previously, when this precondition was not met, GenerateCodeAsync threw a TemplateNotFoundException while CreateCodeDoucmentAsync threw a InvalidOperationException. The exception language and type (in GenerateCodeAsync ) made it sound like these methods are trying to "look up" a template, when in reality they are only checking the Exists property on the project item. These changes clarify the behavior of these methods and make the exception type consistent.

@maxbanas
Copy link
Contributor Author

Tests passed for netcoreapp2.0 and netcoreapp3.1, but I am not able to run the tests for net5.0 yet.

@@ -108,7 +108,7 @@ public virtual async Task<RazorCodeDocument> CreateCodeDocumentAsync(RazorLightP

if (!projectItem.Exists)
{
throw new InvalidOperationException($"Project can not find template with key {projectItem.Key}");
throw new InvalidOperationException($"RazorLightProjectItem with key {projectItem.Key} must exist");
Copy link
Collaborator

@jzabroski jzabroski Nov 17, 2020

Choose a reason for hiding this comment

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

Use $"nameof(RazorLightProjectItem) of type {projectItem.GetType().FullName} with key {projectItem.Key} does not exist."

My rationale being that it would help when people ask for help to know exactly what type of RazorLightProjectItem they passed in.

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've made the requested changes. I also added two additional tests since the new exception messages require that projectItem is not null.

@@ -71,7 +71,7 @@ public async Task<IGeneratedRazorTemplate> GenerateCodeAsync(RazorLightProjectIt

if (!projectItem.Exists)
{
throw new TemplateNotFoundException($"Project can not find template with key {projectItem.Key}");
throw new InvalidOperationException($"RazorLightProjectItem with key {projectItem.Key} must exist");
Copy link
Collaborator

@jzabroski jzabroski Nov 17, 2020

Choose a reason for hiding this comment

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

Use $"nameof(RazorLightProjectItem) of type {projectItem.GetType().FullName} with key {projectItem.Key} does not exist."

My rationale being that it would help when people ask for help to know exactly what type of RazorLightProjectItem they passed in.

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've made the requested changes. I also added two additional tests since the new exception messages require that projectItem is not null.

@jzabroski jzabroski merged commit 8d41e69 into toddams:master Nov 17, 2020
@jzabroski jzabroski added this to the 2.0.0-rc.2 milestone Nov 17, 2020
@jzabroski
Copy link
Collaborator

Thanks. I will try to push this out tonight.

@maxbanas maxbanas deleted the fix/378-1 branch November 19, 2020 17:39
vdurante pushed a commit to vdurante/RazorLight that referenced this pull request Feb 29, 2024
Change exception type/message when project item does not exist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants