episerver / upgrade-assistant-extensions

Apache License 2.0
10 stars 6 forks source link

EP0002 - ArgumentNullException #34

Closed rw-kaber closed 2 years ago

rw-kaber commented 2 years ago

Hi, when running upgrade-assistant with EPi.Source.Updater.1.0.27 I get a lot of errors like below, seemingly in a loop, because the upgrade-assistant is already running for a couple of hours.

[20:07:46 INF] Applying upgrade step Apply fix for EP0002: CMS Classes like PartialContentController or BlockController will be replaced
[20:07:46 ERR] Unexpected error applying step
System.ArgumentNullException: Value cannot be null. (Parameter 'document')
   at Microsoft.DotNet.UpgradeAssistant.Steps.Source.CodeFixerStep.TryFixDiagnosticAsync(Diagnostic diagnostic, Document document, CancellationToken token)
   at Microsoft.DotNet.UpgradeAssistant.Steps.Source.CodeFixerStep.ApplyImplAsync(IUpgradeContext context, CancellationToken token)
   at Microsoft.DotNet.UpgradeAssistant.UpgradeStep.ApplyAsync(IUpgradeContext context, CancellationToken token) in /_/src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/UpgradeStep.cs:line 203
Command (Apply next step (Apply fix for EP0002: CMS Classes like PartialContentController or BlockController will be replaced)) did not succeed

Could this be connected to blocks without controllers?

EDIT: After stopping and re-running the upgrade-assistant a new error emerged:

[21:36:33 ERR] Unexpected error applying step
System.InvalidCastException: Unable to cast object of type 'Microsoft.CodeAnalysis.CSharp.Syntax.QualifiedNameSyntax' to type 'Microsoft.CodeAnalysis.CSharp.Syntax.SimpleNameSyntax'.
   at Epi.Source.Updater.EpiClassReplacementsCodeFixProvider.ReplaceClassesAsync(Document document, BaseTypeSyntax localDeclaration, String newIdentifier, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CodeActions.CodeAction.GetChangedSolutionAsync(CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CodeActions.CodeAction.ComputeOperationsAsync(CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CodeActions.CodeAction.ComputeOperationsAsync(IProgressTracker progressTracker, CancellationToken cancellationToken)
   at Microsoft.CodeAnalysis.CodeActions.CodeAction.GetOperationsCoreAsync(IProgressTracker progressTracker, CancellationToken cancellationToken)
   at Microsoft.DotNet.UpgradeAssistant.Steps.Source.CodeFixerStep.TryFixDiagnosticAsync(Diagnostic diagnostic, Document document, CancellationToken token)
   at Microsoft.DotNet.UpgradeAssistant.Steps.Source.CodeFixerStep.ApplyImplAsync(IUpgradeContext context, CancellationToken token)
   at Microsoft.DotNet.UpgradeAssistant.UpgradeStep.ApplyAsync(IUpgradeContext context, CancellationToken token) in /_/src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/UpgradeStep.cs:line 203
Command (Apply next step (Apply fix for EP0002: CMS Classes like PartialContentController or BlockController will be replaced)) did not succeed
[21:36:33 INF] Applying upgrade step Apply fix for EP0002: CMS Classes like PartialContentController or BlockController will be replaced
lunchin commented 2 years ago

Thanks for reporting. We will look into.

jbearfoot commented 2 years ago

It looks like it is the codeanalyzer/codefixer that rewrites partial controllers (e.g. block controllers) to view components that fails. Do you mind share what your partial controllers look like? It does not need to be all code just the class declarations (including base types?, so something like: public class MyPartialController : BlockController

rw-kaber commented 2 years ago

Our inheritance tree SomeBlock -> ContentBlock -> BaseBlock -> EPiServer.Core.BlockData or SomeOtherBlock -> BaseBlock -> EPiServer.Core.BlockData

PartialContentController - we don't utilize any

We currently have all Block Controllers setup like:

public class SomeModelController : BlockController<SomeModel>
{
    private readonly UrlHelper urlHelper;
    private readonly IContentRepository contentRepository;

    public SomeModelController(UrlHelper urlHelper, IContentRepository contentRepository)
    {
        this.urlHelper = urlHelper;
        this.contentRepository = contentRepository;
    }

    public override ActionResult Index(SomeModel currentBlock)
    {
        return PartialView("/Views/Blocks/SomeModel.cshtml", viewModel);
    }
}

We have one controller that looks something like this

public class SomeController : Controller
{
    private readonly ICrawlerDetectorHelper crawlerDetectorHelper;

    public SomeController(ICrawlerDetectorHelper crawlerDetectorHelper)
    {
        this.crawlerDetectorHelper = crawlerDetectorHelper;
    }

    public ActionResult Render(SomeModel model)
    {
        return PartialView("~/Views/Partials/SomeModelPartial.cshtml", model);
    }
}
jbearfoot commented 2 years ago

Ok, that looks normal and is something that the migration tool should handle.

From the call stack it looks like it is a quite simple class replacement that fails. The step will basically look in file https://github.com/episerver/upgrade-assistant-extensions/blob/main/src/EpiSourceUpdater/EpiClassReplacements.classmap for which types that should be mapped to what type. So e.g. your

public class SomeModelController : BlockController<SomeModel> should be replaced with public class SomeModelController : BlockComponent<SomeModel> When looking here https://github.com/episerver/upgrade-assistant-extensions/blob/main/src/EpiSourceUpdater/EpiClassReplacementsCodeFixProvider.cs#L66 it seems like the code fixer expects the type to map to be a generic type. Could it be that you have a type named BlockController or PartialContentController that is not generic? And use it like public class NoneGenericController : BlockController

Otherwise what you could do to get more information is to clone this repo and build it in debug and then replace the .dll in the folder you unzipped to with the debug compiled .dll and .pdb. Then you could attach a debugger to the upgrade-assistant process and run it with "break at all exceptions" or put a breakpoint at https://github.com/episerver/upgrade-assistant-extensions/blob/main/src/EpiSourceUpdater/EpiClassReplacementsCodeFixProvider.cs#L66

rw-kaber commented 2 years ago

Hi @jbearfoot, I've managed to catch the exception, however it's being thrown for a perfectly reasonable type. System.InvalidCastException: 'Unable to cast object of type 'Microsoft.CodeAnalysis.CSharp.Syntax.QualifiedNameSyntax' to type 'Microsoft.CodeAnalysis.CSharp.Syntax.SimpleNameSyntax'.'

image

The block class looks like follows (I've omitted some details, but the constructor declaration is exact match)

public class SomeBlockController : EPiServer.Web.Mvc.BlockController<SomeBlock>
{
    private readonly IRequiredClientResourceList requiredClientResourceList;

    public SomeBlockController(IRequiredClientResourceList requiredClientResourceList)
    {
        this.requiredClientResourceList = requiredClientResourceList;
    }

    public override ActionResult Index(SomeBlock currentContent)
    {       

        if (someCondition)
        {
            return PartialView("/Views/Blocks/Content/SomeBlockNew.cshtml", currentContent);
        }
        else
        {
            return PartialView("/Views/Blocks/Content/SomeBlockOld.cshtml", currentContent);
        }
    }
}
rw-kaber commented 2 years ago

Hey, also adding to my comment above - all our block use the generic type BlockController<Model>.

jbearfoot commented 2 years ago

Ah, I would guess that it is because you are using a qualified name like EPiServer.Web.Mvc.BlockController instead of just BlockController. If so then it is something we should fix, but meanwhile you can probably just remove EPiServer.Web.Mvc. from your class declaration.

rw-kaber commented 2 years ago

@jbearfoot It worked, thank you :)