Re: svn commit: r1360581 - /camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java

[Claus Ibsen-2]

Hi Willem

Would it be possible to add an unit test that demonstrates this issue?
Its not really clear from the code changes + commit message, why this
change was needed.

And having an unit test helps against regressions.
And the mock endpoint is heavily used, so we should be careful when
changing existing code in it.


On Thu, Jul 12, 2012 at 10:49 AM,  <[hidden email]> wrote:

> Author: ningjiang
> Date: Thu Jul 12 08:49:14 2012
> New Revision: 1360581
>
> URL: http://svn.apache.org/viewvc?rev=1360581&view=rev
> Log:
> CAMEL-5440 Fixed the issue of mock endpoint expectedHeaderReceived
>
> Modified:
>     camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java
>
> Modified: camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java
> URL: http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java?rev=1360581&r1=1360580&r2=1360581&view=diff
> ==============================================================================
> --- camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java (original)
> +++ camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java Thu Jul 12 08:49:14 2012
> @@ -481,32 +481,32 @@ public class MockEndpoint extends Defaul
>      public void expectedHeaderReceived(final String name, final Object value) {
>          if (expectedHeaderValues == null) {
>              expectedHeaderValues = new CaseInsensitiveMap();
> -        }
> -        expectedHeaderValues.put(name, value);
> +            // we just wants to expects to be called once
> +            expects(new Runnable() {
> +                public void run() {
> +                    for (int i = 0; i < getReceivedExchanges().size(); i++) {
> +                        Exchange exchange = getReceivedExchange(i);
> +                        for (Map.Entry<String, Object> entry : expectedHeaderValues.entrySet()) {
> +                            String key = entry.getKey();
> +                            Object expectedValue = entry.getValue();
> +
> +                            // we accept that an expectedValue of null also means that the header may be absent
> +                            if (expectedValue != null) {
> +                                assertTrue("Exchange " + i + " has no headers", exchange.getIn().hasHeaders());
> +                                boolean hasKey = exchange.getIn().getHeaders().containsKey(key);
> +                                assertTrue("No header with name " + key + " found for message: " + i, hasKey);
> +                            }
>
> -        expects(new Runnable() {
> -            public void run() {
> -                for (int i = 0; i < getReceivedExchanges().size(); i++) {
> -                    Exchange exchange = getReceivedExchange(i);
> -                    for (Map.Entry<String, Object> entry : expectedHeaderValues.entrySet()) {
> -                        String key = entry.getKey();
> -                        Object expectedValue = entry.getValue();
> +                            Object actualValue = exchange.getIn().getHeader(key);
> +                            actualValue = extractActualValue(exchange, actualValue, expectedValue);
>
> -                        // we accept that an expectedValue of null also means that the header may be absent
> -                        if (expectedValue != null) {
> -                            assertTrue("Exchange " + i + " has no headers", exchange.getIn().hasHeaders());
> -                            boolean hasKey = exchange.getIn().getHeaders().containsKey(key);
> -                            assertTrue("No header with name " + key + " found for message: " + i, hasKey);
> +                            assertEquals("Header with name " + key + " for message: " + i, expectedValue, actualValue);
>                          }
> -
> -                        Object actualValue = exchange.getIn().getHeader(key);
> -                        actualValue = extractActualValue(exchange, actualValue, expectedValue);
> -
> -                        assertEquals("Header with name " + key + " for message: " + i, expectedValue, actualValue);
>                      }
>                  }
> -            }
> -        });
> +            });
> +        }
> +        expectedHeaderValues.put(name, value);
>      }
>
>      /**
>
>

--
Claus Ibsen
-----------------
FuseSource
Email: [hidden email]
Web: http://fusesource.com
Twitter: davsclaus, fusenews
Blog: http://davsclaus.com
Author of Camel in Action: http://www.manning.com/ibsen

[Willem.Jiang]

The issue is found when I was working on CAMEL-5439. If you revert the
change the tests of camel-ssh will failed.
I will committed an unit test for it.

On Fri Jul 13 14:21:11 2012, Claus Ibsen wrote:

> Hi Willem
>
> Would it be possible to add an unit test that demonstrates this issue?
> Its not really clear from the code changes + commit message, why this
> change was needed.
>
> And having an unit test helps against regressions.
> And the mock endpoint is heavily used, so we should be careful when
> changing existing code in it.
>
>
> On Thu, Jul 12, 2012 at 10:49 AM,  <[hidden email]> wrote:
>> Author: ningjiang
>> Date: Thu Jul 12 08:49:14 2012
>> New Revision: 1360581
>>
>> URL: http://svn.apache.org/viewvc?rev=1360581&view=rev
>> Log:
>> CAMEL-5440 Fixed the issue of mock endpoint expectedHeaderReceived
>>
>> Modified:
>>      camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java
>>
>> Modified: camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java
>> URL: http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java?rev=1360581&r1=1360580&r2=1360581&view=diff
>> ==============================================================================
>> --- camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java (original)
>> +++ camel/trunk/camel-core/src/main/java/org/apache/camel/component/mock/MockEndpoint.java Thu Jul 12 08:49:14 2012
>> @@ -481,32 +481,32 @@ public class MockEndpoint extends Defaul
>>       public void expectedHeaderReceived(final String name, final Object value) {
>>           if (expectedHeaderValues == null) {
>>               expectedHeaderValues = new CaseInsensitiveMap();
>> -        }
>> -        expectedHeaderValues.put(name, value);
>> +            // we just wants to expects to be called once
>> +            expects(new Runnable() {
>> +                public void run() {
>> +                    for (int i = 0; i < getReceivedExchanges().size(); i++) {
>> +                        Exchange exchange = getReceivedExchange(i);
>> +                        for (Map.Entry<String, Object> entry : expectedHeaderValues.entrySet()) {
>> +                            String key = entry.getKey();
>> +                            Object expectedValue = entry.getValue();
>> +
>> +                            // we accept that an expectedValue of null also means that the header may be absent
>> +                            if (expectedValue != null) {
>> +                                assertTrue("Exchange " + i + " has no headers", exchange.getIn().hasHeaders());
>> +                                boolean hasKey = exchange.getIn().getHeaders().containsKey(key);
>> +                                assertTrue("No header with name " + key + " found for message: " + i, hasKey);
>> +                            }
>>
>> -        expects(new Runnable() {
>> -            public void run() {
>> -                for (int i = 0; i < getReceivedExchanges().size(); i++) {
>> -                    Exchange exchange = getReceivedExchange(i);
>> -                    for (Map.Entry<String, Object> entry : expectedHeaderValues.entrySet()) {
>> -                        String key = entry.getKey();
>> -                        Object expectedValue = entry.getValue();
>> +                            Object actualValue = exchange.getIn().getHeader(key);
>> +                            actualValue = extractActualValue(exchange, actualValue, expectedValue);
>>
>> -                        // we accept that an expectedValue of null also means that the header may be absent
>> -                        if (expectedValue != null) {
>> -                            assertTrue("Exchange " + i + " has no headers", exchange.getIn().hasHeaders());
>> -                            boolean hasKey = exchange.getIn().getHeaders().containsKey(key);
>> -                            assertTrue("No header with name " + key + " found for message: " + i, hasKey);
>> +                            assertEquals("Header with name " + key + " for message: " + i, expectedValue, actualValue);
>>                           }
>> -
>> -                        Object actualValue = exchange.getIn().getHeader(key);
>> -                        actualValue = extractActualValue(exchange, actualValue, expectedValue);
>> -
>> -                        assertEquals("Header with name " + key + " for message: " + i, expectedValue, actualValue);
>>                       }
>>                   }
>> -            }
>> -        });
>> +            });
>> +        }
>> +        expectedHeaderValues.put(name, value);
>>       }
>>
>>       /**
>>
>>
>
>
>

--
Willem
----------------------------------
FuseSource
Web: http://www.fusesource.com
Blog:    http://willemjiang.blogspot.com (English)
         http://jnn.javaeye.com (Chinese)
Twitter: willemjiang
Weibo: willemjiang