You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aries.apache.org by dj...@apache.org on 2012/01/24 01:53:35 UTC
svn commit: r1235096 - in /aries/trunk/versioning/versioning-checker/src:
main/java/org/apache/aries/versioning/check/
main/java/org/apache/aries/versioning/utils/
test/java/org/apache/aries/versioning/tests/
Author: djencks
Date: Tue Jan 24 00:53:35 2012
New Revision: 1235096
URL: http://svn.apache.org/viewvc?rev=1235096&view=rev
Log:
ARIES-757 refactor ClassDeclaration to a simpler list of compatibility problems. Avoids dependence on HashSet implementation order
Modified:
aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/check/SemanticVersioningChecker.java
aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/BinaryCompatibilityStatus.java
aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/ClassDeclaration.java
aries/trunk/versioning/versioning-checker/src/test/java/org/apache/aries/versioning/tests/BinaryCompatibilityTest.java
Modified: aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/check/SemanticVersioningChecker.java
URL: http://svn.apache.org/viewvc/aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/check/SemanticVersioningChecker.java?rev=1235096&r1=1235095&r2=1235096&view=diff
==============================================================================
--- aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/check/SemanticVersioningChecker.java (original)
+++ aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/check/SemanticVersioningChecker.java Tue Jan 24 00:53:35 2012
@@ -25,6 +25,7 @@ import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
+import java.lang.String;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
@@ -57,7 +58,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static org.apache.aries.versioning.utils.SemanticVersioningUtils.htmlOneLineBreak;
-import static org.apache.aries.versioning.utils.SemanticVersioningUtils.htmlTwoLineBreaks;;
+import static org.apache.aries.versioning.utils.SemanticVersioningUtils.htmlTwoLineBreaks;
public class SemanticVersioningChecker {
@@ -96,8 +97,8 @@ public class SemanticVersioningChecker {
FileWriter versionStatusFileWriter = null;
try {
versionStatusFileWriter = new FileWriter(versionStatusFile, false);
- Map<String, BundleInfo> baseBundles = new HashMap<String, BundleInfo>();
- Map<String, BundleInfo> currentBundles = new HashMap<String, BundleInfo>();
+ Map<String, BundleInfo> baseBundles;
+ Map<String, BundleInfo> currentBundles;
//scan each individual bundle and find the corresponding bundle in the baseline and verify the version changes
currentBundles = getBundles(currentDir);
@@ -113,7 +114,7 @@ public class SemanticVersioningChecker {
writeRecordToWriter(versionStatusFileWriter, xmlHeader + "\r\n");
// write the comparison base and current level into the file
- writeRecordToWriter(versionStatusFileWriter, "<semanaticVersioning currentDir= \"" + currentDir + "\" baseDir = \"" + baseDir + "\">");
+ writeRecordToWriter(versionStatusFileWriter, "<semanticVersioning currentDir= \"" + currentDir + "\" baseDir = \"" + baseDir + "\">");
for (Map.Entry<String, BundleInfo> entry : currentBundles.entrySet()) {
String bundleSymbolicName = entry.getKey();
@@ -125,12 +126,12 @@ public class SemanticVersioningChecker {
StringBuilder pkgElements = new StringBuilder();
String reason = null;
if (baseBundle == null) {
- _logger.debug("The bundle " + bundleSymbolicName + " has no counterpart in the base. The semanit version validation does not apply to this bundle.");
+ _logger.debug("The bundle " + bundleSymbolicName + " has no counterpart in the base. The semantic version validation does not apply to this bundle.");
} else {
// open the manifest and scan the export package and find the package name and exported version
// The tool assume the one particular package just exports under one version
Map<String, PackageContent> currBundleExpPkgContents = getAllExportedPkgContents(currentBundle);
- Map<String, PackageContent> baseBundleExpPkgContents = new HashMap<String, PackageContent>();
+ Map<String, PackageContent> baseBundleExpPkgContents;
boolean pkg_major_change = false;
boolean pkg_minor_change = false;
String fatal_package = null;
@@ -220,7 +221,7 @@ public class SemanticVersioningChecker {
writeRecordToWriter(versionStatusFileWriter, "</bundle>");
}
}
- writeRecordToWriter(versionStatusFileWriter, "</semanaticVersioning>");
+ writeRecordToWriter(versionStatusFileWriter, "</semanticVersioning>");
} catch (IOException ioe) {
ioe.printStackTrace();
@@ -358,9 +359,11 @@ public class SemanticVersioningChecker {
BinaryCompatibilityStatus bcs = newcd.getBinaryCompatibleStatus(oldcv.getClassDeclaration());
if (!bcs.isCompatible()) {
- major_reason.append(htmlTwoLineBreaks + "In the " + getClassName(changeClass) + " class or its supers, the following changes have been made since the last release of WAS.");
+ major_reason.append(htmlTwoLineBreaks + "In the " + getClassName(changeClass) + " class or its supers, the following changes have been made since the last release.");
// break binary compatibility
- major_reason.append(bcs.getReason());
+ for (String reason: bcs) {
+ major_reason.append(htmlOneLineBreak).append(reason);
+ }
is_major_change = true;
fatal_class = changeClass;
} else {
Modified: aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/BinaryCompatibilityStatus.java
URL: http://svn.apache.org/viewvc/aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/BinaryCompatibilityStatus.java?rev=1235096&r1=1235095&r2=1235096&view=diff
==============================================================================
--- aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/BinaryCompatibilityStatus.java (original)
+++ aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/BinaryCompatibilityStatus.java Tue Jan 24 00:53:35 2012
@@ -18,44 +18,16 @@
*/
package org.apache.aries.versioning.utils;
-public class BinaryCompatibilityStatus {
- private final boolean compatible;
- private final String reason;
-
- public BinaryCompatibilityStatus(boolean compatible, String reason) {
- this.compatible = compatible;
- this.reason = reason;
- }
+import java.util.ArrayList;
- public boolean isCompatible() {
- return compatible;
- }
+public class BinaryCompatibilityStatus extends ArrayList<String> {
- public String getReason() {
- return reason;
- }
- @Override
- public int hashCode() {
- final int prime = 31;
- int result = 1;
- result = prime * result + (compatible ? 1231 : 1237);
- result = prime * result + ((reason == null) ? 0 : reason.hashCode());
- return result;
+ public BinaryCompatibilityStatus() {
}
- @Override
- public boolean equals(Object obj) {
- if (this == obj) return true;
- if (obj == null) return false;
- if (getClass() != obj.getClass()) return false;
- BinaryCompatibilityStatus other = (BinaryCompatibilityStatus) obj;
- if (compatible != other.compatible) return false;
- if (reason == null) {
- if (other.reason != null) return false;
- } else if (!reason.equals(other.reason)) return false;
- return true;
+ public boolean isCompatible() {
+ return isEmpty();
}
-
}
Modified: aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/ClassDeclaration.java
URL: http://svn.apache.org/viewvc/aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/ClassDeclaration.java?rev=1235096&r1=1235095&r2=1235096&view=diff
==============================================================================
--- aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/ClassDeclaration.java (original)
+++ aries/trunk/versioning/versioning-checker/src/main/java/org/apache/aries/versioning/utils/ClassDeclaration.java Tue Jan 24 00:53:35 2012
@@ -25,6 +25,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -32,9 +33,6 @@ import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.Type;
-import static org.apache.aries.versioning.utils.SemanticVersioningUtils.htmlOneLineBreak;
-import static org.apache.aries.versioning.utils.SemanticVersioningUtils.htmlTwoLineBreaks;
-
public class ClassDeclaration extends GenericDeclaration {
// Binary Compatibility - deletion of package-level access field/method/constructors of classes and interfaces in the package
@@ -58,7 +56,6 @@ public class ClassDeclaration extends Ge
private final URLClassLoader jarsLoader;
- private final BinaryCompatibilityStatus binaryCompatible = new BinaryCompatibilityStatus(true, null);
private final SerialVersionClassVisitor serialVisitor;
public Map<String, FieldDeclaration> getFields() {
@@ -89,7 +86,7 @@ public class ClassDeclaration extends Ge
/**
* Get the methods in the current class plus the methods in the upper chain
*
- * @return
+ * @return map of method name to set of method declarations
*/
public Map<String, Set<MethodDeclaration>> getAllMethods() {
@@ -145,15 +142,13 @@ public class ClassDeclaration extends Ge
SerialVersionClassVisitor cv = new SerialVersionClassVisitor(cw);
SemanticVersioningClassVisitor svc = new SemanticVersioningClassVisitor(jarsLoader, cv);
ClassReader cr = new ClassReader(jarsLoader.getResourceAsStream(superClass + SemanticVersioningUtils.classExt));
- if (cr != null) {
- cr.accept(svc, 0);
- ClassDeclaration cd = svc.getClassDeclaration();
- if (cd != null) {
- addFieldInUpperChain(cd.getFields());
- getFieldsRecursively(cd.getSuperName());
- for (String iface : cd.getInterfaces()) {
- getFieldsRecursively(iface);
- }
+ cr.accept(svc, 0);
+ ClassDeclaration cd = svc.getClassDeclaration();
+ if (cd != null) {
+ addFieldInUpperChain(cd.getFields());
+ getFieldsRecursively(cd.getSuperName());
+ for (String iface : cd.getInterfaces()) {
+ getFieldsRecursively(iface);
}
}
} catch (IOException ioe) {
@@ -172,15 +167,13 @@ public class ClassDeclaration extends Ge
// use URLClassLoader to load the class
try {
ClassReader cr = new ClassReader(jarsLoader.getResourceAsStream(superClass + SemanticVersioningUtils.classExt));
- if (cr != null) {
- cr.accept(svc, 0);
- ClassDeclaration cd = svc.getClassDeclaration();
- if (cd != null) {
- addMethodsInUpperChain(cd.getMethods());
- getMethodsRecursively(cd.getSuperName());
- for (String iface : cd.getInterfaces()) {
- getMethodsRecursively(iface);
- }
+ cr.accept(svc, 0);
+ ClassDeclaration cd = svc.getClassDeclaration();
+ if (cd != null) {
+ addMethodsInUpperChain(cd.getMethods());
+ getMethodsRecursively(cd.getSuperName());
+ for (String iface : cd.getInterfaces()) {
+ getMethodsRecursively(iface);
}
}
} catch (IOException ioe) {
@@ -243,7 +236,6 @@ public class ClassDeclaration extends Ge
clazz.add(className);
if (svc.getClassDeclaration() != null) {
String superName = svc.getClassDeclaration().getSuperName();
- className = superName;
clazz.addAll(getUpperChainRecursively(superName));
if (svc.getClassDeclaration().getInterfaces() != null) {
for (String iface : svc.getClassDeclaration().getInterfaces()) {
@@ -297,30 +289,17 @@ public class ClassDeclaration extends Ge
public BinaryCompatibilityStatus getBinaryCompatibleStatus(ClassDeclaration old) {
// check class signature, fields, methods
- if (old == null) {
- return binaryCompatible;
- }
- StringBuilder reason = new StringBuilder();
- boolean isCompatible = true;
- Set<BinaryCompatibilityStatus> bcsSet = new HashSet<BinaryCompatibilityStatus>();
- bcsSet.add(getClassSignatureBinaryCompatibleStatus(old));
- bcsSet.add(getAllMethodsBinaryCompatibleStatus(old));
- bcsSet.add(getAllFieldsBinaryCompatibleStatus(old));
- bcsSet.add(getAllSuperPresentStatus(old));
- bcsSet.add(getSerializableBackCompatable(old));
- for (BinaryCompatibilityStatus bcs : bcsSet) {
- if (!bcs.isCompatible()) {
- isCompatible = false;
- reason.append(bcs.getReason());
- }
- }
- if (!isCompatible) {
- return new BinaryCompatibilityStatus(isCompatible, reason.toString());
- } else {
- return binaryCompatible;
+ BinaryCompatibilityStatus reasons = new BinaryCompatibilityStatus();
+ if (old == null) {
+ return reasons;
}
-
+ getClassSignatureBinaryCompatibleStatus(old, reasons);
+ getAllMethodsBinaryCompatibleStatus(old, reasons);
+ getAllFieldsBinaryCompatibleStatus(old, reasons);
+ getAllSuperPresentStatus(old, reasons);
+ getSerializableBackCompatable(old, reasons);
+ return reasons;
}
@@ -328,60 +307,42 @@ public class ClassDeclaration extends Ge
return Modifier.isAbstract(getAccess());
}
- private BinaryCompatibilityStatus getClassSignatureBinaryCompatibleStatus(ClassDeclaration originalClass) {
+ private void getClassSignatureBinaryCompatibleStatus(ClassDeclaration originalClass, List<String> reasons) {
// if a class was not abstract but changed to abstract
// not final changed to final
// public changed to non-public
String prefix = " The class " + getName();
- StringBuilder reason = new StringBuilder();
- boolean compatible = true;
if (!!!originalClass.isAbstract() && isAbstract()) {
- reason.append(prefix + " was not abstract but is changed to be abstract.");
- compatible = false;
+ reasons.add(prefix + " was not abstract but is changed to be abstract.");
}
if (!!!originalClass.isFinal() && isFinal()) {
- reason.append(prefix + " was not final but is changed to be final.");
- compatible = false;
+ reasons.add(prefix + " was not final but is changed to be final.");
}
if (originalClass.isPublic() && !!!isPublic()) {
- reason.append(prefix + " was public but is changed to be non-public.");
- compatible = false;
+ reasons.add(prefix + " was public but is changed to be non-public.");
}
- return new BinaryCompatibilityStatus(compatible, compatible ? null : reason.toString());
}
- public BinaryCompatibilityStatus getAllFieldsBinaryCompatibleStatus(ClassDeclaration originalClass) {
+ private void getAllFieldsBinaryCompatibleStatus(ClassDeclaration originalClass, List<String> reasons) {
// for each field to see whether the same field has changed
// not final -> final
// static <-> nonstatic
Map<String, FieldDeclaration> oldFields = originalClass.getAllFields();
Map<String, FieldDeclaration> newFields = getAllFields();
- return areFieldsBinaryCompatible(oldFields, newFields);
+ areFieldsBinaryCompatible(oldFields, newFields, reasons);
}
- private BinaryCompatibilityStatus areFieldsBinaryCompatible(Map<String, FieldDeclaration> oldFields, Map<String, FieldDeclaration> currentFields) {
-
- boolean overallCompatible = true;
- StringBuilder reason = new StringBuilder();
+ private void areFieldsBinaryCompatible(Map<String, FieldDeclaration> oldFields, Map<String, FieldDeclaration> currentFields, List<String> reasons) {
for (Map.Entry<String, FieldDeclaration> entry : oldFields.entrySet()) {
FieldDeclaration bef_fd = entry.getValue();
FieldDeclaration cur_fd = currentFields.get(entry.getKey());
- boolean compatible = isFieldBinaryCompatible(reason, bef_fd, cur_fd);
- if (!compatible) {
- overallCompatible = compatible;
- }
-
- }
- if (!overallCompatible) {
- return new BinaryCompatibilityStatus(overallCompatible, reason.toString());
- } else {
- return binaryCompatible;
+ isFieldBinaryCompatible(reasons, bef_fd, cur_fd);
}
}
- private boolean isFieldBinaryCompatible(StringBuilder reason,
+ private boolean isFieldBinaryCompatible(List<String> reasons,
FieldDeclaration bef_fd, FieldDeclaration cur_fd) {
String fieldName = bef_fd.getName();
//only interested in the public or protected fields
@@ -389,34 +350,34 @@ public class ClassDeclaration extends Ge
boolean compatible = true;
if (bef_fd.isPublic() || bef_fd.isProtected()) {
- String prefix = htmlOneLineBreak + "The " + (bef_fd.isPublic() ? "public" : "protcted") + " field " + fieldName;
+ String prefix = "The " + (bef_fd.isPublic() ? "public" : "protected") + " field " + fieldName;
if (cur_fd == null) {
- reason.append(prefix + " has been deleted.");
+ reasons.add(prefix + " has been deleted.");
compatible = false;
} else {
if ((!!!bef_fd.isFinal()) && (cur_fd.isFinal())) {
// make sure it has not been changed to final
- reason.append(prefix + " was not final but has been changed to be final.");
+ reasons.add(prefix + " was not final but has been changed to be final.");
compatible = false;
}
if (bef_fd.isStatic() != cur_fd.isStatic()) {
// make sure it the static signature has not been changed
- reason.append(prefix + " was static but is changed to be non static or vice versa.");
+ reasons.add(prefix + " was static but is changed to be non static or vice versa.");
compatible = false;
}
// check to see the field type is the same
if (!isFieldTypeSame(bef_fd, cur_fd)) {
- reason.append(prefix + " has changed its type.");
+ reasons.add(prefix + " has changed its type.");
compatible = false;
}
if (SemanticVersioningUtils.isLessAccessible(bef_fd, cur_fd)) {
// check whether the new field is less accessible than the old one
- reason.append(prefix + " becomes less accessible.");
+ reasons.add(prefix + " becomes less accessible.");
compatible = false;
}
@@ -428,13 +389,12 @@ public class ClassDeclaration extends Ge
/**
* Return whether the serializable class is binary compatible. The serial verison uid change breaks binary compatibility.
*
- * @param old
- * @return
+ *
+ * @param old Old class declaration
+ * @param reasons list of binary compatibility problems
*/
- private BinaryCompatibilityStatus getSerializableBackCompatable(ClassDeclaration old) {
- // It does not matter one of them is not seralizable.
- boolean serializableBackCompatible = true;
- String reason = null;
+ private void getSerializableBackCompatable(ClassDeclaration old, List<String> reasons) {
+ // It does not matter one of them is not serializable.
if ((getAllSupers().contains(SemanticVersioningUtils.SERIALIZABLE_CLASS_IDENTIFIER)) && (old.getAllSupers().contains(SemanticVersioningUtils.SERIALIZABLE_CLASS_IDENTIFIER))) {
// check to see whether the serializable id is the same
//ignore if it is enum
@@ -442,16 +402,11 @@ public class ClassDeclaration extends Ge
long oldValue = getSerialVersionUID(old);
long curValue = getSerialVersionUID(this);
if ((oldValue != curValue)) {
- serializableBackCompatible = false;
- reason = htmlOneLineBreak + "The serializable class is no longer back compatible as the value of SerialVersionUID has changed from " + oldValue + " to " + curValue + ".";
+ reasons.add("The serializable class is no longer back compatible as the value of SerialVersionUID has changed from " + oldValue + " to " + curValue + ".");
}
}
}
- if (!serializableBackCompatible) {
- return new BinaryCompatibilityStatus(serializableBackCompatible, reason);
- }
- return binaryCompatible;
}
private long getSerialVersionUID(ClassDeclaration cd) {
@@ -459,7 +414,7 @@ public class ClassDeclaration extends Ge
if (serialID != null) {
if (serialID.isFinal() && serialID.isStatic() && Type.LONG_TYPE.equals(Type.getType(serialID.getDesc()))) {
if (serialID.getValue() != null) {
- return ((Long) (serialID.getValue())).longValue();
+ return (Long) (serialID.getValue());
} else {
return 0;
}
@@ -485,20 +440,19 @@ public class ClassDeclaration extends Ge
}
- private BinaryCompatibilityStatus getAllMethodsBinaryCompatibleStatus(ClassDeclaration originalClass) {
+ private void getAllMethodsBinaryCompatibleStatus(ClassDeclaration originalClass, List<String> reasons) {
// for all methods
// no methods should have deleted
// method return type has not changed
// method changed from not abstract -> abstract
Map<String, Set<MethodDeclaration>> oldMethods = originalClass.getAllMethods();
Map<String, Set<MethodDeclaration>> newMethods = getAllMethods();
- return areMethodsBinaryCompatible(oldMethods, newMethods);
+ areMethodsBinaryCompatible(oldMethods, newMethods, reasons);
}
- public BinaryCompatibilityStatus areMethodsBinaryCompatible(
- Map<String, Set<MethodDeclaration>> oldMethods, Map<String, Set<MethodDeclaration>> newMethods) {
+ private void areMethodsBinaryCompatible(
+ Map<String, Set<MethodDeclaration>> oldMethods, Map<String, Set<MethodDeclaration>> newMethods, List<String> reasons) {
- StringBuilder reason = new StringBuilder();
boolean compatible = true;
Map<String, Collection<MethodDeclaration>> extraMethods = new HashMap<String, Collection<MethodDeclaration>>();
@@ -518,7 +472,7 @@ public class ClassDeclaration extends Ge
for (MethodDeclaration md : oldMDSigs) {
String mdName = md.getName();
- String prefix = htmlOneLineBreak + "The " + SemanticVersioningUtils.getReadableMethodSignature(mdName, md.getDesc());
+ String prefix = "The " + SemanticVersioningUtils.getReadableMethodSignature(mdName, md.getDesc());
if (md.isProtected() || md.isPublic()) {
boolean found = false;
if (newMDSigs != null) {
@@ -533,19 +487,19 @@ public class ClassDeclaration extends Ge
if (!!!Modifier.isFinal(md.getAccess()) && !!!Modifier.isStatic(md.getAccess()) && Modifier.isFinal(new_md.getAccess())) {
compatible = false;
- reason.append(prefix + " was not final but has been changed to be final.");
+ reasons.add(prefix + " was not final but has been changed to be final.");
}
if (Modifier.isStatic(md.getAccess()) != Modifier.isStatic(new_md.getAccess())) {
compatible = false;
- reason.append(prefix + " has changed from static to non-static or vice versa.");
+ reasons.add(prefix + " has changed from static to non-static or vice versa.");
}
- if ((Modifier.isAbstract(new_md.getAccess()) == true) && (Modifier.isAbstract(md.getAccess()) == false)) {
+ if ((Modifier.isAbstract(new_md.getAccess())) && (!Modifier.isAbstract(md.getAccess()))) {
compatible = false;
- reason.append(prefix + " has changed from non abstract to abstract. ");
+ reasons.add(prefix + " has changed from non abstract to abstract.");
}
if (SemanticVersioningUtils.isLessAccessible(md, new_md)) {
compatible = false;
- reason.append(prefix + " is less accessible.");
+ reasons.add(prefix + " is less accessible.");
}
if (compatible) {
@@ -566,7 +520,7 @@ public class ClassDeclaration extends Ge
if (!isMethodInSuperClass(md)) {
compatible = false;
- reason.append(prefix + " has been deleted or its return type or parameter list has changed.");
+ reasons.add(prefix + " has been deleted or its return type or parameter list has changed.");
} else {
if (newMDSigs != null) {
for (MethodDeclaration new_md : newMDSigs) {
@@ -587,24 +541,17 @@ public class ClassDeclaration extends Ge
// Check the newly added method has not caused binary incompatibility
for (Map.Entry<String, Collection<MethodDeclaration>> extraMethodSet : extraMethods.entrySet()) {
for (MethodDeclaration md : extraMethodSet.getValue()) {
- String head = htmlOneLineBreak + "The " + SemanticVersioningUtils.getReadableMethodSignature(md.getName(), md.getDesc());
- if (isNewMethodSpecialCase(md, head, reason)) {
- compatible = false;
- }
+ String head = "The " + SemanticVersioningUtils.getReadableMethodSignature(md.getName(), md.getDesc());
+ isNewMethodSpecialCase(md, head, reasons);
}
}
- if (compatible) {
- return binaryCompatible;
- } else {
- return new BinaryCompatibilityStatus(compatible, reason.toString());
- }
}
/**
* Return the newly added fields
*
- * @param old
- * @return
+ * @param old old class declaration
+ * @return FieldDeclarations for fields added to new class
*/
public Collection<FieldDeclaration> getExtraFields(ClassDeclaration old) {
Map<String, FieldDeclaration> oldFields = old.getAllFields();
@@ -619,8 +566,8 @@ public class ClassDeclaration extends Ge
/**
* Return the extra non-private methods
*
- * @param old
- * @return
+ * @param old old class declaration
+ * @return method declarations for methods added to new class
*/
public Collection<MethodDeclaration> getExtraMethods(ClassDeclaration old) {
// Need to find whether there are new methods added.
@@ -668,10 +615,13 @@ public class ClassDeclaration extends Ge
/**
* The newly added method is less accessible than the old one in the super or is a static (respectively instance) method.
*
- * @param md
- * @return
+ *
+ * @param md method declaration
+ * @param prefix beginning of incompatibility message
+ * @param reasons list of binary incompatibility reasons
+ * @return whether new method is less accessible or changed static-ness compared to old class
*/
- public boolean isNewMethodSpecialCase(MethodDeclaration md, String prefix, StringBuilder reason) {
+ private boolean isNewMethodSpecialCase(MethodDeclaration md, String prefix, List<String> reasons) {
// scan the super class and interfaces
String methodName = md.getName();
boolean special = false;
@@ -683,17 +633,17 @@ public class ClassDeclaration extends Ge
if (md.equals(value)) {
if (SemanticVersioningUtils.isLessAccessible(value, md)) {
special = true;
- reason.append(prefix + " is less accessible than the same method in its parent.");
+ reasons.add(prefix + " is less accessible than the same method in its parent.");
}
if (value.isStatic()) {
if (!md.isStatic()) {
special = true;
- reason.append(prefix + " is non-static but the same method in its parent is static.");
+ reasons.add(prefix + " is non-static but the same method in its parent is static.");
}
} else {
if (md.isStatic()) {
special = true;
- reason.append(prefix + " is static but the same method is its parent is not static.");
+ reasons.add(prefix + " is static but the same method is its parent is not static.");
}
}
}
@@ -703,14 +653,13 @@ public class ClassDeclaration extends Ge
return special;
}
- public BinaryCompatibilityStatus getAllSuperPresentStatus(ClassDeclaration old) {
+ private void getAllSuperPresentStatus(ClassDeclaration old, List<String> reasons) {
Collection<String> oldSupers = old.getAllSupers();
boolean containsAll = getAllSupers().containsAll(oldSupers);
if (!!!containsAll) {
oldSupers.removeAll(getAllSupers());
- return new BinaryCompatibilityStatus(false, htmlTwoLineBreaks + "The superclasses or superinterfaces have stopped being super: " + oldSupers.toString() + ".");
+ reasons.add("The superclasses or superinterfaces have stopped being super: " + oldSupers.toString() + ".");
}
- return binaryCompatible;
}
public SerialVersionClassVisitor getSerialVisitor() {
Modified: aries/trunk/versioning/versioning-checker/src/test/java/org/apache/aries/versioning/tests/BinaryCompatibilityTest.java
URL: http://svn.apache.org/viewvc/aries/trunk/versioning/versioning-checker/src/test/java/org/apache/aries/versioning/tests/BinaryCompatibilityTest.java?rev=1235096&r1=1235095&r2=1235096&view=diff
==============================================================================
--- aries/trunk/versioning/versioning-checker/src/test/java/org/apache/aries/versioning/tests/BinaryCompatibilityTest.java (original)
+++ aries/trunk/versioning/versioning-checker/src/test/java/org/apache/aries/versioning/tests/BinaryCompatibilityTest.java Tue Jan 24 00:53:35 2012
@@ -38,6 +38,7 @@ import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.net.URLClassLoader;
+import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
@@ -45,8 +46,6 @@ import org.apache.aries.versioning.utils
import org.apache.aries.versioning.utils.MethodDeclaration;
import org.apache.aries.versioning.utils.SemanticVersioningClassVisitor;
-import static org.apache.aries.versioning.utils.SemanticVersioningUtils.htmlOneLineBreak;
-import static org.apache.aries.versioning.utils.SemanticVersioningUtils.htmlTwoLineBreaks;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -94,11 +93,11 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertFalse(
+ assertTrue(
"When a class is changed from non abstract to abstract, this should break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
- assertEquals(" The class pkg/Test was not abstract but is changed to be abstract.", bcs.getReason());
+ assertEquals(" The class pkg/Test was not abstract but is changed to be abstract.", bcs.get(0));
}
/**
@@ -155,11 +154,11 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertFalse(
+ assertTrue(
"When a class is changed from non final to final, this should break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
- assertEquals(" The class pkg/Test was not final but is changed to be final.", bcs.getReason());
+ assertEquals(" The class pkg/Test was not final but is changed to be final.", bcs.get(0));
}
/**
@@ -296,10 +295,11 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertFalse(
+ assertTrue(
"Changing the direct superclass or the set of direct superinterfaces of a class type results fields changes. This should breake binary compatibility if not losing any members.",
- bcs.isCompatible());
- assertEquals(" The public field bar was static but is changed to be non static or vice versa. The public field bar has changed its type.", bcs.getReason());
+ bcs.size() == 2);
+ assertEquals(new HashSet<String>(Arrays.asList(new String[] {"The public field bar was static but is changed to be non static or vice versa.",
+ "The public field bar has changed its type."})), new HashSet<String>(bcs));
}
/**
@@ -326,10 +326,11 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The public field more has changed its type. The public field more becomes less accessible.", bcs.getReason());
- assertFalse(
+ assertEquals(new HashSet<String>(Arrays.asList(new String[] {"The public field more has changed its type.",
+ "The public field more becomes less accessible."})), new HashSet<String>(bcs));
+ assertTrue(
"Changing the declared access of a field to permit less access , this should break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 2);
}
/**
@@ -358,7 +359,15 @@ public class BinaryCompatibilityTest {
assertFalse(
"If a change to the direct superclass or the set of direct superinterfaces results in any class or interface no longer being a superclass or superinterface, respectively, it will break binary compatibility.",
bcs.isCompatible());
- assertEquals(" The method int getFooLen(java.lang.String) has been deleted or its return type or parameter list has changed. The method java.lang.String getFoo() has changed from non abstract to abstract. The method int getBarLen(java.lang.String) has been deleted or its return type or parameter list has changed. The method int getBooLen(java.lang.String) has been deleted or its return type or parameter list has changed. The superclasses or superinterfaces have stopped being super: [versioning/java/files/TestC, versioning/java/files/TestA]. The protcted field c has been deleted. The public field bar was not final but has been changed to be final. The public field bar was static but is changed to be non static or vice versa. The public field bar has changed its type.", bcs.getReason());
+ assertEquals(new HashSet<String>(Arrays.asList(new String[] {"The method int getFooLen(java.lang.String) has been deleted or its return type or parameter list has changed.",
+ "The method java.lang.String getFoo() has changed from non abstract to abstract.",
+ "The method int getBarLen(java.lang.String) has been deleted or its return type or parameter list has changed.",
+ "The method int getBooLen(java.lang.String) has been deleted or its return type or parameter list has changed.",
+ "The superclasses or superinterfaces have stopped being super: [versioning/java/files/TestC, versioning/java/files/TestA].",
+ "The protected field c has been deleted.",
+ "The public field bar was not final but has been changed to be final.",
+ "The public field bar was static but is changed to be non static or vice versa.",
+ "The public field bar has changed its type."})), new HashSet<String>(bcs));
}
/**
@@ -385,10 +394,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int convert(java.lang.Object) has been deleted or its return type or parameter list has changed.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int convert(java.lang.Object) has been deleted or its return type or parameter list has changed.", bcs.get(0));
+ assertTrue(
"deleting a class member or constructor that is not declared private breaks binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
@@ -473,10 +482,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int convert(java.lang.Object) is less accessible.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int convert(java.lang.Object) is less accessible.", bcs.get(0));
+ assertTrue(
"Changing the declared access of a member or contructor to permit less access , this should break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
@Test
@@ -500,10 +509,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The public field lESS becomes less accessible.", bcs.getReason());
- assertFalse(
+ assertEquals("The public field lESS becomes less accessible.", bcs.get(0));
+ assertTrue(
"Changing the declared access of a field to permit less access , this should break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -558,10 +567,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The public field bar becomes less accessible.", bcs.getReason());
- assertFalse(
+ assertEquals("The public field bar becomes less accessible.", bcs.get(0));
+ assertTrue(
"The new field conflicts with a field in the super class. Check chapter 13.4.7 java spec for more info.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -589,10 +598,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The public field aa was not final but has been changed to be final.", bcs.getReason());
- assertFalse(
+ assertEquals("The public field aa was not final but has been changed to be final.", bcs.get(0));
+ assertTrue(
"Change that a public or protected field was final but is changed to be not final will break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -649,10 +658,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The public field aa was static but is changed to be non static or vice versa.", bcs.getReason());
- assertFalse(
+ assertEquals("The public field aa was static but is changed to be non static or vice versa.", bcs.get(0));
+ assertTrue(
"If a field was static is changed to be non-static, then it will break compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -680,10 +689,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The public field aa was static but is changed to be non static or vice versa.", bcs.getReason());
- assertFalse(
+ assertEquals("The public field aa was static but is changed to be non static or vice versa.", bcs.get(0));
+ assertTrue(
"If a field was non-static is changed to be static, then it will break compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -773,10 +782,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getFooLen(java.lang.STring) has been deleted or its return type or parameter list has changed.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getFooLen(java.lang.STring) has been deleted or its return type or parameter list has changed.", bcs.get(0));
+ assertTrue(
"Deleting a public/protected method when there is no such a method in the superclass breaks binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -834,10 +843,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getFooLen(java.lang.String) is less accessible.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getFooLen(java.lang.String) is less accessible.", bcs.get(0));
+ assertTrue(
"If a change to the direct superclass or the set of direct superinterfaces results in any class or interface no longer being a superclass or superinterface, respectively, it will break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -866,10 +875,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getCooLen(java.lang.String) has been deleted or its return type or parameter list has changed.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getCooLen(java.lang.String) has been deleted or its return type or parameter list has changed.", bcs.get(0));
+ assertTrue(
"Changing a parameter list will break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
@@ -899,10 +908,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getCooLen(java.lang.String) has been deleted or its return type or parameter list has changed.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getCooLen(java.lang.String) has been deleted or its return type or parameter list has changed.", bcs.get(0));
+ assertTrue(
"Changing a method paramether type will break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -962,10 +971,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getCooLen(java.lang.String) has been deleted or its return type or parameter list has changed.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getCooLen(java.lang.String) has been deleted or its return type or parameter list has changed.", bcs.get(0));
+ assertTrue(
"Changing a method return type will break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -994,10 +1003,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getCooLen(java.lang.String) has changed from non abstract to abstract. ", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getCooLen(java.lang.String) has changed from non abstract to abstract.", bcs.get(0));
+ assertTrue(
"Changing a method to be abstract will break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -1057,10 +1066,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getCooLen(java.lang.String) was not final but has been changed to be final.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getCooLen(java.lang.String) was not final but has been changed to be final.", bcs.get(0));
+ assertTrue(
"Changing an instance method from non-final to final will break binary compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -1209,10 +1218,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getCooLen(java.lang.String) has changed from static to non-static or vice versa.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getCooLen(java.lang.String) has changed from static to non-static or vice versa.", bcs.get(0));
+ assertTrue(
"If a method is not private was not declared static and is changed to be decalared static, this should break compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -1241,10 +1250,10 @@ public class BinaryCompatibilityTest {
newCR.accept(newCV, 0);
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getCooLen(java.lang.String) has changed from static to non-static or vice versa.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getCooLen(java.lang.String) has changed from static to non-static or vice versa.", bcs.get(0));
+ assertTrue(
"If a method is not private was declared static and is changed to not be decalared static, this should break compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -1422,7 +1431,9 @@ public class BinaryCompatibilityTest {
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(" The public field bar has been deleted. The superclasses or superinterfaces have stopped being super: [versioning/java/files/TestB]. The method java.lang.String getFoo() has been deleted or its return type or parameter list has changed.", bcs.getReason());
+ assertEquals(new HashSet<String>(Arrays.asList(new String[] {"The public field bar has been deleted.",
+ "The superclasses or superinterfaces have stopped being super: [versioning/java/files/TestB].",
+ "The method java.lang.String getFoo() has been deleted or its return type or parameter list has changed."})), new HashSet<String>(bcs));
assertFalse(
"Changes to the interface hierarchy resulting an interface not being a super interface should break compatibility.",
bcs.isCompatible());
@@ -1454,10 +1465,10 @@ public class BinaryCompatibilityTest {
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getFoo() has been deleted or its return type or parameter list has changed.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getFoo() has been deleted or its return type or parameter list has changed.", bcs.get(0));
+ assertTrue(
"Deleting a method in an interface should break compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -1518,10 +1529,10 @@ public class BinaryCompatibilityTest {
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getFoo() has been deleted or its return type or parameter list has changed.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getFoo() has been deleted or its return type or parameter list has changed.", bcs.get(0));
+ assertTrue(
"Changing a method return type in an interface should break compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
@@ -1551,17 +1562,17 @@ public class BinaryCompatibilityTest {
oldCR.accept(oldCV, 0);
BinaryCompatibilityStatus bcs = newCV.getClassDeclaration().getBinaryCompatibleStatus((oldCV.getClassDeclaration()));
- assertEquals(htmlOneLineBreak + "The method int getFoo(int) has been deleted or its return type or parameter list has changed.", bcs.getReason());
- assertFalse(
+ assertEquals("The method int getFoo(int) has been deleted or its return type or parameter list has changed.", bcs.get(0));
+ assertTrue(
"Changing a method parameter in an interface should break compatibility.",
- bcs.isCompatible());
+ bcs.size() == 1);
}
/**
* Check containing more abstract methods
*/
@Test
- public void test_cotaining_more_abstract_methods() {
+ public void test_containing_more_abstract_methods() {
ClassWriter cw = new ClassWriter(0);
cw.visit(V1_5, ACC_PUBLIC + ACC_ABSTRACT + ACC_INTERFACE, "pkg/Test", null, "java/lang/Object", new String[]{"versioning/java/files/TestB"});
cw.visitMethod(ACC_PUBLIC + ACC_ABSTRACT, "getFoo", "(I)I", null, null).visitEnd();