From b8e84da2e2dc4bcaed40d169c40a97f3d11e0648 Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Mon, 28 Nov 2016 10:10:21 +0300 Subject: Correct all tests that introspect classpath for Vaadin classes. Fixes vaadin/framework8-issues#399 RemoveListenersDeprecatedTest test is fixed. Corrections are made to make the test above passes. Change-Id: I209a4693d241a1488b69b4742f48549dbf4bf0ac --- .../src/main/java/com/vaadin/v7/ui/Grid.java | 7 +- .../com/vaadin/tests/server/ClasspathHelper.java | 122 ------------- .../tests/server/ComponentDesignWriterUtility.java | 2 + .../com/vaadin/tests/server/DeprecatedTest.java | 5 +- .../java/com/vaadin/v7/tests/VaadinClasses.java | 60 +++++-- .../abstractfield/FieldDefaultValuesTest.java | 7 +- .../java/com/vaadin/event/MethodEventSource.java | 6 + .../src/main/java/com/vaadin/event/SortEvent.java | 7 +- .../main/java/com/vaadin/navigator/Navigator.java | 10 +- .../com/vaadin/server/AbstractClientConnector.java | 6 + .../com/vaadin/server/VaadinPortletSession.java | 20 ++- .../main/java/com/vaadin/server/VaadinService.java | 12 +- .../main/java/com/vaadin/server/VaadinSession.java | 14 +- server/src/main/java/com/vaadin/ui/LoginForm.java | 6 +- server/src/main/java/com/vaadin/ui/Slider.java | 2 +- .../components/colorpicker/ColorPickerHistory.java | 4 +- .../components/colorpicker/ColorPickerPopup.java | 40 ++++- .../components/colorpicker/ColorPickerPreview.java | 30 +++- .../components/colorpicker/ColorPickerSelect.java | 13 ++ .../com/vaadin/ui/declarative/DesignContext.java | 9 +- .../server/RemoveListenersDeprecatedTest.java | 78 ++++++++- .../test/java/com/vaadin/tests/VaadinClasses.java | 193 ++++----------------- .../com/vaadin/tests/server/ClasspathHelper.java | 137 +++++++++++++++ .../tests/server/component/FinalMethodTest.java | 21 ++- .../component/StateGetDoesNotMarkDirtyTest.java | 77 +++++--- .../componentcontainer/AddRemoveComponentTest.java | 23 ++- .../com/vaadin/shared/ui/ComponentStateUtil.java | 11 +- .../main/java/com/vaadin/tests/VaadinClasses.java | 15 -- 28 files changed, 545 insertions(+), 392 deletions(-) delete mode 100644 compatibility-server/src/test/java/com/vaadin/tests/server/ClasspathHelper.java create mode 100644 server/src/test/java/com/vaadin/tests/server/ClasspathHelper.java diff --git a/compatibility-server/src/main/java/com/vaadin/v7/ui/Grid.java b/compatibility-server/src/main/java/com/vaadin/v7/ui/Grid.java index db4d75bbfc..7422fd8ffc 100644 --- a/compatibility-server/src/main/java/com/vaadin/v7/ui/Grid.java +++ b/compatibility-server/src/main/java/com/vaadin/v7/ui/Grid.java @@ -57,6 +57,7 @@ import com.vaadin.server.JsonCodec; import com.vaadin.server.KeyMapper; import com.vaadin.server.VaadinSession; import com.vaadin.shared.MouseEventDetails; +import com.vaadin.shared.Registration; import com.vaadin.shared.data.sort.SortDirection; import com.vaadin.shared.util.SharedUtil; import com.vaadin.ui.AbstractFocusable; @@ -89,9 +90,9 @@ import com.vaadin.v7.data.util.IndexedContainer; import com.vaadin.v7.data.util.converter.Converter; import com.vaadin.v7.data.util.converter.ConverterUtil; import com.vaadin.v7.event.ItemClickEvent; -import com.vaadin.v7.event.SelectionEvent; import com.vaadin.v7.event.ItemClickEvent.ItemClickListener; import com.vaadin.v7.event.ItemClickEvent.ItemClickNotifier; +import com.vaadin.v7.event.SelectionEvent; import com.vaadin.v7.event.SelectionEvent.SelectionListener; import com.vaadin.v7.event.SelectionEvent.SelectionNotifier; import com.vaadin.v7.server.communication.data.DataGenerator; @@ -6155,8 +6156,10 @@ public class Grid extends AbstractFocusable implements SelectionNotifier, * the sort order change listener to add */ @Override - public void addSortListener(SortListener listener) { + public Registration addSortListener(SortListener listener) { addListener(SortEvent.class, listener, SORT_ORDER_CHANGE_METHOD); + return () -> removeListener(SortEvent.class, listener, + SORT_ORDER_CHANGE_METHOD); } /** diff --git a/compatibility-server/src/test/java/com/vaadin/tests/server/ClasspathHelper.java b/compatibility-server/src/test/java/com/vaadin/tests/server/ClasspathHelper.java deleted file mode 100644 index 97005966f3..0000000000 --- a/compatibility-server/src/test/java/com/vaadin/tests/server/ClasspathHelper.java +++ /dev/null @@ -1,122 +0,0 @@ -package com.vaadin.tests.server; - -import java.io.File; -import java.io.IOException; -import java.lang.reflect.Modifier; -import java.net.URI; -import java.nio.file.FileSystems; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Locale; -import java.util.Objects; -import java.util.function.Predicate; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -/** - * Allows to get classes from the current classpath using classes FQN filter. - *

- * The methods in the class return all real (not anonymous and not private) - * classes from the filtered classpath. - * - * @author Vaadin Ltd - * - */ -class ClasspathHelper { - - public static final String COM_VAADIN_FILE_PREFIX = "com" + File.separatorChar + "vaadin" + File.separatorChar; - private final Predicate skipClassesFilter; - - ClasspathHelper(Predicate skipClassesFilter) { - this.skipClassesFilter = skipClassesFilter; - } - - Stream> getVaadinClassesFromClasspath( - Predicate classpathFilter, - Predicate> classFilter) { - return getRawClasspathEntries().stream().filter(classpathFilter) - .map(File::new).map(file -> getVaadinClassesFromFile(file)) - .flatMap(List::stream).filter(classFilter) - .filter(cls -> !cls.isSynthetic() && !cls.isAnonymousClass() - && !Modifier.isPrivate(cls.getModifiers())); - - } - - Stream> getVaadinClassesFromClasspath( - Predicate classpathFilter) { - return getVaadinClassesFromClasspath(classpathFilter, cls -> true); - } - - private List> getVaadinClassesFromFile(File classesRoot) { - try { - if (classesRoot.isDirectory()) { - return Files.walk(classesRoot.toPath()) - .filter(Files::isRegularFile) - .filter(path -> path.toFile().getName() - .endsWith(".class")) - .filter(path -> classesRoot.toPath().relativize(path) - .toString().contains(COM_VAADIN_FILE_PREFIX)) - .map(path -> getClassFromFile(path, - classesRoot.toPath())) - .filter(Objects::nonNull).collect(Collectors.toList()); - } else if (classesRoot.getName().toLowerCase(Locale.ENGLISH) - .endsWith(".jar")) { - URI uri = URI.create("jar:file:" + classesRoot.getPath()); - Path root = FileSystems - .newFileSystem(uri, Collections.emptyMap()) - .getPath(File.separator); - return Files.walk(root).filter(Files::isRegularFile) - .filter(path -> path.toUri().getSchemeSpecificPart() - .endsWith(".class")) - .filter(path -> root.relativize(path).toString() - .contains(COM_VAADIN_FILE_PREFIX)) - .map(path -> getClassFromFile(path, root)) - .filter(Objects::nonNull).collect(Collectors.toList()); - } - return null; - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - private Class getClassFromFile(Path path, Path root) { - Path relative = root.relativize(path); - String name = relative.toString(); - name = name.substring(0, name.length() - ".class".length()); - name = name.replace(File.separatorChar, '.'); - if (skipClassesFilter.test(name)) { - return null; - } - try { - return Class.forName(name, false, getClass().getClassLoader()); - } catch (ClassNotFoundException e) { - throw new RuntimeException(e); - } - } - - private final static List getRawClasspathEntries() { - List locations = new ArrayList<>(); - - String pathSep = System.getProperty("path.separator"); - String classpath = System.getProperty("java.class.path"); - - if (classpath.startsWith("\"")) { - classpath = classpath.substring(1); - } - if (classpath.endsWith("\"")) { - classpath = classpath.substring(0, classpath.length() - 1); - } - - String[] split = classpath.split(pathSep); - for (int i = 0; i < split.length; i++) { - String classpathEntry = split[i]; - locations.add(classpathEntry); - } - - return locations; - } - -} diff --git a/compatibility-server/src/test/java/com/vaadin/tests/server/ComponentDesignWriterUtility.java b/compatibility-server/src/test/java/com/vaadin/tests/server/ComponentDesignWriterUtility.java index 6009385ccc..00022ba44c 100644 --- a/compatibility-server/src/test/java/com/vaadin/tests/server/ComponentDesignWriterUtility.java +++ b/compatibility-server/src/test/java/com/vaadin/tests/server/ComponentDesignWriterUtility.java @@ -73,6 +73,8 @@ public class ComponentDesignWriterUtility { "com.vaadin.server.communication.PushAtmosphereHandler$AtmosphereResourceListener"); WHITE_LIST_FQNS .add("com.vaadin.server.communication.PushAtmosphereHandler"); + WHITE_LIST_FQNS + .add("com.vaadin.server.communication.PushRequestHandler$1"); WHITE_LIST_FQNS .add("com.vaadin.server.communication.PushRequestHandler$2"); WHITE_LIST_FQNS.add("com.vaadin.server.LegacyVaadinPortlet"); diff --git a/compatibility-server/src/test/java/com/vaadin/tests/server/DeprecatedTest.java b/compatibility-server/src/test/java/com/vaadin/tests/server/DeprecatedTest.java index ab04f0616a..f00592af6d 100644 --- a/compatibility-server/src/test/java/com/vaadin/tests/server/DeprecatedTest.java +++ b/compatibility-server/src/test/java/com/vaadin/tests/server/DeprecatedTest.java @@ -34,7 +34,7 @@ public class DeprecatedTest { File testRoot = new File(DeprecatedTest.class.getResource("/").toURI()); - new ClasspathHelper(fqn -> false) + new ClasspathHelper() .getVaadinClassesFromClasspath( entry -> entry.contains("compatibility-server") && !testRoot.equals(new File(entry))) @@ -45,7 +45,8 @@ public class DeprecatedTest { + " is in compatability package and it's not deprecated", cls.getAnnotation(Deprecated.class)); }); - Assert.assertNotEquals("Total number of checked classes", 0, count.get()); + Assert.assertNotEquals("Total number of checked classes", 0, + count.get()); } } diff --git a/compatibility-server/src/test/java/com/vaadin/v7/tests/VaadinClasses.java b/compatibility-server/src/test/java/com/vaadin/v7/tests/VaadinClasses.java index 251f8b0223..612f85cd33 100644 --- a/compatibility-server/src/test/java/com/vaadin/v7/tests/VaadinClasses.java +++ b/compatibility-server/src/test/java/com/vaadin/v7/tests/VaadinClasses.java @@ -1,31 +1,65 @@ package com.vaadin.v7.tests; -import java.io.IOException; +import java.io.File; +import java.lang.reflect.Modifier; +import java.net.URISyntaxException; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import com.vaadin.tests.server.ClasspathHelper; import com.vaadin.ui.Component; import com.vaadin.v7.ui.Field; @SuppressWarnings("deprecation") public class VaadinClasses { + private static final Set WHITE_LIST_FQNS = new HashSet<>(); + public static List> getFields() { + return getServerClasses(Field.class::isAssignableFrom) + .map(VaadinClasses::castFieldClass) + .collect(Collectors.toList()); + } + + public static Stream> getServerClasses( + Predicate> predicate) { try { - return com.vaadin.tests.VaadinClasses.findClasses(Field.class, - "com.vaadin.v7.ui"); - } catch (IOException e) { - e.printStackTrace(); - return null; + File testRoot = new File(com.vaadin.tests.VaadinClasses.class + .getResource("/").toURI()); + File compatibilityTestRoot = new File( + VaadinClasses.class.getResource("/").toURI()); + ClasspathHelper helper = new ClasspathHelper( + fqn -> !fqn.startsWith("com.vaadin.v7.ui")); + return helper.getVaadinClassesFromClasspath( + entry -> !compatibilityTestRoot.equals(new File(entry)) + && !testRoot.equals(new File(entry)), + cls -> predicate.test(cls) && !cls.isInterface() + && !Modifier.isAbstract(cls.getModifiers())); + } catch (URISyntaxException e) { + throw new RuntimeException(e); } } public static List> getComponents() { - try { - return com.vaadin.tests.VaadinClasses.findClasses(Component.class, - "com.vaadin.v7.ui"); - } catch (IOException e) { - throw new RuntimeException( - "Could not find all Vaadin component classes", e); - } + return getServerClasses(Component.class::isAssignableFrom) + .map(VaadinClasses::castComponentClass) + .collect(Collectors.toList()); + } + + private static Class castFieldClass(Class clazz) { + return (Class) clazz; + } + + private static Class castComponentClass( + Class clazz) { + return (Class) clazz; + } + + protected static Set getWhiteListFqns() { + return WHITE_LIST_FQNS; } } diff --git a/compatibility-server/src/test/java/com/vaadin/v7/tests/server/component/abstractfield/FieldDefaultValuesTest.java b/compatibility-server/src/test/java/com/vaadin/v7/tests/server/component/abstractfield/FieldDefaultValuesTest.java index a53e3bbbd1..3ee4ce607b 100644 --- a/compatibility-server/src/test/java/com/vaadin/v7/tests/server/component/abstractfield/FieldDefaultValuesTest.java +++ b/compatibility-server/src/test/java/com/vaadin/v7/tests/server/component/abstractfield/FieldDefaultValuesTest.java @@ -21,9 +21,9 @@ import java.util.List; import org.junit.Assert; import org.junit.Test; -import com.vaadin.ui.Slider; import com.vaadin.v7.tests.VaadinClasses; import com.vaadin.v7.ui.Field; +import com.vaadin.v7.ui.Slider; public class FieldDefaultValuesTest { @@ -45,7 +45,9 @@ public class FieldDefaultValuesTest { @Test public void testFieldsAreEmptyAfterClear() throws Exception { + int count = 0; for (Field field : createFields()) { + count++; field.clear(); if (field instanceof Slider) { @@ -60,12 +62,13 @@ public class FieldDefaultValuesTest { field.isEmpty()); } } + Assert.assertTrue(count > 0); } @SuppressWarnings("rawtypes") private static List> createFields() throws InstantiationException, IllegalAccessException { - List> fieldInstances = new ArrayList>(); + List> fieldInstances = new ArrayList<>(); for (Class fieldType : VaadinClasses.getFields()) { fieldInstances.add(fieldType.newInstance()); diff --git a/server/src/main/java/com/vaadin/event/MethodEventSource.java b/server/src/main/java/com/vaadin/event/MethodEventSource.java index 2ba5586eb1..e59ccfd4d4 100644 --- a/server/src/main/java/com/vaadin/event/MethodEventSource.java +++ b/server/src/main/java/com/vaadin/event/MethodEventSource.java @@ -141,7 +141,10 @@ public interface MethodEventSource extends Serializable { * @param method * the method owned by the target that's registered to listen to * events of type eventType. + * @deprecated use a {@link Registration} returned by + * {@link #addListener(Class, Object, Method)} */ + @Deprecated public void removeListener(Class eventType, Object target, Method method); @@ -172,7 +175,10 @@ public interface MethodEventSource extends Serializable { * @param methodName * the name of the method owned by target that's * registered to listen to events of type eventType. + * @deprecated use a {@link Registration} returned by + * {@link #addListener(Class, Object, String)} */ + @Deprecated public void removeListener(Class eventType, Object target, String methodName); } diff --git a/server/src/main/java/com/vaadin/event/SortEvent.java b/server/src/main/java/com/vaadin/event/SortEvent.java index e9dd4c75ae..0bc6d4b2cf 100644 --- a/server/src/main/java/com/vaadin/event/SortEvent.java +++ b/server/src/main/java/com/vaadin/event/SortEvent.java @@ -19,6 +19,7 @@ import java.io.Serializable; import java.util.List; import com.vaadin.data.sort.SortOrder; +import com.vaadin.shared.Registration; import com.vaadin.ui.Component; /** @@ -95,8 +96,9 @@ public class SortEvent extends Component.Event { * * @param listener * the sort order change listener to add + * @return a registration object for removing the listener */ - public void addSortListener(SortListener listener); + public Registration addSortListener(SortListener listener); /** * Removes a sort order change listener previously added using @@ -104,7 +106,10 @@ public class SortEvent extends Component.Event { * * @param listener * the sort order change listener to remove + * @deprecated use a {@link Registration} returned by + * {@link #addSortListener(SortListener)} */ + @Deprecated public void removeSortListener(SortListener listener); } } diff --git a/server/src/main/java/com/vaadin/navigator/Navigator.java b/server/src/main/java/com/vaadin/navigator/Navigator.java index 07d7c5c085..163db74feb 100644 --- a/server/src/main/java/com/vaadin/navigator/Navigator.java +++ b/server/src/main/java/com/vaadin/navigator/Navigator.java @@ -41,6 +41,7 @@ import com.vaadin.navigator.ViewChangeListener.ViewChangeEvent; import com.vaadin.server.Page; import com.vaadin.server.Page.UriFragmentChangedEvent; import com.vaadin.server.Page.UriFragmentChangedListener; +import com.vaadin.shared.Registration; import com.vaadin.shared.util.SharedUtil; import com.vaadin.ui.Component; import com.vaadin.ui.ComponentContainer; @@ -345,7 +346,7 @@ public class Navigator implements Serializable { throw new RuntimeException(e); } // TODO error handling - + } return null; } @@ -975,8 +976,9 @@ public class Navigator implements Serializable { * @param listener * Listener to invoke during a view change. */ - public void addViewChangeListener(ViewChangeListener listener) { + public Registration addViewChangeListener(ViewChangeListener listener) { listeners.add(listener); + return () -> listeners.remove(listener); } /** @@ -984,7 +986,11 @@ public class Navigator implements Serializable { * * @param listener * Listener to remove. + * @deprecated use a {@link Registration} object returned by + * {@link #addViewChangeListener(ViewChangeListener)} to remove + * a listener */ + @Deprecated public void removeViewChangeListener(ViewChangeListener listener) { listeners.remove(listener); } diff --git a/server/src/main/java/com/vaadin/server/AbstractClientConnector.java b/server/src/main/java/com/vaadin/server/AbstractClientConnector.java index 32f62edb8e..5c905e2f84 100644 --- a/server/src/main/java/com/vaadin/server/AbstractClientConnector.java +++ b/server/src/main/java/com/vaadin/server/AbstractClientConnector.java @@ -103,6 +103,7 @@ public abstract class AbstractClientConnector } @Override + @Deprecated public void removeAttachListener(AttachListener listener) { removeListener(AttachEvent.ATTACH_EVENT_IDENTIFIER, AttachEvent.class, listener); @@ -115,6 +116,7 @@ public abstract class AbstractClientConnector } @Override + @Deprecated public void removeDetachListener(DetachListener listener) { removeListener(DetachEvent.DETACH_EVENT_IDENTIFIER, DetachEvent.class, listener); @@ -940,8 +942,12 @@ public abstract class AbstractClientConnector * @param method * the method owned by target that's registered to * listen to events of type eventType. + * @deprecated use a {@link Registration} from + * {@link #addListener(Class, Object, Method)} to remove a + * listener */ @Override + @Deprecated public void removeListener(Class eventType, Object target, Method method) { if (eventRouter != null) { diff --git a/server/src/main/java/com/vaadin/server/VaadinPortletSession.java b/server/src/main/java/com/vaadin/server/VaadinPortletSession.java index f1b32efabe..68a1470e05 100644 --- a/server/src/main/java/com/vaadin/server/VaadinPortletSession.java +++ b/server/src/main/java/com/vaadin/server/VaadinPortletSession.java @@ -42,6 +42,7 @@ import javax.servlet.http.HttpSessionBindingListener; import javax.xml.namespace.QName; import com.vaadin.server.communication.PortletListenerNotifier; +import com.vaadin.shared.Registration; import com.vaadin.ui.UI; import com.vaadin.util.CurrentInstance; @@ -123,8 +124,9 @@ public class VaadinPortletSession extends VaadinSession { * @param listener * to add */ - public void addPortletListener(PortletListener listener) { + public Registration addPortletListener(PortletListener listener) { portletListeners.add(listener); + return () -> portletListeners.remove(listener); } /** @@ -133,7 +135,11 @@ public class VaadinPortletSession extends VaadinSession { * * @param listener * to remove + * @deprecated Use a {@link Registration} object returned by + * {@link #addPortletListener(PortletListener)} to remove a + * listener */ + @Deprecated public void removePortletListener(PortletListener listener) { portletListeners.remove(listener); } @@ -143,8 +149,7 @@ public class VaadinPortletSession extends VaadinSession { */ public void firePortletRenderRequest(UI uI, RenderRequest request, RenderResponse response) { - for (PortletListener l : new ArrayList<>( - portletListeners)) { + for (PortletListener l : new ArrayList<>(portletListeners)) { l.handleRenderRequest(request, new RestrictedRenderResponse(response), uI); } @@ -172,8 +177,7 @@ public class VaadinPortletSession extends VaadinSession { sharedParameterActionValueMap.remove(key); } else { // normal action request, notify listeners - for (PortletListener l : new ArrayList<>( - portletListeners)) { + for (PortletListener l : new ArrayList<>(portletListeners)) { l.handleActionRequest(request, response, uI); } } @@ -184,8 +188,7 @@ public class VaadinPortletSession extends VaadinSession { */ public void firePortletEventRequest(UI uI, EventRequest request, EventResponse response) { - for (PortletListener l : new ArrayList<>( - portletListeners)) { + for (PortletListener l : new ArrayList<>(portletListeners)) { l.handleEventRequest(request, response, uI); } } @@ -195,8 +198,7 @@ public class VaadinPortletSession extends VaadinSession { */ public void firePortletResourceRequest(UI uI, ResourceRequest request, ResourceResponse response) { - for (PortletListener l : new ArrayList<>( - portletListeners)) { + for (PortletListener l : new ArrayList<>(portletListeners)) { l.handleResourceRequest(request, response, uI); } } diff --git a/server/src/main/java/com/vaadin/server/VaadinService.java b/server/src/main/java/com/vaadin/server/VaadinService.java index 62d9d75edb..70a60a4f1f 100644 --- a/server/src/main/java/com/vaadin/server/VaadinService.java +++ b/server/src/main/java/com/vaadin/server/VaadinService.java @@ -415,10 +415,8 @@ public abstract class VaadinService implements Serializable { * @return a registration object for removing the listener */ public Registration addSessionInitListener(SessionInitListener listener) { - eventRouter.addListener(SessionInitEvent.class, listener, + return eventRouter.addListener(SessionInitEvent.class, listener, SESSION_INIT_METHOD); - return () -> eventRouter.removeListener(SessionInitEvent.class, - listener, SESSION_INIT_METHOD); } /** @@ -454,10 +452,8 @@ public abstract class VaadinService implements Serializable { */ public Registration addSessionDestroyListener( SessionDestroyListener listener) { - eventRouter.addListener(SessionDestroyEvent.class, listener, + return eventRouter.addListener(SessionDestroyEvent.class, listener, SESSION_DESTROY_METHOD); - return () -> eventRouter.removeListener(SessionInitEvent.class, - listener, SESSION_DESTROY_METHOD); } /** @@ -1877,10 +1873,8 @@ public abstract class VaadinService implements Serializable { */ public Registration addServiceDestroyListener( ServiceDestroyListener listener) { - eventRouter.addListener(ServiceDestroyEvent.class, listener, + return eventRouter.addListener(ServiceDestroyEvent.class, listener, SERVICE_DESTROY_METHOD); - return () -> eventRouter.removeListener(ServiceDestroyEvent.class, - listener, SERVICE_DESTROY_METHOD); } /** diff --git a/server/src/main/java/com/vaadin/server/VaadinSession.java b/server/src/main/java/com/vaadin/server/VaadinSession.java index ebda81b9c7..3e3202ee1b 100644 --- a/server/src/main/java/com/vaadin/server/VaadinSession.java +++ b/server/src/main/java/com/vaadin/server/VaadinSession.java @@ -48,6 +48,7 @@ import javax.servlet.http.HttpSessionBindingEvent; import javax.servlet.http.HttpSessionBindingListener; import com.vaadin.event.EventRouter; +import com.vaadin.shared.Registration; import com.vaadin.shared.communication.PushMode; import com.vaadin.ui.UI; import com.vaadin.util.CurrentInstance; @@ -822,13 +823,20 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { * * @param listener * the bootstrap listener to add + * @return a registration object for removing the listener */ - public void addBootstrapListener(BootstrapListener listener) { + public Registration addBootstrapListener(BootstrapListener listener) { assert hasLock(); eventRouter.addListener(BootstrapFragmentResponse.class, listener, BOOTSTRAP_FRAGMENT_METHOD); eventRouter.addListener(BootstrapPageResponse.class, listener, BOOTSTRAP_PAGE_METHOD); + return () -> { + eventRouter.removeListener(BootstrapFragmentResponse.class, + listener, BOOTSTRAP_FRAGMENT_METHOD); + eventRouter.removeListener(BootstrapPageResponse.class, listener, + BOOTSTRAP_PAGE_METHOD); + }; } /** @@ -838,7 +846,11 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { * * @param listener * the bootstrap listener to remove + * @deprecated Use a {@link Registration} object returned by + * {@link #addBootstrapListener(BootstrapListener)} to remove a + * listener */ + @Deprecated public void removeBootstrapListener(BootstrapListener listener) { assert hasLock(); eventRouter.removeListener(BootstrapFragmentResponse.class, listener, diff --git a/server/src/main/java/com/vaadin/ui/LoginForm.java b/server/src/main/java/com/vaadin/ui/LoginForm.java index 6b6bfec5bc..abef0c1276 100644 --- a/server/src/main/java/com/vaadin/ui/LoginForm.java +++ b/server/src/main/java/com/vaadin/ui/LoginForm.java @@ -326,17 +326,17 @@ public class LoginForm extends AbstractSingleComponentContainer { private TextField getUsernameField() { assert initialized; - return (TextField) getState().userNameFieldConnector; + return (TextField) getState(false).userNameFieldConnector; } private PasswordField getPasswordField() { assert initialized; - return (PasswordField) getState().passwordFieldConnector; + return (PasswordField) getState(false).passwordFieldConnector; } private Button getLoginButton() { assert initialized; - return (Button) getState().loginButtonConnector; + return (Button) getState(false).loginButtonConnector; } /** diff --git a/server/src/main/java/com/vaadin/ui/Slider.java b/server/src/main/java/com/vaadin/ui/Slider.java index 16c287d92c..58964d1378 100644 --- a/server/src/main/java/com/vaadin/ui/Slider.java +++ b/server/src/main/java/com/vaadin/ui/Slider.java @@ -304,7 +304,7 @@ public class Slider extends AbstractField { @Override public Double getValue() { - return getState().value; + return getState(false).value; } @Override diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerHistory.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerHistory.java index ae1c304fd2..101a4b642a 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerHistory.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerHistory.java @@ -50,8 +50,8 @@ public class ColorPickerHistory extends CustomField { ColorPickerGrid grid = new ColorPickerGrid(ROWS, COLUMNS); grid.setWidth("100%"); grid.setPosition(0, 0); - grid.addValueChangeListener(event -> fireEvent(new ValueChangeEvent<>(this, - event.isUserOriginated()))); + grid.addValueChangeListener(event -> fireEvent( + new ValueChangeEvent<>(this, event.isUserOriginated()))); return grid; } diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java index 15fc1bcb45..9d88b03fa5 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java @@ -18,6 +18,7 @@ package com.vaadin.ui.components.colorpicker; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Objects; import java.util.Set; @@ -33,6 +34,7 @@ import com.vaadin.ui.Alignment; import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickEvent; import com.vaadin.ui.Component; +import com.vaadin.ui.HasComponents; import com.vaadin.ui.HorizontalLayout; import com.vaadin.ui.Layout; import com.vaadin.ui.Slider; @@ -119,6 +121,10 @@ public class ColorPickerPopup extends Window implements HasValue { /** The selectors. */ private final Set> selectors = new HashSet<>(); + private boolean readOnly; + + private boolean required; + /** * Set true while the slider values are updated after colorChange. When * true, valueChange reactions from the sliders are disabled, because @@ -175,6 +181,7 @@ public class ColorPickerPopup extends Window implements HasValue { selPreview = new ColorPickerPreview(selectedColor); selPreview.setWidth("100%"); selPreview.setHeight("20px"); + selPreview.setRequiredIndicatorVisible(required); selPreview.addValueChangeListener(this::colorChanged); selectors.add(selPreview); @@ -711,12 +718,15 @@ public class ColorPickerPopup extends Window implements HasValue { @Override public void setRequiredIndicatorVisible(boolean visible) { - super.setRequiredIndicatorVisible(visible); + required = visible; + if (selPreview != null) { + selPreview.setRequiredIndicatorVisible(required); + } } @Override public boolean isRequiredIndicatorVisible() { - return super.isRequiredIndicatorVisible(); + return required; } private static Logger getLogger() { @@ -725,11 +735,33 @@ public class ColorPickerPopup extends Window implements HasValue { @Override public void setReadOnly(boolean readOnly) { - super.setReadOnly(readOnly); + this.readOnly = readOnly; + updateColorComponents(); } @Override public boolean isReadOnly() { - return super.isReadOnly(); + return readOnly; + } + + private void updateColorComponents() { + if (getContent() != null) { + updateColorComponents(getContent()); + } + } + + private void updateColorComponents(Component component) { + if (component instanceof HasValue) { + ((HasValue) component).setReadOnly(isReadOnly()); + ((HasValue) component) + .setRequiredIndicatorVisible(isRequiredIndicatorVisible()); + } + if (component instanceof HasComponents) { + Iterator iterator = ((HasComponents) component) + .iterator(); + while (iterator.hasNext()) { + updateColorComponents(iterator.next()); + } + } } } diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java index 9c4ee78069..0c99779658 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java @@ -15,6 +15,7 @@ */ package com.vaadin.ui.components.colorpicker; +import java.util.Iterator; import java.util.Objects; import com.vaadin.data.HasValue; @@ -22,6 +23,7 @@ import com.vaadin.shared.Registration; import com.vaadin.shared.ui.colorpicker.Color; import com.vaadin.ui.Component; import com.vaadin.ui.CssLayout; +import com.vaadin.ui.HasComponents; import com.vaadin.ui.TextField; /** @@ -44,6 +46,8 @@ public class ColorPickerPreview extends CssLayout implements HasValue { private String oldValue; private Registration valueChangeListenerRegistration = null; + private boolean readOnly; + private ColorPickerPreview() { setStyleName("v-colorpicker-preview"); field = new TextField(); @@ -187,21 +191,39 @@ public class ColorPickerPreview extends CssLayout implements HasValue { @Override public void setRequiredIndicatorVisible(boolean visible) { - super.setRequiredIndicatorVisible(visible); + field.setRequiredIndicatorVisible(visible); } @Override public boolean isRequiredIndicatorVisible() { - return super.isRequiredIndicatorVisible(); + return field.isRequiredIndicatorVisible(); } @Override public void setReadOnly(boolean readOnly) { - super.setReadOnly(readOnly); + this.readOnly = readOnly; + updateColorComponents(); } @Override public boolean isReadOnly() { - return super.isReadOnly(); + return readOnly; + } + + private void updateColorComponents() { + iterator().forEachRemaining(this::updateColorComponents); + } + + private void updateColorComponents(Component component) { + if (component instanceof HasValue) { + ((HasValue) component).setReadOnly(isReadOnly()); + } + if (component instanceof HasComponents) { + Iterator iterator = ((HasComponents) component) + .iterator(); + while (iterator.hasNext()) { + updateColorComponents(iterator.next()); + } + } } } diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerSelect.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerSelect.java index f762aae827..9ee87dc0c8 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerSelect.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerSelect.java @@ -182,8 +182,21 @@ public class ColorPickerSelect extends CustomField { } } + /** + * Returns the selected value. + *

+ * Value can be {@code null} if component is not yet initialized via + * {@link #initContent()} + * + * @see ColorPickerSelect#initContent() + * + * @return the selected color, may be {@code null} + */ @Override public Color getValue() { + if (grid == null) { + return null; + } return grid.getValue(); } diff --git a/server/src/main/java/com/vaadin/ui/declarative/DesignContext.java b/server/src/main/java/com/vaadin/ui/declarative/DesignContext.java index b72edc4c1a..28004104cb 100644 --- a/server/src/main/java/com/vaadin/ui/declarative/DesignContext.java +++ b/server/src/main/java/com/vaadin/ui/declarative/DesignContext.java @@ -33,6 +33,7 @@ import com.vaadin.annotations.DesignRoot; import com.vaadin.server.Constants; import com.vaadin.server.DeploymentConfiguration; import com.vaadin.server.VaadinService; +import com.vaadin.shared.Registration; import com.vaadin.ui.Component; import com.vaadin.ui.HasComponents; import com.vaadin.ui.declarative.Design.ComponentFactory; @@ -628,10 +629,12 @@ public class DesignContext implements Serializable { * * @param listener * the component creation listener to be added + * @return a registration object for removing the listener */ - public void addComponentCreationListener( + public Registration addComponentCreationListener( ComponentCreationListener listener) { listeners.add(listener); + return () -> listeners.remove(listener); } /** @@ -639,7 +642,11 @@ public class DesignContext implements Serializable { * * @param listener * the component creation listener to be removed + * @deprecated Use a {@link Registration} object returned by + * {@link #addComponentCreationListener(ComponentCreationListener)} + * a listener */ + @Deprecated public void removeComponentCreationListener( ComponentCreationListener listener) { listeners.remove(listener); diff --git a/server/src/test/java/com/vaadin/server/RemoveListenersDeprecatedTest.java b/server/src/test/java/com/vaadin/server/RemoveListenersDeprecatedTest.java index 3ca5092ac5..e9c97d1b80 100644 --- a/server/src/test/java/com/vaadin/server/RemoveListenersDeprecatedTest.java +++ b/server/src/test/java/com/vaadin/server/RemoveListenersDeprecatedTest.java @@ -1,29 +1,97 @@ package com.vaadin.server; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Predicate; import java.util.regex.Pattern; import org.junit.Assert; import org.junit.Test; +import com.vaadin.event.EventRouter; +import com.vaadin.event.MethodEventSource; +import com.vaadin.server.data.AbstractDataProvider; +import com.vaadin.shared.Registration; import com.vaadin.tests.VaadinClasses; public class RemoveListenersDeprecatedTest { + private static final List> ALLOW_REMOVE_LISTENER = new ArrayList<>(); + + static { + ALLOW_REMOVE_LISTENER.add( + RemoveListenersDeprecatedTest::acceptAbstarctClientConnectorRemoveMethods); + ALLOW_REMOVE_LISTENER + .add(RemoveListenersDeprecatedTest::acceptAbstractDataProvider); + ALLOW_REMOVE_LISTENER + .add(RemoveListenersDeprecatedTest::acceptMethodEventSource); + } + @Test public void allRemoveListenerMethodsMarkedAsDeprecated() { - Pattern methodPattern = Pattern.compile("remove.*Listener"); + Pattern removePattern = Pattern.compile("remove.*Listener"); + Pattern addPattern = Pattern.compile("add.*Listener"); + int count = 0; for (Class serverClass : VaadinClasses - .getComponents()) { - for (Method method : serverClass.getMethods()) { - if (methodPattern.matcher(method.getName()).matches()) { + .getAllServerSideClasses()) { + count++; + if (serverClass.equals(EventRouter.class)) { + continue; + } + for (Method method : serverClass.getDeclaredMethods()) { + if (Modifier.isPrivate(method.getModifiers())) { + continue; + } + if (addPattern.matcher(method.getName()).matches() + && method.getAnnotation(Deprecated.class) == null) { + Class returnType = method.getReturnType(); + Assert.assertEquals( + "Method " + method.getName() + + " is not deprectated in class " + + serverClass.getName() + + " and doesn't return a Registration object", + Registration.class, returnType); + } + if (ALLOW_REMOVE_LISTENER.stream() + .anyMatch(predicate -> predicate.test(method))) { + continue; + } + + if (removePattern.matcher(method.getName()).matches()) { Assert.assertNotNull( "Method " + method.getName() + " in class " - + serverClass.getSimpleName() + + serverClass.getName() + " has not been marked as deprecated.", method.getAnnotation(Deprecated.class)); } } } + Assert.assertTrue(count > 0); + } + + private static boolean acceptMethodEventSource(Method method) { + return method.getDeclaringClass().equals(MethodEventSource.class) + && method.getParameterCount() == 2; + } + + private static boolean acceptAbstarctClientConnectorRemoveMethods( + Method method) { + if (method.getDeclaringClass().equals(AbstractClientConnector.class)) { + if (method.getParameterCount() == 2) { + return true; + } else if (method.getParameterCount() == 0) { + return false; + } else { + return method.getParameterTypes()[0].equals(String.class); + } + } + return false; + } + + private static boolean acceptAbstractDataProvider(Method method) { + return method.getDeclaringClass().equals(AbstractDataProvider.class) + && method.getParameterCount() == 2; } } diff --git a/server/src/test/java/com/vaadin/tests/VaadinClasses.java b/server/src/test/java/com/vaadin/tests/VaadinClasses.java index 3e3c641de8..287222be48 100644 --- a/server/src/test/java/com/vaadin/tests/VaadinClasses.java +++ b/server/src/test/java/com/vaadin/tests/VaadinClasses.java @@ -1,23 +1,13 @@ package com.vaadin.tests; import java.io.File; -import java.io.IOException; -import java.lang.reflect.Method; -import java.lang.reflect.Modifier; -import java.net.JarURLConnection; import java.net.URISyntaxException; -import java.net.URL; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; -import java.util.Enumeration; import java.util.List; -import java.util.jar.JarEntry; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; -import org.junit.Test; - -import com.vaadin.server.VaadinSession; +import com.vaadin.tests.server.ClasspathHelper; import com.vaadin.ui.Component; import com.vaadin.ui.ComponentContainer; import com.vaadin.ui.CustomComponent; @@ -53,40 +43,26 @@ public class VaadinClasses { } public static List> getComponents() { - try { - return findClasses(Component.class, "com.vaadin.ui"); - } catch (IOException e) { - e.printStackTrace(); - return null; - } + return getServerClasses(Component.class::isAssignableFrom) + .map(VaadinClasses::castComponentClass) + .collect(Collectors.toList()); } public static List> getThemeClasses() { - try { - return findClasses(Object.class, "com.vaadin.ui.themes"); - } catch (IOException e) { - e.printStackTrace(); - return null; - } + return getServerClasses(clazz -> clazz.getPackage().getName() + .equals("com.vaadin.ui.themes")).collect(Collectors.toList()); } public static List> getAllServerSideClasses() { - try { - return findClassesNoTests(Object.class, "com.vaadin", - new String[] { "com.vaadin.tests", "com.vaadin.client" }); - } catch (IOException e) { - e.printStackTrace(); - return null; - } + return getServerClasses(clazz -> true).collect(Collectors.toList()); } public static List> getComponentContainers() { - try { - return findClasses(ComponentContainer.class, "com.vaadin.ui"); - } catch (IOException e) { - e.printStackTrace(); - return null; - } + return getServerClasses(ComponentContainer.class::isAssignableFrom) + .filter(clazz -> clazz.getPackage().getName() + .startsWith("com.vaadin.ui")) + .map(VaadinClasses::castContainerClass) + .collect(Collectors.toList()); } public static List> getComponentContainersSupportingAddRemoveComponent() { @@ -110,133 +86,28 @@ public class VaadinClasses { return classes; } - @SuppressWarnings({ "unchecked", "rawtypes" }) - public static List> getBasicComponentTests() { + public static Stream> getServerClasses( + Predicate> predicate) { try { - // Given as name to avoid dependencies on testbench source folder - return (List) findClasses( - Class.forName( - "com.vaadin.tests.components.AbstractComponentTest"), - "com.vaadin.tests.components"); - } catch (Exception e) { - e.printStackTrace(); - return null; + File testRoot = new File( + VaadinClasses.class.getResource("/").toURI()); + ClasspathHelper helper = new ClasspathHelper(); + return helper + .getVaadinClassesFromClasspath( + entry -> !testRoot.equals(new File(entry))) + .filter(predicate); + } catch (URISyntaxException e) { + throw new RuntimeException(e); } - - } - - public static List> findClasses(Class baseClass, - String basePackage) throws IOException { - return findClasses(baseClass, basePackage, new String[] {}); } - private static List> findClasses(Class baseClass, - String basePackage, String[] ignoredPackages) throws IOException { - List> classes = new ArrayList<>(); - String basePackageDirName = "/" + basePackage.replace('.', '/'); - URL location = VaadinSession.class.getResource(basePackageDirName); - if (location.getProtocol().equals("file")) { - try { - File f = new File(location.toURI()); - if (!f.exists()) { - throw new IOException( - "Directory " + f.toString() + " does not exist"); - } - findPackages(f, basePackage, baseClass, classes, - ignoredPackages); - } catch (URISyntaxException e) { - throw new IOException(e.getMessage()); - } - } else if (location.getProtocol().equals("jar")) { - JarURLConnection juc = (JarURLConnection) location.openConnection(); - findPackages(juc, basePackage, baseClass, classes); - } - - Collections.sort(classes, (Class o1, Class o2) -> o1.getName().compareTo(o2.getName())); - return classes; + private static Class castComponentClass( + Class clazz) { + return (Class) clazz; } - private static List> findClassesNoTests( - Class baseClass, String basePackage, String[] ignoredPackages) - throws IOException { - List> classes = findClasses(baseClass, basePackage, - ignoredPackages); - List> classesNoTests = new ArrayList<>(); - for (Class clazz : classes) { - if (!clazz.getName().contains("Test")) { - boolean testPresent = false; - for (Method method : clazz.getMethods()) { - if (method.isAnnotationPresent(Test.class)) { - testPresent = true; - break; - } - } - if (!testPresent) { - classesNoTests.add(clazz); - } - } - } - return classesNoTests; - } - - private static void findPackages(JarURLConnection juc, - String javaPackage, Class baseClass, - Collection> result) throws IOException { - String prefix = "com/vaadin/ui"; - Enumeration ent = juc.getJarFile().entries(); - while (ent.hasMoreElements()) { - JarEntry e = ent.nextElement(); - if (e.getName().endsWith(".class") - && e.getName().startsWith(prefix)) { - String fullyQualifiedClassName = e.getName().replace('/', '.') - .replace(".class", ""); - addClassIfMatches(result, fullyQualifiedClassName, baseClass); - } - } - } - - private static void findPackages(File parent, String javaPackage, - Class baseClass, Collection> result, - String[] ignoredPackages) { - for (String ignoredPackage : ignoredPackages) { - if (javaPackage.equals(ignoredPackage)) { - return; - } - } - - for (File file : parent.listFiles()) { - if (file.isDirectory()) { - findPackages(file, javaPackage + "." + file.getName(), - baseClass, result, ignoredPackages); - } else if (file.getName().endsWith(".class")) { - String fullyQualifiedClassName = javaPackage + "." - + file.getName().replace(".class", ""); - addClassIfMatches(result, fullyQualifiedClassName, baseClass); - } - } - - } - - @SuppressWarnings("unchecked") - private static void addClassIfMatches( - Collection> result, - String fullyQualifiedClassName, Class baseClass) { - try { - // Try to load the class - - Class c = Class.forName(fullyQualifiedClassName); - if (baseClass.isAssignableFrom(c) - && !Modifier.isAbstract(c.getModifiers()) - && !c.isAnonymousClass() && !c.isMemberClass() - && !c.isLocalClass()) { - result.add((Class) c); - } - } catch (Exception e) { - // Could ignore that class cannot be loaded - e.printStackTrace(); - } catch (LinkageError e) { - // Ignore. Client side classes will at least throw LinkageErrors - } - + private static Class castContainerClass( + Class clazz) { + return (Class) clazz; } } diff --git a/server/src/test/java/com/vaadin/tests/server/ClasspathHelper.java b/server/src/test/java/com/vaadin/tests/server/ClasspathHelper.java new file mode 100644 index 0000000000..8dfe96d1c3 --- /dev/null +++ b/server/src/test/java/com/vaadin/tests/server/ClasspathHelper.java @@ -0,0 +1,137 @@ +package com.vaadin.tests.server; + +import java.io.File; +import java.io.IOException; +import java.lang.reflect.Modifier; +import java.net.URI; +import java.nio.file.FileSystem; +import java.nio.file.FileSystemNotFoundException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * Allows to get classes from the current classpath using classes FQN filter. + *

+ * The methods in the class return all real (not anonymous and not private) + * classes from the filtered classpath. + * + * @author Vaadin Ltd + * + */ +public class ClasspathHelper { + + public static final String COM_VAADIN_FILE_PREFIX = "com" + + File.separatorChar + "vaadin" + File.separatorChar; + private final Predicate skipClassesFilter; + + public ClasspathHelper(Predicate skipClassesFilter) { + this.skipClassesFilter = skipClassesFilter; + } + + public ClasspathHelper() { + this(fqn -> false); + } + + public Stream> getVaadinClassesFromClasspath( + Predicate classpathFilter, + Predicate> classFilter) { + return getRawClasspathEntries().stream().filter(classpathFilter) + .map(File::new).map(file -> getVaadinClassesFromFile(file)) + .flatMap(List::stream).filter(classFilter) + .filter(cls -> !cls.isSynthetic() && !cls.isAnonymousClass() + && !Modifier.isPrivate(cls.getModifiers())); + + } + + public Stream> getVaadinClassesFromClasspath( + Predicate classpathFilter) { + return getVaadinClassesFromClasspath(classpathFilter, cls -> true); + } + + private List> getVaadinClassesFromFile(File classesRoot) { + try { + if (classesRoot.isDirectory()) { + return Files.walk(classesRoot.toPath()) + .filter(Files::isRegularFile) + .filter(path -> path.toFile().getName() + .endsWith(".class")) + .filter(path -> classesRoot.toPath().relativize(path) + .toString().contains(COM_VAADIN_FILE_PREFIX)) + .map(path -> getClassFromFile(path, + classesRoot.toPath())) + .filter(Objects::nonNull).collect(Collectors.toList()); + } else if (classesRoot.getName().toLowerCase(Locale.ENGLISH) + .endsWith(".jar")) { + URI uri = URI.create("jar:file:" + classesRoot.getPath()); + FileSystem fileSystem; + try { + fileSystem = FileSystems.getFileSystem(uri); + } catch (FileSystemNotFoundException e) { + fileSystem = null; + } + if (fileSystem == null) { + fileSystem = FileSystems.newFileSystem(uri, + Collections.emptyMap()); + } + Path root = fileSystem.getPath(File.separator); + return Files.walk(root).filter(Files::isRegularFile) + .filter(path -> path.toUri().getSchemeSpecificPart() + .endsWith(".class")) + .filter(path -> root.relativize(path).toString() + .contains(COM_VAADIN_FILE_PREFIX)) + .map(path -> getClassFromFile(path, root)) + .filter(Objects::nonNull).collect(Collectors.toList()); + } + return null; + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private Class getClassFromFile(Path path, Path root) { + Path relative = root.relativize(path); + String name = relative.toString(); + name = name.substring(0, name.length() - ".class".length()); + name = name.replace(File.separatorChar, '.'); + if (skipClassesFilter.test(name)) { + return null; + } + try { + return Class.forName(name, false, getClass().getClassLoader()); + } catch (ClassNotFoundException e) { + throw new RuntimeException(e); + } + } + + private final static List getRawClasspathEntries() { + List locations = new ArrayList<>(); + + String pathSep = System.getProperty("path.separator"); + String classpath = System.getProperty("java.class.path"); + + if (classpath.startsWith("\"")) { + classpath = classpath.substring(1); + } + if (classpath.endsWith("\"")) { + classpath = classpath.substring(0, classpath.length() - 1); + } + + String[] split = classpath.split(pathSep); + for (int i = 0; i < split.length; i++) { + String classpathEntry = split[i]; + locations.add(classpathEntry); + } + + return locations; + } + +} diff --git a/server/src/test/java/com/vaadin/tests/server/component/FinalMethodTest.java b/server/src/test/java/com/vaadin/tests/server/component/FinalMethodTest.java index 5519163012..71d423808e 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/FinalMethodTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/FinalMethodTest.java @@ -4,6 +4,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.HashSet; +import org.junit.Assert; import org.junit.Test; import com.vaadin.tests.VaadinClasses; @@ -23,35 +24,39 @@ public class FinalMethodTest { @Test public void testThatComponentsHaveNoFinalMethods() { HashSet> tested = new HashSet<>(); + int count = 0; for (Class c : VaadinClasses.getComponents()) { ensureNoFinalMethods(c, tested); + count++; } + Assert.assertTrue(count > 0); } - private void ensureNoFinalMethods(Class c, HashSet> tested) { - if (tested.contains(c)) { + private void ensureNoFinalMethods(Class clazz, + HashSet> tested) { + if (tested.contains(clazz)) { return; } - tested.add(c); + tested.add(clazz); - if (c == Object.class) { + if (clazz == null || clazz.equals(Object.class)) { return; } - System.out.println("Checking " + c.getName()); - for (Method m : c.getDeclaredMethods()) { + System.out.println("Checking " + clazz.getName()); + for (Method m : clazz.getDeclaredMethods()) { if (isPrivate(m)) { continue; } if (isFinal(m)) { - String error = "Class " + c.getName() + " contains a " + String error = "Class " + clazz.getName() + " contains a " + (isPublic(m) ? "public" : "non-public") + " final method: " + m.getName(); // System.err.println(error); throw new RuntimeException(error); } } - ensureNoFinalMethods(c.getSuperclass(), tested); + ensureNoFinalMethods(clazz.getSuperclass(), tested); } diff --git a/server/src/test/java/com/vaadin/tests/server/component/StateGetDoesNotMarkDirtyTest.java b/server/src/test/java/com/vaadin/tests/server/component/StateGetDoesNotMarkDirtyTest.java index b6392f0910..87986c1b5c 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/StateGetDoesNotMarkDirtyTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/StateGetDoesNotMarkDirtyTest.java @@ -1,16 +1,20 @@ package com.vaadin.tests.server.component; import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.Arrays; import java.util.HashSet; import java.util.Locale; import java.util.Set; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; +import com.vaadin.server.VaadinSession; import com.vaadin.tests.VaadinClasses; import com.vaadin.ui.Component; import com.vaadin.ui.ConnectorTracker; @@ -25,17 +29,27 @@ public class StateGetDoesNotMarkDirtyTest { public void setUp() { excludedMethods.add(Label.class.getName() + "getDataProviderValue"); excludedMethods.add("getConnectorId"); + excludedMethods.add("getContent"); } @Test public void testGetDoesntMarkStateDirty() throws Exception { - for (Class c : VaadinClasses.getComponents()) { - Component newInstance = construct(c); + int count = 0; + for (Class clazz : VaadinClasses.getComponents()) { + if (clazz.isInterface() + || Modifier.isAbstract(clazz.getModifiers())) { + continue; + } + Component newInstance = construct(clazz); + if (newInstance == null) { + continue; + } + count++; prepareMockUI(newInstance); Set methods = new HashSet<>(); - methods.addAll(Arrays.asList(c.getMethods())); - methods.addAll(Arrays.asList(c.getDeclaredMethods())); + methods.addAll(Arrays.asList(clazz.getMethods())); + methods.addAll(Arrays.asList(clazz.getDeclaredMethods())); for (Method method : methods) { try { if (method.getName().startsWith("is") @@ -49,7 +63,7 @@ public class StateGetDoesNotMarkDirtyTest { continue; } if (excludedMethods - .contains(c.getName() + method.getName())) { + .contains(clazz.getName() + method.getName())) { // blacklisted method for specific classes continue; } @@ -62,40 +76,63 @@ public class StateGetDoesNotMarkDirtyTest { method.invoke(newInstance); } } catch (Exception e) { - System.err.println("problem with method " + c.getName() + System.err.println("problem with method " + clazz.getName() + "# " + method.getName()); e.printStackTrace(); throw e; } } } - + Assert.assertTrue(count > 0); } private void prepareMockUI(Component newInstance) { + UI ui = mockUI(); + ConnectorTracker connectorTracker = ui.getConnectorTracker(); + Mockito.doThrow(new RuntimeException("getState(true) called in getter")) + .when(connectorTracker).markDirty(newInstance); + newInstance.setParent(null); + newInstance.setParent(ui); + } + + private UI mockUI() { UI ui = Mockito.mock(UI.class); Mockito.when(ui.getLocale()).thenReturn(Locale.ENGLISH); ConnectorTracker connectorTracker = Mockito .mock(ConnectorTracker.class); Mockito.when(ui.getConnectorTracker()).thenReturn(connectorTracker); - Mockito.doThrow(new RuntimeException("getState(true) called in getter")) - .when(connectorTracker).markDirty(newInstance); - - newInstance.setParent(ui); + return ui; } - private Component construct(Class c) { + private Component construct(Class clazz) { try { - try { - Constructor declaredConstructor = c - .getDeclaredConstructor(); - declaredConstructor.setAccessible(true); - return declaredConstructor.newInstance(); - } catch (NoSuchMethodException e) { - return c.newInstance(); + Constructor declaredConstructor = clazz + .getDeclaredConstructor(); + declaredConstructor.setAccessible(true); + Component component = declaredConstructor.newInstance(); + + if (component instanceof UI) { + return component; } - } catch (Exception e) { + emulateAttach(component); + return component; + } catch (NoSuchMethodException e) { + // no default CTOR, skip + return null; + } catch (InstantiationException | IllegalAccessException + | IllegalArgumentException | InvocationTargetException e) { throw new RuntimeException(e); } } + + private void emulateAttach(Component component) { + UI ui = mockUI(); + VaadinSession session = Mockito.mock(VaadinSession.class); + Mockito.when(session.hasLock()).thenReturn(true); + Mockito.when(ui.getSession()).thenReturn(session); + component.setParent(ui); + + component.attach(); + } + } diff --git a/server/src/test/java/com/vaadin/tests/server/componentcontainer/AddRemoveComponentTest.java b/server/src/test/java/com/vaadin/tests/server/componentcontainer/AddRemoveComponentTest.java index 4662f6f84b..e5537992c0 100644 --- a/server/src/test/java/com/vaadin/tests/server/componentcontainer/AddRemoveComponentTest.java +++ b/server/src/test/java/com/vaadin/tests/server/componentcontainer/AddRemoveComponentTest.java @@ -1,5 +1,8 @@ package com.vaadin.tests.server.componentcontainer; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Modifier; import java.util.List; import org.junit.Assert; @@ -15,16 +18,30 @@ public class AddRemoveComponentTest { @Test public void testRemoveComponentFromWrongContainer() - throws InstantiationException, IllegalAccessException { + throws InstantiationException, IllegalAccessException, + IllegalArgumentException, InvocationTargetException { List> containerClasses = VaadinClasses .getComponentContainersSupportingAddRemoveComponent(); + Assert.assertTrue(containerClasses.size() > 0); + // No default constructor, special case containerClasses.remove(CustomLayout.class); testRemoveComponentFromWrongContainer(new CustomLayout("dummy")); - for (Class c : containerClasses) { - testRemoveComponentFromWrongContainer(c.newInstance()); + for (Class clazz : containerClasses) { + if (Modifier.isAbstract(clazz.getModifiers())) { + continue; + } + try { + Constructor constructor = clazz + .getConstructor(); + constructor.setAccessible(true); + testRemoveComponentFromWrongContainer( + constructor.newInstance()); + } catch (NoSuchMethodException ignore) { + // if there is no default CTOR, just ignore + } } } diff --git a/shared/src/main/java/com/vaadin/shared/ui/ComponentStateUtil.java b/shared/src/main/java/com/vaadin/shared/ui/ComponentStateUtil.java index a38b828471..bcc4b3f496 100644 --- a/shared/src/main/java/com/vaadin/shared/ui/ComponentStateUtil.java +++ b/shared/src/main/java/com/vaadin/shared/ui/ComponentStateUtil.java @@ -19,6 +19,7 @@ import java.io.Serializable; import java.util.HashSet; import com.vaadin.shared.AbstractComponentState; +import com.vaadin.shared.Registration; import com.vaadin.shared.communication.SharedState; public final class ComponentStateUtil implements Serializable { @@ -57,7 +58,11 @@ public final class ComponentStateUtil implements Serializable { * * @param eventListenerId * The event identifier to remove + * @deprecated Use a {@link Registration} object returned by + * {@link #addRegisteredEventListener(SharedState, String)} to + * remove a listener */ + @Deprecated public static final void removeRegisteredEventListener(SharedState state, String eventIdentifier) { if (state.registeredEventListeners == null) { @@ -74,12 +79,14 @@ public final class ComponentStateUtil implements Serializable { * * @param eventListenerId * The event identifier to add + * @return a registration object for removing the listener */ - public static final void addRegisteredEventListener(SharedState state, - String eventListenerId) { + public static final Registration addRegisteredEventListener( + SharedState state, String eventListenerId) { if (state.registeredEventListeners == null) { state.registeredEventListeners = new HashSet<>(); } state.registeredEventListeners.add(eventListenerId); + return () -> removeRegisteredEventListener(state, eventListenerId); } } diff --git a/uitest/src/main/java/com/vaadin/tests/VaadinClasses.java b/uitest/src/main/java/com/vaadin/tests/VaadinClasses.java index bee83d8e01..f5af00234c 100644 --- a/uitest/src/main/java/com/vaadin/tests/VaadinClasses.java +++ b/uitest/src/main/java/com/vaadin/tests/VaadinClasses.java @@ -68,21 +68,6 @@ public class VaadinClasses { return classes; } - @SuppressWarnings({ "unchecked", "rawtypes" }) - public static List> getBasicComponentTests() { - try { - // Given as name to avoid dependencies on testbench source folder - return (List) findClasses( - Class.forName( - "com.vaadin.tests.components.AbstractComponentTest"), - "com.vaadin.tests.components"); - } catch (Exception e) { - e.printStackTrace(); - return null; - } - - } - private static List> findClasses(Class baseClass, String basePackage) throws IOException { return findClasses(baseClass, basePackage, new String[] {}); -- cgit v1.2.3