FELIX-4586 : A field must be volatile if methods are generated for a dynamic reference

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1613904 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scrplugin/generator/changelog.txt b/scrplugin/generator/changelog.txt
index f5bb239..6366f0f 100644
--- a/scrplugin/generator/changelog.txt
+++ b/scrplugin/generator/changelog.txt
@@ -1,3 +1,11 @@
+Changes from 1.11.0 to 1.10.0
+----------------------------
+** Improvement
+    * [FELIX-4586] - A field must be volatile if methods are generated for a dynamic reference
+** Bug
+    * [FELIX-4296] - Cannot deactivate service interface detection in DS annotations
+
+
 Changes from 1.10.0 to 1.9.0
 ----------------------------
 ** Improvement
diff --git a/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/Options.java b/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/Options.java
index 610ca29..673877a 100644
--- a/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/Options.java
+++ b/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/Options.java
@@ -46,6 +46,9 @@
     /** Is this an incremental build? */
     private boolean incremental = false;
 
+    /** Skip volatile check. */
+    private boolean skipVolatileCheck = false;
+
     /**
      * @see #setGenerateAccessors(boolean)
      * @return Whether accessor methods should be generated.
@@ -174,4 +177,18 @@
     public void setIncremental(final boolean incremental) {
         this.incremental = incremental;
     }
+
+    /**
+     * Should the check for volatile fields be skipped?
+     */
+    public boolean isSkipVolatileCheck() {
+        return skipVolatileCheck;
+    }
+
+    /**
+     * Set whether the check should be skipped
+     */
+    public void setSkipVolatileCheck(final boolean skipVolatileCheck) {
+        this.skipVolatileCheck = skipVolatileCheck;
+    }
 }
diff --git a/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/SCRDescriptorGenerator.java b/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/SCRDescriptorGenerator.java
index b1fb78b..2268c7b 100644
--- a/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/SCRDescriptorGenerator.java
+++ b/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/SCRDescriptorGenerator.java
@@ -280,6 +280,9 @@
                                     this.project.getClassLoader(),
                                     this.project.getClassesDirectory(),
                                     this.logger);
+                    // set a flag for validation
+                    ref.setBindMethodCreated(createBind);
+                    ref.setUnbindMethodCreated(createUnbind);
                 }
             }
         }
diff --git a/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/description/ReferenceDescription.java b/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/description/ReferenceDescription.java
index 0c2da4b..b5dbd27 100644
--- a/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/description/ReferenceDescription.java
+++ b/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/description/ReferenceDescription.java
@@ -62,6 +62,9 @@
     private String unbind;
     private String updated;
 
+    private boolean bindMethodCreated = false;
+    private boolean unbindMethodCreated = false;
+
     public ReferenceDescription(final ScannedAnnotation annotation) {
         super(annotation);
     }
@@ -154,6 +157,23 @@
         this.field = field;
     }
 
+    public boolean isBindMethodCreated() {
+        return bindMethodCreated;
+    }
+
+    public void setBindMethodCreated(final boolean bindMethodCreated) {
+        this.bindMethodCreated = bindMethodCreated;
+    }
+
+    public boolean isUnbindMethodCreated() {
+        return unbindMethodCreated;
+    }
+
+    public void setUnbindMethodCreated(final boolean unbindMethodCreated) {
+        this.unbindMethodCreated = unbindMethodCreated;
+    }
+
+
     @Override
     public String toString() {
         return "ReferenceDescription [name=" + name + ", interfaceName="
@@ -165,6 +185,15 @@
                 + "]";
     }
 
+
+    @Override
+    public String getIdentifier() {
+        if ( this.getField() != null ) {
+            return super.getIdentifier() + "(" + this.getField().getName() + ")";
+        }
+        return super.getIdentifier();
+    }
+
     @Override
     public AbstractDescription clone() {
         final ReferenceDescription cd = new ReferenceDescription(this.annotation);
@@ -179,6 +208,8 @@
         cd.setBind(this.getBind());
         cd.setUnbind(this.getUnbind());
         cd.setUpdated(this.getUpdated());
+        cd.setBindMethodCreated(this.isBindMethodCreated());
+        cd.setUnbindMethodCreated(this.isUnbindMethodCreated());
 
         return cd;
     }
diff --git a/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/helper/Validator.java b/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/helper/Validator.java
index 6403286..9b3fa6d 100644
--- a/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/helper/Validator.java
+++ b/scrplugin/generator/src/main/java/org/apache/felix/scrplugin/helper/Validator.java
@@ -480,7 +480,7 @@
                 if ( bindName != null ) {
                     bindName = this.validateMethod(ref, bindName, componentIsAbstract);
                     if ( bindName == null && ref.getField() != null ) {
-                        iLog.addError("Something went wrong: " + canGenerate + " - " + this.options.isGenerateAccessors() + " - " + ref.getCardinality(), ref.getField().getName());
+                        this.logError(ref, "Something went wrong: " + canGenerate + " - " + this.options.isGenerateAccessors() + " - " + ref.getCardinality());
                     }
                 } else {
                     bindName = "bind" + Character.toUpperCase(ref.getName().charAt(0)) + ref.getName().substring(1);
@@ -491,10 +491,32 @@
                     unbindName = "unbind" + Character.toUpperCase(ref.getName().charAt(0)) + ref.getName().substring(1);
                 }
 
+                // check for volatile on dynamic field reference with cardinality unary
+                if ( !this.options.isSkipVolatileCheck() ) {
+                    if ( ref.getField() != null
+                         && (ref.getCardinality() == ReferenceCardinality.OPTIONAL_UNARY || ref.getCardinality() == ReferenceCardinality.MANDATORY_UNARY)
+                         && ref.getPolicy() == ReferencePolicy.DYNAMIC ) {
+                        final boolean fieldIsVolatile = Modifier.isVolatile(ref.getField().getModifiers());
+
+                        if ( ref.isBindMethodCreated() || ref.isUnbindMethodCreated() ) {
+                            // field must be volatile
+                            if (!fieldIsVolatile) {
+                                this.logError(ref, "Dynamic field must be declared volatile for unary references");
+                            }
+                        } else {
+                            // field should be volatile
+                            if (!fieldIsVolatile) {
+                                this.logError(ref, "Dynamic field should be declared volatile for unary references");
+                            }
+                        }
+                    }
+                }
+
                 if (iLog.getNumberOfErrors() == currentIssueCount) {
                     ref.setBind(bindName);
                     ref.setUnbind(unbindName);
                 }
+
             } else {
                 ref.setBind(null);
                 ref.setUnbind(null);
diff --git a/scrplugin/maven-scr-plugin/changelog.txt b/scrplugin/maven-scr-plugin/changelog.txt
index a331a49..e280674 100644
--- a/scrplugin/maven-scr-plugin/changelog.txt
+++ b/scrplugin/maven-scr-plugin/changelog.txt
@@ -1,3 +1,12 @@
+Changes from 1.18.0 to 1.17.0
+-----------------------------
+** Improvement
+    * [FELIX-4586] - A field must be volatile if methods are generated for a dynamic reference
+    * [FELIX-4530] - Revisit setting the default output to target/classes
+** Bug
+    * [FELIX-4296] - Cannot deactivate service interface detection in DS annotations
+
+
 Changes from 1.17.0 to 1.16.0
 -----------------------------
 ** Improvement
diff --git a/scrplugin/maven-scr-plugin/src/main/java/org/apache/felix/scrplugin/mojo/SCRDescriptorMojo.java b/scrplugin/maven-scr-plugin/src/main/java/org/apache/felix/scrplugin/mojo/SCRDescriptorMojo.java
index c93d361..ffa879c 100644
--- a/scrplugin/maven-scr-plugin/src/main/java/org/apache/felix/scrplugin/mojo/SCRDescriptorMojo.java
+++ b/scrplugin/maven-scr-plugin/src/main/java/org/apache/felix/scrplugin/mojo/SCRDescriptorMojo.java
@@ -164,6 +164,13 @@
     private boolean scanClasses;
 
     /**
+     * Skip volatile check for fields.
+     *
+     * @parameter default-value="false"
+     */
+    private boolean skipVolatileCheck;
+
+    /**
      * @component
      */
     private BuildContext buildContext;
@@ -202,6 +209,7 @@
         options.setProperties(properties);
         options.setSpecVersion(SpecVersion.fromName(specVersion));
         options.setIncremental(this.buildContext.isIncremental());
+        options.setSkipVolatileCheck(this.skipVolatileCheck);
 
         if ( specVersion != null && options.getSpecVersion() == null ) {
             throw new MojoExecutionException("Unknown spec version specified: " + specVersion);
diff --git a/scrplugin/scrtask/changelog.txt b/scrplugin/scrtask/changelog.txt
index 021b910..8cd24e8 100644
--- a/scrplugin/scrtask/changelog.txt
+++ b/scrplugin/scrtask/changelog.txt
@@ -1,3 +1,11 @@
+Changes from 1.12.0 to 1.11.0
+-----------------------------
+** Improvement
+    * [FELIX-4586] - A field must be volatile if methods are generated for a dynamic reference
+** Bug
+    * [FELIX-4296] - Cannot deactivate service interface detection in DS annotations
+
+
 Changes from 1.11.0 to 1.10.0
 -----------------------------
 ** Improvement