CCDM: allow page configuration in client mode

[manolo]

Fixes #6938

• Added a VaadinAppShell class for configurin clientSide page
• Added a ServletContinerInitializer for checking annotations on startup
• Implement Meta annotations in clientSide mode

---

This change is 

[vlukashov]

Reviewable status: 30 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, @vaadin-bot, and @vlukashov)

a discussion (no related file):
Is it possible to include this functionality into one of the existing integration tests?
E.g. the test-ccdm and flow-ccdm-connect test projects seem to be suitable candidates.

---

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 21 at r1 (raw file):

when in clientSide mode

Let's try to use the same terms as in the documentation, what do you think?
I am not sure if there is a docs page describing what clientSide mode is. If there is, then this line is probably fine.

On the other hand, I know that there is a page describing what client-side bootstrapping is (https://vaadin.com/docs/v15/flow/ccdm/client-side-bootstrapping.html).
Would it be better to change the term used here to be more like when using client-side bootstrapping?

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 23 at r1 (raw file):

There should be only one class in the application extending this.

nit: would it be better to word this as There should be at max one class extending VaadinAppShell in the application?

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 25 at r1 (raw file):

 remove all annotations from other class

This is ambiguous: one can read this as literally no other annotations are allowed on any other classes in the app.

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 25 at r1 (raw file):

class

classes

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 39 at r1 (raw file):

@Meta(name = "charset", content = "UTF-8")

When looking at this line a am getting a bunch of questions at once:

• Is this @Meta used by default when a developer adds a class AppShell extends VaadinAppShell?
• Is it used if the developer does not add the app shell class?
• What happens with the <meta name="charset" content="UTF-8"> tag that is present in the auto-generated index.html file? Would it be duplicated?

Can you please clarify these points in the Javadoc comment for this class?

UPD:
I've tried this in the test-ccdm test app, and this @Meta causes a startup failure (if the app defines its own class AppShell extends VaadinAppShell).

---

flow-server/src/main/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandler.java, line 113 at r2 (raw file):

    private void includeAppShellElements(Document document, ServletContext context) {
        VaadinAppShellRegistry.getInstance(context).getElements()
                .forEach(elem -> document.head().appendChild(elem));

This approach works very well for @Meta, @BodySize, @Push.
But would it work for @Inline and @PWA? Those annotations require adding some content into the body of the document as well.
Would it make more sense to change the interface so that instead of
getElements() it would be applyAppShellDocumentModifications(Document document) or similar?

---

flow-server/src/main/java/com/vaadin/flow/server/startup/AbstractAnnotationValidator.java, line 140 at r1 (raw file):

// This is handled in separated validation

Could you please also describe where is that separate validation? Otherwise this comment leaves me wondering.

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 50 at r1 (raw file):

@WebListener
public class VaadinAppShellInitializer implements ServletContainerInitializer,
        Serializable, ServletContextListener {

Why does this class need to implement the ServletContextListener interface? Both methods of that interface are implemented as no-op in this class: contextInitialized() and contextDestroyed().

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 62 at r1 (raw file):

        }

        DeploymentConfiguration config = StubServletConfig

This is an area where I do not have much context, so maybe it's a silly question.

I've noticed that the approach to getting a Servlet deployment config used here is different from the one used in the main Vaadin's ServletDeployer:

1. ServletDeployer expects that there can be several deployment configurations, not only one
2. ServletDeployer does not assume the servlet class to always be VaadinServlet

Are these differences relevant here, in VaadinAppShellInitializer?

https://github.com/vaadin/flow/blob/ccdm/flow-server/src/main/java/com/vaadin/flow/server/startup/ServletDeployer.java

private Collection<DeploymentConfiguration> getServletConfigurations(
        ServletContext context) {
    Collection<? extends ServletRegistration> registrations = context
            .getServletRegistrations().values();
    Collection<DeploymentConfiguration> result = new ArrayList<>(
            registrations.size());
    for (ServletRegistration registration : registrations) {
        loadClass(context.getClassLoader(), registration.getClassName())
                .ifPresent(servletClass -> result.add(
                        StubServletConfig.createDeploymentConfiguration(
                                context, registration, servletClass)));
    }
    return result;
}

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 100 at r1 (raw file):

        // Reset the registry
        registry.setShell(null, context);

nit: would it be more convenient if the method name was reset, so that the comment would be unnecessary?

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 129 at r2 (raw file):

            String message = String.format(VaadinAppShellRegistry.ERROR_HEADER,
                    registry.getShell().getName(),
                    String.join("\n", offendingAnnotations));

This produces error messages like this one:

Found configuration annotations in non `VaadinApplicationShell` classes.
The following annotations must be moved to the 'com.vaadin.flow.ccdmtest.AppShell' class:
  - com.vaadin.flow.component.page.VaadinAppShell contains: Meta
  - com.vaadin.flow.ccdmtest.MainLayout contains: Meta

I really like that they are detailed and specific! Great job!

nit: would it make sense to add a @ symbol before annotation names? (e.g. @Meta)

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 129 at r1 (raw file):
Indeed, the check in the servlet initializer uses the isAssignableFrom method:

} else if (VaadinAppShell.class.isAssignableFrom(clazz)) {
    // This is handled in separated validation
}

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 44 at r2 (raw file):

VaadinApplicationShell

VaadinAppShell

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 48 at r2 (raw file):

    private static final String ERROR_LINE = "  - %s contains: %s";
    private static final String ERROR_MULTIPLE_SHELL = "%nFound multiple classes extending `VaadinApplicationShell` in the application%n  %s%n  %s%n";

This produces error messages like

Found multiple classes extending `VaadinApplicationShell` in the application
  class com.vaadin.flow.ccdmtest.AppShell
  com.vaadin.flow.ccdmtest.AbstractAppShell

I really like that the messages are specific and tell exactly what classes are found.
But, the message does not really tell what the developer should do to fix the issue. Please add a line telling what to do.

nit: also the first class does include the class word whereas the second line does not.

NOTE: in a similar case Spring Boot has the following error message

Unable to find a single main class from the following candidates [com.example.application.spring.Application2, com.example.application.spring.Application]

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 63 at r2 (raw file):

     */
    @SuppressWarnings("unchecked")
    public static VaadinAppShellRegistry getInstance(ServletContext context) {

This is an area where I do not have much context, so maybe it's a silly question.

I've noticed that the use case of creating an instance of VaadinAppShellRegistry is very similar to that of ApplicationRouteRegistry, but has a different implementation:

1. ApplicationRouteRegistry.getInstance() gets a VaadinContext as a parameter (not ServletContext)
2. ApplicationRouteRegistry uses a ApplicationRouteRegistryWrapper class and keeps the instance as a context attribute, whereas keeps the instance as a static member

Are these differences relevant here, in VaadinAppShellRegistry? Can we follow the same logic in both places?

public static ApplicationRouteRegistry getInstance(VaadinContext context) {
    assert context != null;

    ApplicationRouteRegistryWrapper attribute;
    synchronized (context) {
        attribute = context
                .getAttribute(ApplicationRouteRegistryWrapper.class);

        if (attribute == null) {
            attribute = new ApplicationRouteRegistryWrapper(
                    createRegistry(context));
            context.setAttribute(attribute);
        }
    }

    return attribute.getRegistry();
}

https://github.com/vaadin/flow/blob/ccdm/flow-server/src/main/java/com/vaadin/flow/server/startup/ApplicationRouteRegistry.java#L254

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 104 at r2 (raw file):

        this.shell = shell;
        if (shell != null) {
            context.setAttribute(SHELL_KEY, shell.getName());

noobie question: does accessing the servlet context like this require synchronization?

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 130 at r2 (raw file):

        // Compare string names in order to works when classloaders are different
        return clz.getSuperclass().getName()
                .equals(VaadinAppShell.class.getName()); // NOSONAR

This check won't pass if the shell class is not a direct child of VaadinAppShell. E.g. with class MyShell extends MyAbstractShell and abstract class MyAbstractShell extends VaadinAppShell class MyShell won't pass the check, and the MyAbstractShell class would.

Developers may end up with such classes structure.
At very least we should give a meaningful error message (in case we do not support this use case).
Even better would be if Vaadin is able to handle this case.

Do you have any ideas on how to improve this?

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 144 at r2 (raw file):

    public String validateClass(Class<?> clz) {
        String error = null;
        if (shell != null) {

What's the main reason to skip the validation when shell is null?
This leads to a situation where there are no validation errors with a @Meta annotation on a MainLayout class as long as there is not AppShell class in the app. However, in that case the @Meta annotation is still ignored.

Would it make sense to run this validation in all cases where we know that the @Meta annotation (and others) won't have any effect?

[manolo]

Reviewable status: 30 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, and @vaadin-bot)

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 129 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

Indeed, the check in the servlet initializer uses the isAssignableFrom method:

} else if (VaadinAppShell.class.isAssignableFrom(clazz)) {
    // This is handled in separated validation
}

The check does not work in spring, for some reason the class passed in the initializer is taken with a different classloader.
I spent a while trying to figure out a better solution by figuring out the other classpath for the comparison.
The same happened for the scanner used for the dev-mode updaters, and there is a workaround in the ClassFinder used for that. I was tempted to use that class here, but the string comparison is simpler

[manolo]

Reviewable status: 19 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, @vaadin-bot, and @vlukashov)

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 21 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

when in clientSide mode

Let's try to use the same terms as in the documentation, what do you think?
I am not sure if there is a docs page describing what clientSide mode is. If there is, then this line is probably fine.

On the other hand, I know that there is a page describing what client-side bootstrapping is (https://vaadin.com/docs/v15/flow/ccdm/client-side-bootstrapping.html).
Would it be better to change the term used here to be more like when using client-side bootstrapping?

Done.

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 25 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

class

classes

Done.

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 25 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

 remove all annotations from other class

This is ambiguous: one can read this as literally no other annotations are allowed on any other classes in the app.

Done.

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 39 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

@Meta(name = "charset", content = "UTF-8")

When looking at this line a am getting a bunch of questions at once:

• Is this @Meta used by default when a developer adds a class AppShell extends VaadinAppShell?
• Is it used if the developer does not add the app shell class?
• What happens with the <meta name="charset" content="UTF-8"> tag that is present in the auto-generated index.html file? Would it be duplicated?

Can you please clarify these points in the Javadoc comment for this class?

UPD:
I've tried this in the test-ccdm test app, and this @Meta causes a startup failure (if the app defines its own class AppShell extends VaadinAppShell).

The intention was to use defaults values here. But yes, this is confusing and magic, removed.

---

flow-server/src/main/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandler.java, line 113 at r2 (raw file):

applyAppShellDocumentModifications
Done, but the method name is applyModifications because adding className and argument name sounds redundant, to me this is quite readable appShellRegistry.applyModifications(document)

---

flow-server/src/main/java/com/vaadin/flow/server/startup/AbstractAnnotationValidator.java, line 140 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

// This is handled in separated validation

Could you please also describe where is that separate validation? Otherwise this comment leaves me wondering.

Done.

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 50 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

Why does this class need to implement the ServletContextListener interface? Both methods of that interface are implemented as no-op in this class: contextInitialized() and contextDestroyed().

Good point, removed

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 62 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

This is an area where I do not have much context, so maybe it's a silly question.

I've noticed that the approach to getting a Servlet deployment config used here is different from the one used in the main Vaadin's ServletDeployer:

1. ServletDeployer expects that there can be several deployment configurations, not only one
2. ServletDeployer does not assume the servlet class to always be VaadinServlet

Are these differences relevant here, in VaadinAppShellInitializer?

https://github.com/vaadin/flow/blob/ccdm/flow-server/src/main/java/com/vaadin/flow/server/startup/ServletDeployer.java

private Collection<DeploymentConfiguration> getServletConfigurations(
        ServletContext context) {
    Collection<? extends ServletRegistration> registrations = context
            .getServletRegistrations().values();
    Collection<DeploymentConfiguration> result = new ArrayList<>(
            registrations.size());
    for (ServletRegistration registration : registrations) {
        loadClass(context.getClassLoader(), registration.getClassName())
                .ifPresent(servletClass -> result.add(
                        StubServletConfig.createDeploymentConfiguration(
                                context, registration, servletClass)));
    }
    return result;
}

This has been copied from the DevModeInitializer not sure if that approach also needs to be changed, but I think it does not have an impact in our case where we are checking just the isClientSideMode that should be the same.

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 100 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

        // Reset the registry
        registry.setShell(null, context);

nit: would it be more convenient if the method name was reset, so that the comment would be unnecessary?

Done

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 129 at r2 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

This produces error messages like this one:

Found configuration annotations in non `VaadinApplicationShell` classes.
The following annotations must be moved to the 'com.vaadin.flow.ccdmtest.AppShell' class:
  - com.vaadin.flow.component.page.VaadinAppShell contains: Meta
  - com.vaadin.flow.ccdmtest.MainLayout contains: Meta

I really like that they are detailed and specific! Great job!

nit: would it make sense to add a @ symbol before annotation names? (e.g. @Meta)

Done

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 44 at r2 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

VaadinApplicationShell

VaadinAppShell

Done.

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 48 at r2 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

This produces error messages like

Found multiple classes extending `VaadinApplicationShell` in the application
  class com.vaadin.flow.ccdmtest.AppShell
  com.vaadin.flow.ccdmtest.AbstractAppShell

I really like that the messages are specific and tell exactly what classes are found.
But, the message does not really tell what the developer should do to fix the issue. Please add a line telling what to do.

nit: also the first class does include the class word whereas the second line does not.

NOTE: in a similar case Spring Boot has the following error message

Unable to find a single main class from the following candidates [com.example.application.spring.Application2, com.example.application.spring.Application]

Done.

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 104 at r2 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

noobie question: does accessing the servlet context like this require synchronization?

Done.

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 130 at r2 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

This check won't pass if the shell class is not a direct child of VaadinAppShell. E.g. with class MyShell extends MyAbstractShell and abstract class MyAbstractShell extends VaadinAppShell class MyShell won't pass the check, and the MyAbstractShell class would.

Developers may end up with such classes structure.
At very least we should give a meaningful error message (in case we do not support this use case).
Even better would be if Vaadin is able to handle this case.

Do you have any ideas on how to improve this?

Done.

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 144 at r2 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

What's the main reason to skip the validation when shell is null?
This leads to a situation where there are no validation errors with a @Meta annotation on a MainLayout class as long as there is not AppShell class in the app. However, in that case the @Meta annotation is still ignored.

Would it make sense to run this validation in all cases where we know that the @Meta annotation (and others) won't have any effect?

Done.

[vlukashov]

Reviewable status: 9 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, @vaadin-bot, and @vlukashov)

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 25 at r1 (raw file):

No other annotations are allowed on any other classes in the application.

To me it's still not clear that this applies only to the @Meta annotation (and later also to @Viewport, @BodySize, and a few others). Let's try to re-write this to explain that point:

A marker class to configure the index.hml page when in client-side
bootstrapping. This class supports the following annotations that affect
the generated index.html page:

• @Meta: appends an HTML <meta> tag to the bottom of the <head> element
• other annotations to be added here
• and some more

There should be at max one class extending {@link VaadinAppShell} in the
application.

NOTE: the app shell class is the only valid target to place the page configuration
annotations listed above. The application would fail to start if any of these
annotations is wrongly placed on a class other than the app shell class.
would lead to a

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 25 at r3 (raw file):

?

looks like a typo (I expected a .)

[manolo]

Reviewable status: 9 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, @vaadin-bot, and @vlukashov)

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 25 at r1 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

No other annotations are allowed on any other classes in the application.

To me it's still not clear that this applies only to the @Meta annotation (and later also to @Viewport, @BodySize, and a few others). Let's try to re-write this to explain that point:

A marker class to configure the index.hml page when in client-side
bootstrapping. This class supports the following annotations that affect
the generated index.html page:

• @Meta: appends an HTML <meta> tag to the bottom of the <head> element
• other annotations to be added here
• and some more

There should be at max one class extending {@link VaadinAppShell} in the
application.

NOTE: the app shell class is the only valid target to place the page configuration
annotations listed above. The application would fail to start if any of these
annotations is wrongly placed on a class other than the app shell class.
would lead to a

Done

---

flow-server/src/main/java/com/vaadin/flow/component/page/VaadinAppShell.java, line 25 at r3 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

?

looks like a typo (I expected a .)

Done.

[vlukashov]

Reviewable status: 7 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, and @vaadin-bot)

a discussion (no related file):
It looks like most of the issues are addressed and the PR is ready to merge. I've found a bunch of small issues, but I did not want them to block this PR. So, I've collected them in a 'PR for PR': #7058

Please feel free to pick the ones that make sense for you, and discard the other.

---

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 48 at r4 (raw file):

@HandlesTypes({ VaadinAppShell.class, Meta.class })

I've just realized that Meta.Container.class is missing from this list. Now validation fails when a route class has one @Meta annotation, but passes when it has several of them.

Do you want to leave this for a separate PR? This one seems to be stuck in review for a while now.

[vlukashov]

Reviewed 3 of 10 files at r1, 1 of 6 files at r3, 7 of 7 files at r4.
Reviewable status: 7 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, and @vaadin-bot)

[manolo]

Reviewable status: 7 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, @vaadin-bot, and @vlukashov)

a discussion (no related file):

Previously, vlukashov (Viktor Lukashov) wrote…

Is it possible to include this functionality into one of the existing integration tests?
E.g. the test-ccdm and flow-ccdm-connect test projects seem to be suitable candidates.

working on this I will do in separated PR for the same ticket

---

a discussion (no related file):

Previously, vlukashov (Viktor Lukashov) wrote…

It looks like most of the issues are addressed and the PR is ready to merge. I've found a bunch of small issues, but I did not want them to block this PR. So, I've collected them in a 'PR for PR': #7058

Please feel free to pick the ones that make sense for you, and discard the other.

Done.

---

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 48 at r4 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

@HandlesTypes({ VaadinAppShell.class, Meta.class })

I've just realized that Meta.Container.class is missing from this list. Now validation fails when a route class has one @Meta annotation, but passes when it has several of them.

Do you want to leave this for a separate PR? This one seems to be stuck in review for a while now.

Done.

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellRegistry.java, line 129 at r1 (raw file):

Previously, manolo (Manuel Carrasco Moñino) wrote…

The check does not work in spring, for some reason the class passed in the initializer is taken with a different classloader.
I spent a while trying to figure out a better solution by figuring out the other classpath for the comparison.
The same happened for the scanner used for the dev-mode updaters, and there is a workaround in the ClassFinder used for that. I was tempted to use that class here, but the string comparison is simpler

Done.

[vlukashov]

Reviewed 7 of 8 files at r5, 1 of 1 files at r6.
Reviewable status: 5 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, and @vaadin-bot)

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 48 at r4 (raw file):

Previously, manolo (Manuel Carrasco Moñino) wrote…

Done.

That was quick! :)
Apparently, this requires a bit extra effort still because now the error message is off:

Found app shell configuration annotations in non `VaadinAppShell` classes.
Please create a custom class extending `VaadinAppShell` and move the following annotations to it:
    - @Container from com.vaadin.flow.ccdmtest.MainLayout

Can be done in a separate PR though.

[manolo]

Reviewable status: 5 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @manolo, @platosha, @vaadin-bot, and @vlukashov)

---

flow-server/src/main/java/com/vaadin/flow/server/startup/VaadinAppShellInitializer.java, line 48 at r4 (raw file):

Previously, vlukashov (Viktor Lukashov) wrote…

That was quick! :)
Apparently, this requires a bit extra effort still because now the error message is off:

Found app shell configuration annotations in non `VaadinAppShell` classes.
Please create a custom class extending `VaadinAppShell` and move the following annotations to it:
    - @Container from com.vaadin.flow.ccdmtest.MainLayout

Can be done in a separate PR though.

Done

[vaadin-bot]

Convert the abstract class "VaadinAppShell" into an interface.

[vaadin-bot]

Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed.

[vaadin-bot]

SonarQube analysis reported 2 issues

• 1 major
• 1 minor

Watch the comments in this conversation to review them.

[vlukashov]

Reviewed 2 of 10 files at r7, 4 of 8 files at r8, 3 of 3 files at r9, 1 of 1 files at r10.
Dismissed @vaadin-bot from 3 discussions.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @manolo and @platosha)

[vlukashov]

Reviewable status: 1 unresolved discussion, 1 of 1 LGTMs obtained (waiting on @manolo and @platosha)

[manolo]

Dismissed @vaadin-bot from 2 discussions.
Reviewable status: complete! all discussions resolved, 1 of 1 LGTMs obtained (waiting on @platosha)