TUSCANY-3136 trap use of @Property reference on methods

git-svn-id: http://svn.us.apache.org/repos/asf/tuscany@792573 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 73 insertions, 32 deletions

diff --git a/java/sca/modules/implementation-java/src/main/java/org/apache/tuscany/sca/implementation/java/introspect/impl/AbstractPropertyProcessor.java b/java/sca/modules/implementation-java/src/main/java/org/apache/tuscany/sca/implementation/java/introspect/impl/AbstractPropertyProcessor.java
index e7f2638255..51e7164e0e 100644
--- a/java/sca/modules/implementation-java/src/main/java/org/apache/tuscany/sca/implementation/java/introspect/impl/AbstractPropertyProcessor.java
+++ b/java/sca/modules/implementation-java/src/main/java/org/apache/tuscany/sca/implementation/java/introspect/impl/AbstractPropertyProcessor.java
@@ -67,41 +67,52 @@ public abstract class AbstractPropertyProcessor<A extends Annotation> extends Ba
     @Override
     public void visitMethod(Method method, JavaImplementation type) throws IntrospectionException {
         A annotation = method.getAnnotation(annotationClass);
-        if (annotation == null) {
-            return;
-        }
-
-        if (!JavaIntrospectionHelper.isSetter(method)) {
-            throw new IllegalPropertyException("Annotated method is not a setter: " + method, method);
-        }
-
-        String name = getName(annotation);
-        if (name == null || "".equals(name)) {
-            name = method.getName();
-            if (name.startsWith("set")) {
-                name = JavaIntrospectionHelper.toPropertyName(method.getName());
-            }
-        }
-
-        Map<String, JavaElementImpl> properties = type.getPropertyMembers();
-        JavaElementImpl prop = properties.get(name);
-        // Setter override field
-        if (prop != null && prop.getElementType() != ElementType.FIELD) {
-            throw new DuplicatePropertyException(name);
+        if (annotation != null) {
+        	
+	        if (!JavaIntrospectionHelper.isSetter(method)) {
+	            throw new IllegalPropertyException("Annotated method is not a setter: " + method, method);
+	        }
+	
+	        String name = getName(annotation);
+	        if (name == null || "".equals(name)) {
+	            name = method.getName();
+	            if (name.startsWith("set")) {
+	                name = JavaIntrospectionHelper.toPropertyName(method.getName());
+	            }
+	        }
+	
+	        Map<String, JavaElementImpl> properties = type.getPropertyMembers();
+	        JavaElementImpl prop = properties.get(name);
+	        // Setter override field
+	        if (prop != null && prop.getElementType() != ElementType.FIELD) {
+	            throw new DuplicatePropertyException(name);
+	        }
+	
+	        removeProperty(prop, type);
+	        
+	        JavaElementImpl element = new JavaElementImpl(method, 0);
+	        Property property = createProperty(name, element);
+	
+	        // add databinding available as annotations, as extensions
+	
+	        initProperty(property, annotation);
+	        type.getProperties().add(property);
+	        properties.put(name, element);
         }
-
-        removeProperty(prop, type);
         
-        JavaElementImpl element = new JavaElementImpl(method, 0);
-        Property property = createProperty(name, element);
-
-        // add databinding available as annotations, as extensions
-
-        initProperty(property, annotation);
-        type.getProperties().add(property);
-        properties.put(name, element);
+        // enforce the constraint that an ordinary method's argument can not be a Property
+        Annotation paramsAnnotations[][] = method.getParameterAnnotations();
+        for (int i = 0; i < paramsAnnotations.length; i++) {
+        	Annotation argAnnotations[] = paramsAnnotations[i];
+        	for (int j = 0; j < argAnnotations.length; j++) {
+        		if(argAnnotations[j].annotationType() == org.oasisopen.sca.annotation.Property.class) {
+        			throw new IllegalPropertyException("Argument " + (i+1) + " of method " + method.getName() + " in class " + method.getDeclaringClass() + " can not be a Property");
+        		}
+        	}
+		}
     }
 
+
     @Override
     public void visitField(Field field, JavaImplementation type) throws IntrospectionException {
 
diff --git a/java/sca/modules/implementation-java/src/main/java/org/apache/tuscany/sca/implementation/java/introspect/impl/ReferenceProcessor.java b/java/sca/modules/implementation-java/src/main/java/org/apache/tuscany/sca/implementation/java/introspect/impl/ReferenceProcessor.java
index 2c6232d66a..1c0cc3595a 100644
--- a/java/sca/modules/implementation-java/src/main/java/org/apache/tuscany/sca/implementation/java/introspect/impl/ReferenceProcessor.java
+++ b/java/sca/modules/implementation-java/src/main/java/org/apache/tuscany/sca/implementation/java/introspect/impl/ReferenceProcessor.java
@@ -88,7 +88,6 @@ public class ReferenceProcessor extends BaseJavaClassVisitor {
         for (int i = 0; i < paramsAnnotations.length; i++) {
         	Annotation argAnnotations[] = paramsAnnotations[i];
         	for (int j = 0; j < argAnnotations.length; j++) {
-        		Annotation ann = argAnnotations[j];
         		if(argAnnotations[j].annotationType() == Reference.class) {
         			throw new IllegalReferenceException("Argument " + (i+1) + " of method " + method.getName() + " in class " + method.getDeclaringClass() + " can not be a Reference");
         		}
diff --git a/java/sca/modules/implementation-java/src/test/java/org/apache/tuscany/sca/implementation/java/introspect/impl/ConstructorPropertyTestCase.java b/java/sca/modules/implementation-java/src/test/java/org/apache/tuscany/sca/implementation/java/introspect/impl/ConstructorPropertyTestCase.java
index 3f531e48d6..be90ba297e 100644
--- a/java/sca/modules/implementation-java/src/test/java/org/apache/tuscany/sca/implementation/java/introspect/impl/ConstructorPropertyTestCase.java
+++ b/java/sca/modules/implementation-java/src/test/java/org/apache/tuscany/sca/implementation/java/introspect/impl/ConstructorPropertyTestCase.java
@@ -25,6 +25,7 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
 import java.util.List;
 
 import org.apache.tuscany.sca.implementation.java.DefaultJavaImplementationFactory;
@@ -119,6 +120,22 @@ public class ConstructorPropertyTestCase extends AbstractProcessorTest {
     // TODO multiplicity
 //    }
 
+    @Test
+    public void testClassWithBadMethodArgProperty() throws Exception {
+        JavaImplementation type = javaImplementationFactory.createJavaImplementation();
+        Method meth = BadFoo2.class.getMethod("BadFoo2Method", String.class);
+
+        try {
+        	propertyProcessor.visitMethod(meth, type);
+        	
+            fail();
+        } catch (IllegalPropertyException e) {
+        	e.printStackTrace();
+        	System.out.println("Exception successfully received");
+        }
+
+    }
+    
     private static class Foo {
 
         @org.oasisopen.sca.annotation.Constructor()
@@ -165,5 +182,19 @@ public class ConstructorPropertyTestCase extends AbstractProcessorTest {
         }
 
     }
+    
+    private static class BadFoo2 {
+
+        @org.oasisopen.sca.annotation.Constructor()
+        public BadFoo2(@Property(name = "myProp", required = true)String prop) {
+
+        }
+        
+        /** Java can't tell that the @reference argument is disallowed by SCA, but the run time must reject it*/
+        public void BadFoo2Method(@Property(name = "badMethodArgProp")String methArg) 
+        {}
+
+ 
+    }
 
 }