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

Jump to bottom

Register a servlet automatically if WCs present #5331

Merged
merged 1 commit into from
Mar 25, 2019
Merged

Register a servlet automatically if WCs present #5331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 19 additions & 10 deletions flow-server/src/main/java/com/vaadin/flow/server/startup/ServletDeployer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@
*/
package com.vaadin.flow.server.startup;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.Optional;

import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
import javax.servlet.ServletContextEvent;
import javax.servlet.ServletContextListener;
import javax.servlet.ServletException;
import javax.servlet.ServletRegistration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.Optional;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -35,6 +36,7 @@
import com.vaadin.flow.server.DeploymentConfigurationFactory;
import com.vaadin.flow.server.VaadinServlet;
import com.vaadin.flow.server.VaadinServletConfiguration;
import com.vaadin.flow.server.webcomponent.WebComponentConfigurationRegistry;

/**
* Context listener that automatically registers Vaadin servlets.
Expand Down Expand Up @@ -65,8 +67,7 @@
public class ServletDeployer implements ServletContextListener {
private static final String SKIPPING_AUTOMATIC_SERVLET_REGISTRATION_BECAUSE = "Skipping automatic servlet registration because";

private static class StubServletConfig
implements ServletConfig {
private static class StubServletConfig implements ServletConfig {
private final ServletContext context;
private final ServletRegistration registration;

Expand Down Expand Up @@ -144,7 +145,8 @@ private DeploymentConfiguration createDeploymentConfiguration(
ServletConfig servletConfig, Class<?> servletClass) {
try {
return DeploymentConfigurationFactory
.createPropertyDeploymentConfiguration(servletClass, servletConfig);
.createPropertyDeploymentConfiguration(servletClass,
servletConfig);
} catch (ServletException e) {
throw new IllegalStateException(String.format(
"Failed to get deployment configuration data for servlet with name '%s' and class '%s'",
Expand All @@ -153,9 +155,16 @@ private DeploymentConfiguration createDeploymentConfiguration(
}

private void createAppServlet(ServletContext context) {
if (!ApplicationRouteRegistry.getInstance(context).hasNavigationTargets()) {
boolean createServlet = ApplicationRouteRegistry.getInstance(context)
.hasNavigationTargets();

createServlet = createServlet || WebComponentConfigurationRegistry
.getInstance(context).hasExporters();

if (!createServlet) {
getLogger().info(
"{} there are no navigation targets registered to the registry",
"{} there are no navigation targets registered to the "
+ "route registry and there are no web component exporters",
SKIPPING_AUTOMATIC_SERVLET_REGISTRATION_BECAUSE);
return;
}
Expand Down
108 changes: 60 additions & 48 deletions .../src/main/java/com/vaadin/flow/server/webcomponent/WebComponentConfigurationRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package com.vaadin.flow.server.webcomponent;

import javax.servlet.ServletContext;
import java.io.Serializable;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -26,6 +25,8 @@
import java.util.concurrent.locks.ReentrantLock;
import java.util.stream.Collectors;

import javax.servlet.ServletContext;

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.WebComponentExporter;
import com.vaadin.flow.component.webcomponent.WebComponentConfiguration;
Expand All @@ -47,10 +48,8 @@ public class WebComponentConfigurationRegistry implements Serializable {
*/
private final ReentrantLock configurationLock = new ReentrantLock(true);

private HashMap<String, Class<? extends WebComponentExporter<?
extends Component>>> exporterClasses = null;
private HashMap<String, WebComponentConfigurationImpl<? extends Component>>
builderCache = new HashMap<>();
private HashMap<String, Class<? extends WebComponentExporter<? extends Component>>> exporterClasses = null;
private HashMap<String, WebComponentConfigurationImpl<? extends Component>> builderCache = new HashMap<>();

/**
* Protected constructor for internal OSGi extensions.
Expand All @@ -63,23 +62,25 @@ protected WebComponentConfigurationRegistry() {
* registered.
*
* @param tag
* custom element tag
* custom element tag
* @return Optional containing a web component matching given tag
*/
public Optional<WebComponentConfiguration<? extends Component>> getConfiguration(String tag) {
public Optional<WebComponentConfiguration<? extends Component>> getConfiguration(
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR Remove usage of generic wildcard type. rule

String tag) {
return Optional.ofNullable(getConfigurationInternal(tag));
}

/**
* Retrieves {@link WebComponentConfigurationImpl} matching the {@code} tag. If the
* builder is not readily available, attempts to construct it from the
* web component exporter cache.
* Retrieves {@link WebComponentConfigurationImpl} matching the {@code} tag.
* If the builder is not readily available, attempts to construct it from
* the web component exporter cache.
*
* @param tag
* tag name of the web component
* tag name of the web component
* @return {@link WebComponentConfigurationImpl} by the tag
*/
protected WebComponentConfigurationImpl<? extends Component> getConfigurationInternal(String tag) {
protected WebComponentConfigurationImpl<? extends Component> getConfigurationInternal(
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR Remove usage of generic wildcard type. rule

String tag) {
WebComponentConfigurationImpl<? extends Component> builder = null;
configurationLock.lock();
try {
Expand All @@ -97,11 +98,14 @@ protected WebComponentConfigurationImpl<? extends Component> getConfigurationInt
* Get an unmodifiable set containing all registered web component
* configurations for a specific {@link Component} type.
*
* @param componentClass type of the exported {@link Component}
* @param <T> component
* @return set of {@link WebComponentConfiguration} or an empty set.
* @param componentClass
* type of the exported {@link Component}
* @param <T>
* component
* @return set of {@link WebComponentConfiguration} or an empty set.
*/
public <T extends Component> Set<WebComponentConfiguration<T>> getConfigurationsByComponentType(Class<T> componentClass) {
public <T extends Component> Set<WebComponentConfiguration<T>> getConfigurationsByComponentType(
Class<T> componentClass) {
configurationLock.lock();
try {
if (exporterClasses == null) {
Expand All @@ -112,7 +116,7 @@ public <T extends Component> Set<WebComponentConfiguration<T>> getConfigurations
}
return Collections.unmodifiableSet(builderCache.values().stream()
.filter(b -> componentClass.equals(b.getComponentClass()))
.map(b -> (WebComponentConfiguration<T>)b)
.map(b -> (WebComponentConfiguration<T>) b)
.collect(Collectors.toSet()));

} finally {
Expand All @@ -124,10 +128,10 @@ public <T extends Component> Set<WebComponentConfiguration<T>> getConfigurations
* Internal method for updating registry.
*
* @param exporters
* map of web component exporters to register
* map of web component exporters to register
*/
protected void updateRegistry(Map<String, Class<?
extends WebComponentExporter<? extends Component>>> exporters) {
protected void updateRegistry(
Map<String, Class<? extends WebComponentExporter<? extends Component>>> exporters) {
configurationLock.lock();
try {
if (exporters.isEmpty()) {
Expand All @@ -151,11 +155,11 @@ protected void updateRegistry(Map<String, Class<?
* false.
*
* @param exporters
* map of web component exporters to register
* map of web component exporters to register
* @return true if set successfully or false if not set
*/
public boolean setExporters(Map<String, Class<?
extends WebComponentExporter<? extends Component>>> exporters) {
public boolean setExporters(
Map<String, Class<? extends WebComponentExporter<? extends Component>>> exporters) {
configurationLock.lock();
try {
if (exporterClasses != null) {
Expand All @@ -171,15 +175,20 @@ public boolean setExporters(Map<String, Class<?
}

public boolean hasExporters() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

CRITICAL Document this public method by adding an explicit description. rule

return exporterClasses != null;
configurationLock.lock();
try {
return exporterClasses != null;
} finally {
configurationLock.unlock();
}
}

/**
* Get an unmodifiable set containing all registered web component
* configurations.
*
* @return unmodifiable set of web component builders in registry or
* empty set
* @return unmodifiable set of web component builders in registry or empty
* set
*/
public Set<WebComponentConfiguration<? extends Component>> getConfigurations() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR Remove usage of generic wildcard type. rule

configurationLock.lock();
Expand All @@ -188,8 +197,8 @@ public Set<WebComponentConfiguration<? extends Component>> getConfigurations() {
populateCacheWithMissingConfigurations();
}

return Collections.unmodifiableSet(new HashSet<>(
builderCache.values()));
return Collections
.unmodifiableSet(new HashSet<>(builderCache.values()));
} finally {
configurationLock.unlock();
}
Expand All @@ -199,7 +208,7 @@ public Set<WebComponentConfiguration<? extends Component>> getConfigurations() {
* Get WebComponentRegistry instance for given servlet context.
*
* @param servletContext
* servlet context to get registry for
* servlet context to get registry for
* @return WebComponentRegistry instance
*/
public static WebComponentConfigurationRegistry getInstance(
Expand All @@ -208,14 +217,14 @@ public static WebComponentConfigurationRegistry getInstance(

Object attribute;
synchronized (servletContext) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR "servletContext" is a method parameter, and should not be used for synchronization. rule

attribute = servletContext
.getAttribute(WebComponentConfigurationRegistry.class.getName());
attribute = servletContext.getAttribute(
WebComponentConfigurationRegistry.class.getName());

if (attribute == null) {
attribute = createRegistry(servletContext);
servletContext
.setAttribute(WebComponentConfigurationRegistry.class.getName(),
attribute);
servletContext.setAttribute(
WebComponentConfigurationRegistry.class.getName(),
attribute);
}
}

Expand All @@ -227,12 +236,14 @@ public static WebComponentConfigurationRegistry getInstance(
}
}

private static WebComponentConfigurationRegistry createRegistry(ServletContext context) {
private static WebComponentConfigurationRegistry createRegistry(
ServletContext context) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR Remove this unused method parameter "context". rule

if (OSGiAccess.getInstance().getOsgiServletContext() == null) {
return new WebComponentConfigurationRegistry();
}
Object attribute = OSGiAccess.getInstance().getOsgiServletContext()
.getAttribute(WebComponentConfigurationRegistry.class.getName());
.getAttribute(
WebComponentConfigurationRegistry.class.getName());
if (attribute != null
&& attribute instanceof OSGiWebComponentConfigurationRegistry) {
return (WebComponentConfigurationRegistry) attribute;
Expand All @@ -245,7 +256,8 @@ private static WebComponentConfigurationRegistry createRegistry(ServletContext c
* Adds a builder to the builder cache if one is not already present,
* exporters have been set, and a matching exporter is found.
*
* @param tag name of the web component
* @param tag
* name of the web component
*/
protected void populateCacheByTag(String tag) {
configurationLock.lock();
Expand All @@ -255,11 +267,12 @@ protected void populateCacheByTag(String tag) {
return;
}

Class<? extends WebComponentExporter<? extends Component>> exporterClass
= exporterClasses.get(tag);
Class<? extends WebComponentExporter<? extends Component>> exporterClass = exporterClasses
.get(tag);

if (exporterClass != null) {
builderCache.put(tag, constructConfigurations(tag, exporterClass));
builderCache.put(tag,
constructConfigurations(tag, exporterClass));
// remove the class reference from the data bank - it has
// already been constructed and is no longer needed
exporterClasses.remove(tag);
Expand Down Expand Up @@ -287,22 +300,21 @@ protected boolean areAllConfigurationsAvailable() {
configurationLock.lock();

try {
return exporterClasses != null &&
exporterClasses.size() == 0;
return exporterClasses != null && exporterClasses.size() == 0;
} finally {
configurationLock.unlock();
}
}

protected WebComponentConfigurationImpl<? extends Component> constructConfigurations(
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR Remove usage of generic wildcard type. rule

String tag, Class<? extends WebComponentExporter<?
extends Component>> exporterClass) {
String tag,
Class<? extends WebComponentExporter<? extends Component>> exporterClass) {

Instantiator instantiator =
VaadinService.getCurrent().getInstantiator();
Instantiator instantiator = VaadinService.getCurrent()
.getInstantiator();

WebComponentExporter<? extends Component> exporter =
instantiator.getOrCreate(exporterClass);
WebComponentExporter<? extends Component> exporter = instantiator
.getOrCreate(exporterClass);

return new WebComponentConfigurationImpl<>(tag, exporter);
}
Expand Down
Loading