From 104854e399001150622b638e194d732048bbb914 Mon Sep 17 00:00:00 2001
From: simonbrandhof <simon.brandhof@gmail.com>
Date: Tue, 19 Apr 2011 11:55:28 +0200
Subject: [PATCH] SONAR-2278 Improve exception handling of decorators

---
 .../batch/phases/DecoratorsExecutor.java      | 19 +++++++---
 .../batch/phases/DecoratorsExecutorTest.java  | 35 ++++++++++++++++---
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/sonar-batch/src/main/java/org/sonar/batch/phases/DecoratorsExecutor.java b/sonar-batch/src/main/java/org/sonar/batch/phases/DecoratorsExecutor.java
index 4d4607accd6..1605a1623d4 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/phases/DecoratorsExecutor.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/phases/DecoratorsExecutor.java
@@ -27,6 +27,7 @@ import org.sonar.api.batch.DecoratorContext;
 import org.sonar.api.batch.SonarIndex;
 import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
+import org.sonar.api.utils.SonarException;
 import org.sonar.batch.DecoratorsSelector;
 import org.sonar.batch.DefaultDecoratorContext;
 import org.sonar.batch.events.EventBus;
@@ -53,7 +54,7 @@ public class DecoratorsExecutor implements BatchComponent {
     eventBus.fireEvent(new DecoratorsPhaseEvent(Lists.newArrayList(decorators), false));
   }
 
-  private DecoratorContext decorateResource(Resource resource, Collection<Decorator> decorators, boolean executeDecorators) {
+  DecoratorContext decorateResource(Resource resource, Collection<Decorator> decorators, boolean executeDecorators) {
     List<DecoratorContext> childrenContexts = Lists.newArrayList();
     for (Resource child : index.getChildren(resource)) {
       boolean isModule = (child instanceof Project);
@@ -64,12 +65,22 @@ public class DecoratorsExecutor implements BatchComponent {
     DefaultDecoratorContext context = new DefaultDecoratorContext(resource, index, childrenContexts);
     if (executeDecorators) {
       for (Decorator decorator : decorators) {
-        eventBus.fireEvent(new DecoratorExecutionEvent(decorator, true));
-        decorator.decorate(resource, context);
-        eventBus.fireEvent(new DecoratorExecutionEvent(decorator, false));
+        executeDecorator(decorator, context, resource);
       }
     }
     return context;
   }
 
+  void executeDecorator(Decorator decorator, DefaultDecoratorContext context, Resource resource) {
+    try {
+      eventBus.fireEvent(new DecoratorExecutionEvent(decorator, true));
+      decorator.decorate(resource, context);
+      eventBus.fireEvent(new DecoratorExecutionEvent(decorator, false));
+      
+    } catch (Exception e) {
+      // SONAR-2278 the resource should not be lost in exception stacktrace.
+      throw new SonarException("Fail to decorate '" + resource + "'", e);
+    }
+  }
+
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/phases/DecoratorsExecutorTest.java b/sonar-batch/src/test/java/org/sonar/batch/phases/DecoratorsExecutorTest.java
index 396ce2b4e36..fc86a6d4c27 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/phases/DecoratorsExecutorTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/phases/DecoratorsExecutorTest.java
@@ -19,15 +19,27 @@
  */
 package org.sonar.batch.phases;
 
-import static org.hamcrest.number.OrderingComparisons.greaterThanOrEqualTo;
-import static org.hamcrest.number.OrderingComparisons.lessThan;
-import static org.junit.Assert.assertThat;
-
 import org.junit.Test;
+import org.sonar.api.batch.BatchExtensionDictionnary;
 import org.sonar.api.batch.Decorator;
 import org.sonar.api.batch.DecoratorContext;
+import org.sonar.api.batch.SonarIndex;
+import org.sonar.api.resources.File;
 import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
+import org.sonar.api.utils.SonarException;
+import org.sonar.batch.DefaultDecoratorContext;
+import org.sonar.batch.events.EventBus;
+
+import static org.hamcrest.number.OrderingComparisons.greaterThanOrEqualTo;
+import static org.hamcrest.number.OrderingComparisons.lessThan;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+import static org.junit.matchers.JUnitMatchers.containsString;
+import static org.mockito.Matchers.any;
+
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
 
 public class DecoratorsExecutorTest {
 
@@ -49,6 +61,21 @@ public class DecoratorsExecutorTest {
     assertThat(profiler.getMessage().indexOf("Decorator1"), lessThan(profiler.getMessage().indexOf("Decorator2")));
   }
 
+  @Test
+  public void exceptionShouldIncludeResource() {
+    Decorator decorator = mock(Decorator.class);
+    doThrow(new SonarException()).when(decorator).decorate(any(Resource.class), any(DecoratorContext.class));
+
+    DecoratorsExecutor executor = new DecoratorsExecutor(mock(BatchExtensionDictionnary.class), mock(SonarIndex.class), mock(EventBus.class));
+    try {
+      executor.executeDecorator(decorator, mock(DefaultDecoratorContext.class), new File("org/foo/Bar.java"));
+      fail("Exception has not been thrown");
+
+    } catch (SonarException e) {
+      assertThat(e.getMessage(), containsString("org/foo/Bar.java"));
+    }
+  }
+
   static class Decorator1 implements Decorator {
     public void decorate(Resource resource, DecoratorContext context) {
     }
-- 
2.39.5