Vojtech Szocs has uploaded a new change for review.
Change subject: core: Add GWT RPC serializable check
......................................................................
core: Add GWT RPC serializable check
This patch adds GwtRpcSerializableCheck to ensure that common
classes which participate in GWT RPC calls meet standard rules
for Serializable User-defined Classes [*]:
* is assignable to Serializable or IsSerializable interface,
either because it directly implements one of these interfaces
or because it derives from a superclass that does
* all non-final, non-transient instance fields are themselves
serializable
* has a default (zero argument) constructor (with any access
modifier) or no constructor at all
[*] http://www.gwtproject.org/doc/latest/
DevGuideServerCommunication.html#DevGuideSerializableTypes
GwtRpcSerializableCheck applies to 'common' module and checks
all classes assignable to following root types:
* VdcQueryParametersBase [all query parameters]
* VdcActionParametersBase [all action parameters]
* VdcUser [used in GenericApiGWTService.logOff method]
* BusinessEntity [all business entities]
GwtRpcSerializableCheck uses Java Reflection API instead of
Checkstyle's AST Token API. This is because of advanced class
introspection (i.e. check if given class implements interface
X either directly or indirectly) required by this check.
GwtRpcSerializableCheck makes best effort not to load/check
given class more than once.
Change-Id: I51c87efc432444226daa7a18be047435d21b84ef
Signed-off-by: Vojtech Szocs <vsz...@redhat.com>
---
M backend/manager/modules/common/pom.xml
M build-tools-root/checkstyles/src/main/resources/checkstyle.xml
A
build-tools-root/ovirt-checkstyle-extension/src/main/java/checks/GwtRpcSerializableCheck.java
3 files changed, 190 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/10/18910/1
diff --git a/backend/manager/modules/common/pom.xml
b/backend/manager/modules/common/pom.xml
index a184fa2..d94be7a 100644
--- a/backend/manager/modules/common/pom.xml
+++ b/backend/manager/modules/common/pom.xml
@@ -68,6 +68,7 @@
<configuration>
<propertyExpansion>disallowFinals=true</propertyExpansion>
<propertyExpansion>disallowMemberInit=true</propertyExpansion>
+
<propertyExpansion>runGwtRpcSerializableCheck=true</propertyExpansion>
</configuration>
</plugin>
<plugin>
diff --git a/build-tools-root/checkstyles/src/main/resources/checkstyle.xml
b/build-tools-root/checkstyles/src/main/resources/checkstyle.xml
index 77fc4a6..09831fd 100644
--- a/build-tools-root/checkstyles/src/main/resources/checkstyle.xml
+++ b/build-tools-root/checkstyles/src/main/resources/checkstyle.xml
@@ -28,5 +28,8 @@
<module name="checks.NoMemberInitializationCheck">
<property name="run" value="${disallowMemberInit}" default="false"/>
</module>
+ <module name="checks.GwtRpcSerializableCheck">
+ <property name="run" value="${runGwtRpcSerializableCheck}"
default="false"/>
+ </module>
</module>
</module>
diff --git
a/build-tools-root/ovirt-checkstyle-extension/src/main/java/checks/GwtRpcSerializableCheck.java
b/build-tools-root/ovirt-checkstyle-extension/src/main/java/checks/GwtRpcSerializableCheck.java
new file mode 100644
index 0000000..8d7e6f4
--- /dev/null
+++
b/build-tools-root/ovirt-checkstyle-extension/src/main/java/checks/GwtRpcSerializableCheck.java
@@ -0,0 +1,186 @@
+package checks;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+import com.puppycrawl.tools.checkstyle.checks.AbstractTypeAwareCheck;
+
+/**
+ * Checks that classes meet GWT RPC rules for <em>Serializable User-defined
Classes</em>:
+ * <ul>
+ * <li>is assignable to Serializable or IsSerializable interface, either
because it directly implements one of these
+ * interfaces or because it derives from a superclass that does</li>
+ * <li>all non-final, non-transient instance fields are themselves
serializable</li>
+ * <li>has a default (zero argument) constructor (with any access modifier) or
no constructor at all</li>
+ * </ul>
+ */
+public class GwtRpcSerializableCheck extends AbstractTypeAwareCheck {
+
+ /**
+ * Root types whose subclasses should be checked:
+ * <ul>
+ * <li>type is used in GWT RPC service method signature</li>
+ * <li>type is used at runtime (cannot be easily determined through static
analysis)</li>
+ * </ul>
+ */
+ private static final String[] RPC_SERIALIZABLE_ROOTS = {
+ "org.ovirt.engine.core.common.queries.VdcQueryParametersBase",
+ "org.ovirt.engine.core.common.action.VdcActionParametersBase",
+ "org.ovirt.engine.core.common.users.VdcUser",
+ "org.ovirt.engine.core.common.businessentities.BusinessEntity"
+ };
+
+ /**
+ * Types to exclude from checking in addition to <code>java.lang.*</code>
classes:
+ * <ul>
+ * <li>type serialization is provided by GWT RPC out of the box</li>
+ * <li>type serialization is provided by custom field serializer</li>
+ * </ul>
+ */
+ private static final String[] RPC_SERIALIZABLE_EXCLUDES = {
+ "java.util.Date", "java.math.BigDecimal", "java.math.BigInteger",
+ "java.util.UUID"
+ };
+
+ public static final String JAVA_SERIALIZABLE = "java.io.Serializable";
+ public static final String GWT_ISSERIALIZABLE =
"com.google.gwt.user.client.rpc.IsSerializable";
+
+ private final Map<String, Class<?>> typeCache = new HashMap<String,
Class<?>>();
+ private final Set<String> typesChecked = new HashSet<String>();
+
+ private boolean run = true;
+
+ public void setRun(boolean run) {
+ this.run = run;
+ }
+
+ @Override
+ public int[] getDefaultTokens() {
+ return new int[] {
+ // Processed by AbstractTypeAwareCheck
+ TokenTypes.PACKAGE_DEF, TokenTypes.IMPORT,
+ TokenTypes.CLASS_DEF, TokenTypes.ENUM_DEF,
+ // Processed by this check
+ TokenTypes.OBJBLOCK
+ };
+ }
+
+ @Override
+ public int[] getAcceptableTokens() {
+ return new int[] {
+ TokenTypes.OBJBLOCK
+ };
+ }
+
+ @Override
+ protected void processAST(DetailAST ast) {
+ // Expect object block enclosed by class declaration
+ if (!run || ast.getType() != TokenTypes.OBJBLOCK ||
ast.getParent().getType() != TokenTypes.CLASS_DEF) {
+ return;
+ }
+
+ // Check class enclosing current object block
+ Class<?> clazz = resolveClass(getCurrentClassName());
+ if (clazz != null && isUserDefinedClass(clazz) &&
isRootTypeSubclass(clazz)) {
+ performCheck(clazz);
+ }
+ }
+
+ @Override
+ protected void logLoadError(Token ident) {
+ logLoadErrorImpl(ident.getLineNo(), ident.getColumnNo(),
+ "Cannot load class {0}", ident.getText());
+ }
+
+ Class<?> resolveClass(String name) {
+ if (!typeCache.containsKey(name)) {
+ Class<?> clazz = resolveClass(name, "");
+ typeCache.put(name, clazz);
+ }
+ return typeCache.get(name);
+ }
+
+ boolean isJavaLangClass(Class<?> clazz) {
+ return clazz.getPackage().getName().startsWith("java.lang");
+ }
+
+ boolean isExcludedClass(Class<?> clazz) {
+ for (String exclude : RPC_SERIALIZABLE_EXCLUDES) {
+ if (clazz.getName().equals(exclude)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ boolean isUserDefinedClass(Class<?> clazz) {
+ return !clazz.isPrimitive() && !clazz.isArray() && !clazz.isEnum() &&
!clazz.isAnnotation()
+ && !clazz.isInterface() && !isJavaLangClass(clazz) &&
!isExcludedClass(clazz);
+ }
+
+ boolean isRootTypeSubclass(Class<?> clazz) {
+ for (String root : RPC_SERIALIZABLE_ROOTS) {
+ if (isSubclass(clazz, resolveClass(root))) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ void performCheck(Class<?> clazz) {
+ // Skip class if already checked
+ if (typesChecked.contains(clazz.getName())) {
+ return;
+ }
+
+ // 1. is assignable to Serializable or IsSerializable interface,
either because it directly
+ // implements one of these interfaces or because it derives from a
superclass that does
+ if (!isSubclass(clazz, resolveClass(JAVA_SERIALIZABLE))
+ && !isSubclass(clazz, resolveClass(GWT_ISSERIALIZABLE))) {
+ log(0, "Class {0} must be assignable to " + JAVA_SERIALIZABLE
+ + " or " + GWT_ISSERIALIZABLE + " interface",
clazz.getName());
+ }
+
+ // 2. all non-final, non-transient instance fields are themselves
serializable
+ for (Field field : clazz.getDeclaredFields()) {
+ int mods = field.getModifiers();
+ if (!Modifier.isStatic(mods) && !Modifier.isFinal(mods) &&
!Modifier.isTransient(mods)) {
+ Class<?> fieldType = field.getType();
+ if (isUserDefinedClass(fieldType)) {
+ // Check field's declared type
+ performCheck(fieldType);
+ } else if (fieldType.isArray()) {
+ Class<?> componentType = fieldType.getComponentType();
+ if (isUserDefinedClass(componentType)) {
+ // Check field's array component type
+ performCheck(componentType);
+ }
+ }
+ }
+ }
+
+ // 3. has a default (zero argument) constructor (with any access
modifier) or no constructor at all
+ boolean foundDefaultConstructor = false;
+ for (Constructor<?> ctor : clazz.getDeclaredConstructors()) {
+ if (ctor.getParameterTypes().length == 0) {
+ foundDefaultConstructor = true;
+ break;
+ }
+ }
+ if (!foundDefaultConstructor) {
+ log(0, "Class {0} must have a default (zero argument) constructor"
+
+ " (with any access modifier) or no constructor at all",
clazz.getName());
+ }
+
+ // Mark class as checked
+ typesChecked.add(clazz.getName());
+ }
+
+}
--
To view, visit http://gerrit.ovirt.org/18910
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I51c87efc432444226daa7a18be047435d21b84ef
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
