You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by th...@apache.org on 2014/06/06 16:50:52 UTC

git commit: Resolves two tickets by refusing to have two different decorator methods with the same in different classes and suggesting a solution. TAP5-2012: Make multiple same name service decoration methods error message better TAP5-1305 Service decora

Repository: tapestry-5
Updated Branches:
  refs/heads/master 270c3830f -> 63ad93a7d


Resolves two tickets by refusing to have two different decorator methods with the same in different classes and suggesting a solution.
TAP5-2012: Make multiple same name service decoration methods error message better
TAP5-1305 Service decorations can fail if using the conventional naming


Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/63ad93a7
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/63ad93a7
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/63ad93a7

Branch: refs/heads/master
Commit: 63ad93a7dc1cdac66c81a43cbd5a21723d54a93b
Parents: 270c383
Author: Thiago H. de Paula Figueiredo <th...@apache.org>
Authored: Fri Jun 6 11:49:00 2014 -0300
Committer: Thiago H. de Paula Figueiredo <th...@apache.org>
Committed: Fri Jun 6 11:49:00 2014 -0300

----------------------------------------------------------------------
 .../tapestry5/ioc/internal/RegistryImpl.java    | 16 ++++++---
 .../tapestry5/ioc/internal/util/Orderer.java    | 20 ++++++++++--
 .../test/groovy/ioc/specs/DecoratorsSpec.groovy | 15 +++++++++
 .../ioc/SpecificDecoratorModuleAgain.java       | 34 ++++++++++++++++++++
 4 files changed, 79 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/63ad93a7/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
index 0407c5d..2859a2d 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/RegistryImpl.java
@@ -841,10 +841,10 @@ public class RegistryImpl implements Registry, InternalRegistry, ServiceProxyPro
         lock.check();
 
         assert serviceDef != null;
-
+        
         Logger logger = getServiceLogger(serviceDef.getServiceId());
 
-        Orderer<ServiceDecorator> orderer = new Orderer<ServiceDecorator>(logger);
+        Orderer<ServiceDecorator> orderer = new Orderer<ServiceDecorator>(logger, true);
 
         for (Module module : moduleToServiceDefs.keySet())
         {
@@ -858,8 +858,16 @@ public class RegistryImpl implements Registry, InternalRegistry, ServiceProxyPro
             for (DecoratorDef decoratorDef : decoratorDefs)
             {
                 ServiceDecorator decorator = decoratorDef.createDecorator(module, resources);
-
-                orderer.add(decoratorDef.getDecoratorId(), decorator, decoratorDef.getConstraints());
+                try
+                {
+                    orderer.add(decoratorDef.getDecoratorId(), decorator, decoratorDef.getConstraints());
+                }
+                catch (IllegalArgumentException e) {
+                    throw new RuntimeException(String.format(
+                            "Service %s has two different decorators methods named decorate%s in different module classes. "
+                            + "You can solve this by renaming one of them and annotating it with @Match(\"%2$s\").", 
+                            serviceDef.getServiceId(), decoratorDef.getDecoratorId()));
+                }
             }
         }
 

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/63ad93a7/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/Orderer.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/Orderer.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/Orderer.java
index 9f7adfc..1a088c6 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/Orderer.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/Orderer.java
@@ -18,7 +18,9 @@ import org.apache.tapestry5.ioc.IdMatcher;
 import org.apache.tapestry5.ioc.Orderable;
 import org.apache.tapestry5.ioc.internal.IdMatcherImpl;
 import org.apache.tapestry5.ioc.internal.OrIdMatcher;
+
 import static org.apache.tapestry5.ioc.internal.util.CollectionFactory.newList;
+
 import org.slf4j.Logger;
 
 import java.util.Collection;
@@ -34,6 +36,8 @@ public class Orderer<T>
     private final OneShotLock lock = new OneShotLock();
 
     private final Logger logger;
+    
+    private final boolean exceptionWhenDuplicateId;
 
     private final List<Orderable> orderables = CollectionFactory.newList();
 
@@ -77,7 +81,13 @@ public class Orderer<T>
 
     public Orderer(Logger logger)
     {
+        this(logger, false);
+    }
+    
+    public Orderer(Logger logger, boolean exceptionWhenDuplicateId)
+    {
         this.logger = logger;
+        this.exceptionWhenDuplicateId = exceptionWhenDuplicateId;
     }
 
     /**
@@ -93,8 +103,14 @@ public class Orderer<T>
 
         if (idToOrderable.containsKey(id))
         {
-            logger.warn(UtilMessages.duplicateOrderer(id));
-            return;
+            final String message = UtilMessages.duplicateOrderer(id);
+            if (exceptionWhenDuplicateId) {
+                throw new IllegalArgumentException(message);
+            }
+            else {
+                logger.warn(message);
+                return;
+            }
         }
 
         orderables.add(orderable);

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/63ad93a7/tapestry-ioc/src/test/groovy/ioc/specs/DecoratorsSpec.groovy
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/test/groovy/ioc/specs/DecoratorsSpec.groovy b/tapestry-ioc/src/test/groovy/ioc/specs/DecoratorsSpec.groovy
index 457988c..0a62539 100644
--- a/tapestry-ioc/src/test/groovy/ioc/specs/DecoratorsSpec.groovy
+++ b/tapestry-ioc/src/test/groovy/ioc/specs/DecoratorsSpec.groovy
@@ -34,6 +34,21 @@ class DecoratorsSpec extends AbstractRegistrySpecification {
 
     g.greeting == "HELLO"
   }
+  
+  // TAP5-1305, TAP5-2012
+  def "two different modules with the same decorator method name"() {
+    
+    buildRegistry GreeterModule, SpecificDecoratorModule, SpecificDecoratorModuleAgain
+        
+    when: 
+    def g = getService "HelloGreeter", Greeter
+    println(g.greeting)
+    
+    then:
+    RuntimeException e = thrown();
+    e.message == "Exception constructing service 'HelloGreeter': Service HelloGreeter has two different decorators methods named decorateHelloGreeter in different module classes. You can solve this by renaming one of them and annotating it with @Match(\"HelloGreeter\").";
+    
+  }
 
   def "a service builder method with @PreventServiceDecoration is not decorated"() {
     buildRegistry PreventDecorationModule

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/63ad93a7/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/SpecificDecoratorModuleAgain.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/SpecificDecoratorModuleAgain.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/SpecificDecoratorModuleAgain.java
new file mode 100644
index 0000000..9dc1e43
--- /dev/null
+++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/SpecificDecoratorModuleAgain.java
@@ -0,0 +1,34 @@
+//  Copyright 2014 The Apache Software Foundation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.tapestry5.ioc;
+
+/**
+ * Part of the test for two different decorate() methods in different classes, but the same name. 
+ * TAP5-1305
+ */
+public class SpecificDecoratorModuleAgain
+{
+    public Greeter decorateHelloGreeter(final Greeter delegate)
+    {
+        return new Greeter()
+        {
+            @Override
+            public String getGreeting()
+            {
+                return delegate.getGreeting() + " again!";
+            }
+        };
+    }
+}