[jira] [Commented] (CAMEL-4754) The onException clause should make use of the correct logger name given through the log DSL

[Babak Vahdat (Commented) (JIRA)]

    [ 
https://issues.apache.org/jira/browse/CAMEL-4754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13166223#comment-13166223
 ] 

Babak Vahdat commented on CAMEL-4754:
-------------------------------------

@Claus,

I'm afraid that the test ContextScopedOnExceptionCorrectRouteContextTest 
doesn't really verify the fix, as we do swallow all the possible exceptions 
after the template.sendBody() call, so that using the processor DSL even if we 
would assert on:

{code}
assertEquals("balabala", routeId);
{code}

The tests would still pass well! as we swallow all kindes of the exceptions 
after the template.sendBody() calls (including a CamelExecutionException 
wrapping junit.framework.ComparisonFailure)

I went for another approach using a logger-name-recording-log4j-appender (see 
below) and removed the processor approach. If you think this would make more 
sense to you I would append the diff (tomorrow) into this ticket so that you 
can apply it into the trunk.

{code}
public class ContextScopedOnExceptionCorrectRouteContextTest extends 
ContextTestSupport {

    private LoggerNameRecordingAppender loggerNameRecordingAppender;

    @Override
    protected void setUp() throws Exception {
        super.setUp();

        // append our custom log4j Appender through which we want to verify the
        // logger name of the log messages
        loggerNameRecordingAppender = new LoggerNameRecordingAppender("Error 
due Forced foo error", "Error due Forced bar error");
        Logger.getRootLogger().addAppender(loggerNameRecordingAppender);
    }

    @Override
    protected void tearDown() throws Exception {
        // we're done, so remove our custom Appender from the root logger
        Logger.getRootLogger().removeAppender(loggerNameRecordingAppender);

        super.tearDown();
    }

    @Override
    public boolean isUseRouteBuilder() {
        return false;
    }

    public void testContextScopedOnExceptionLogRouteBarFail() throws Exception {
        context.addRoutes(new RouteBuilder() {
            @Override
            public void configure() throws Exception {
                onException(Exception.class)
                    .log("Error due ${exception.message}");

                from("direct:start").routeId("foo")
                    .to("mock:foo")
                    .to("direct:bar")
                    .to("mock:result");

                from("direct:bar").routeId("bar")
                    .to("mock:bar")
                    .throwException(new IllegalArgumentException("Forced bar 
error"));
            }
        });
        context.start();

        getMockEndpoint("mock:foo").expectedMessageCount(1);
        getMockEndpoint("mock:bar").expectedMessageCount(1);
        getMockEndpoint("mock:result").expectedMessageCount(0);
        
        try {
            template.sendBody("direct:start", "Hello World");
            fail("Should have thrown exception");
        } catch (Exception e) {
            // ignore
        }

        assertMockEndpointsSatisfied();

        // assert on the logger name through which we've logged the message
        // using the log dsl
        List<String> barHits = 
loggerNameRecordingAppender.getRecordedLoggerNamesFor("Error due Forced bar 
error");
        assertEquals(1, barHits.size());
        assertEquals("bar", barHits.get(0));

        List<String> fooHits = 
loggerNameRecordingAppender.getRecordedLoggerNamesFor("Error due Forced foo 
error");
        assertEquals(0, fooHits.size());
    }

    public void testContextScopedOnExceptionLogRouteFooFail() throws Exception {
        context.addRoutes(new RouteBuilder() {
            @Override
            public void configure() throws Exception {
                onException(Exception.class)
                    .log("Error due ${exception.message}");

                from("direct:start").routeId("foo")
                    .to("mock:foo")
                    .throwException(new IllegalArgumentException("Forced foo 
error"))
                    .to("direct:bar")
                    .to("mock:result");

                from("direct:bar").routeId("bar")
                    .to("mock:bar");

                from("direct:killer").routeId("killer")
                    .to("mock:killer");
            }
        });
        context.start();

        getMockEndpoint("mock:foo").expectedMessageCount(1);
        getMockEndpoint("mock:bar").expectedMessageCount(0);
        getMockEndpoint("mock:result").expectedMessageCount(0);

        try {
            template.sendBody("direct:start", "Hello World");
            fail("Should have thrown exception");
        } catch (Exception e) {
            // expected
        }

        assertMockEndpointsSatisfied();

        // assert on the logger name through which we've logged the message
        // using the log dsl
        List<String> fooHits = 
loggerNameRecordingAppender.getRecordedLoggerNamesFor("Error due Forced foo 
error");
        assertEquals(1, fooHits.size());
        assertEquals("foo", fooHits.get(0));

        List<String> barHits = 
loggerNameRecordingAppender.getRecordedLoggerNamesFor("Error due Forced bar 
error");
        assertEquals(0, barHits.size());
    }

    private static class LoggerNameRecordingAppender extends AppenderSkeleton {

        private final List<String> recordedLogMessages;
        private final Map<String, List<String>> recordedLoggerNames;

        LoggerNameRecordingAppender(String... logMessages) {
            this.recordedLogMessages = new 
ArrayList<String>(Arrays.asList(logMessages));
            this.recordedLoggerNames = new HashMap<String, List<String>>();
            for (String logMessage : logMessages) {
                recordedLoggerNames.put(logMessage, new ArrayList<String>());
            }
        }

        @Override
        protected void append(LoggingEvent event) {
            String logMessage = event.getRenderedMessage();
            if (recordedLogMessages.contains(logMessage)) {
                // add this hit to the list of the logger names we have got so 
far
                // for this log message
                List<String> actual = recordedLoggerNames.get(logMessage);
                actual.add(event.getLoggerName());
            }
        }

        List<String> getRecordedLoggerNamesFor(String logMessage) {
            return 
Collections.unmodifiableList(recordedLoggerNames.get(logMessage));
        }

        @Override
        public boolean requiresLayout() {
            return false;
        }

        @Override
        public void close() {
            // noop
        }
    }
}
{code}
                
> The onException clause should make use of the correct logger name given 
> through the log DSL 
> --------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-4754
>                 URL: https://issues.apache.org/jira/browse/CAMEL-4754
>             Project: Camel
>          Issue Type: Bug
>          Components: camel-core
>    Affects Versions: 2.8.3
>            Reporter: Babak Vahdat
>            Assignee: Claus Ibsen
>            Priority: Minor
>             Fix For: 2.9.0
>
>         Attachments: debugger.jpg
>
>
> See 
> http://camel.465427.n5.nabble.com/global-onException-clause-wrongly-identifies-route-in-which-exception-occurs-log-name-td5058304.html

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira