You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by vsair <gi...@git.apache.org> on 2017/08/29 04:46:07 UTC

[GitHub] incubator-rocketmq pull request #160: [ROCKETMQ-284] ExpressionMessageFilter...

GitHub user vsair opened a pull request:

    https://github.com/apache/incubator-rocketmq/pull/160

    [ROCKETMQ-284] ExpressionMessageFilter will pass some message.

    https://issues.apache.org/jira/browse/ROCKETMQ-284
    
    Solution:
    
    Keep the tagscode transfer into MessageFilter is legal.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/vsair/incubator-rocketmq ROCKETMQ-284

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-rocketmq/pull/160.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #160
    
----
commit 0c9495f2458259bc419872a727403317de18bfb2
Author: vsair <li...@gmail.com>
Date:   2017-08-29T04:39:09Z

    [ROCKETMQ-284] Keep the tagscode transfer into MessageFilter is legal.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #160: [ROCKETMQ-284] ExpressionMessageFilter will p...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/160
  
    
    [![Coverage Status](https://coveralls.io/builds/13031998/badge)](https://coveralls.io/builds/13031998)
    
    Coverage increased (+0.1%) to 38.755% when pulling **3f78c4e2160f6bb9d8a08934e391b3c81db7b72e on vsair:ROCKETMQ-284** into **4c5e58b46acaf4541fbd741b97593618f027be2d on apache:develop**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #160: [ROCKETMQ-284] ExpressionMessageFilter will p...

Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/160
  
    
    [![Coverage Status](https://coveralls.io/builds/13028196/badge)](https://coveralls.io/builds/13028196)
    
    Coverage increased (+0.02%) to 38.67% when pulling **0c9495f2458259bc419872a727403317de18bfb2 on vsair:ROCKETMQ-284** into **4c5e58b46acaf4541fbd741b97593618f027be2d on apache:develop**.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #160: [ROCKETMQ-284] ExpressionMessageFilter...

Posted by vsair <gi...@git.apache.org>.
Github user vsair closed the pull request at:

    https://github.com/apache/incubator-rocketmq/pull/160


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq issue #160: [ROCKETMQ-284] ExpressionMessageFilter will p...

Posted by vsair <gi...@git.apache.org>.
Github user vsair commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/160
  
    @vongosling  pls have a review.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #160: [ROCKETMQ-284] ExpressionMessageFilter...

Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/160#discussion_r135722707
  
    --- Diff: broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java ---
    @@ -369,4 +367,36 @@ public void testGetMessage_withFilterBitMap() {
                 }
             }
         }
    +
    +    @Test
    +    public void testGetMessage_withFilter_checkTagsCode() {
    +        try {
    +            putMsg(master, topicCount, msgPerTopic);
    +            // sleep to wait for consume queue has been constructed.
    +            Thread.sleep(200);
    +        } catch (Exception e) {
    --- End diff --
    
    Could we using expected  for exception test ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #160: [ROCKETMQ-284] ExpressionMessageFilter...

Posted by vsair <gi...@git.apache.org>.
Github user vsair commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/160#discussion_r135745011
  
    --- Diff: broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java ---
    @@ -369,4 +367,36 @@ public void testGetMessage_withFilterBitMap() {
                 }
             }
         }
    +
    +    @Test
    +    public void testGetMessage_withFilter_checkTagsCode() {
    +        try {
    +            putMsg(master, topicCount, msgPerTopic);
    +            // sleep to wait for consume queue has been constructed.
    --- End diff --
    
    sleep to replace sleep ? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #160: [ROCKETMQ-284] ExpressionMessageFilter...

Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/160#discussion_r135768140
  
    --- Diff: broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java ---
    @@ -79,24 +79,17 @@
             try {
                 StoreHost = new InetSocketAddress(InetAddress.getLocalHost(), 8123);
             } catch (UnknownHostException e) {
    -            e.printStackTrace();
             }
             try {
                 BornHost = new InetSocketAddress(InetAddress.getByName("127.0.0.1"), 0);
             } catch (UnknownHostException e) {
    -            e.printStackTrace();
             }
         }
     
         @Before
    -    public void init() {
    +    public void init() throws Exception {
             filterManager = ConsumerFilterManagerTest.gen(topicCount, msgPerTopic);
    -        try {
    -            master = gen(filterManager);
    -        } catch (Exception e) {
    -            e.printStackTrace();
    -            assertThat(true).isFalse();
    -        }
    --- End diff --
    
    Excellent !


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #160: [ROCKETMQ-284] ExpressionMessageFilter...

Posted by vsair <gi...@git.apache.org>.
Github user vsair commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/160#discussion_r135773045
  
    --- Diff: broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java ---
    @@ -79,24 +79,17 @@
             try {
                 StoreHost = new InetSocketAddress(InetAddress.getLocalHost(), 8123);
             } catch (UnknownHostException e) {
    -            e.printStackTrace();
             }
             try {
                 BornHost = new InetSocketAddress(InetAddress.getByName("127.0.0.1"), 0);
             } catch (UnknownHostException e) {
    --- End diff --
    
    Here,  don't  care about this exception, so not hide it .


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #160: [ROCKETMQ-284] ExpressionMessageFilter...

Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/160#discussion_r135724026
  
    --- Diff: broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java ---
    @@ -369,4 +367,36 @@ public void testGetMessage_withFilterBitMap() {
                 }
             }
         }
    +
    +    @Test
    +    public void testGetMessage_withFilter_checkTagsCode() {
    +        try {
    +            putMsg(master, topicCount, msgPerTopic);
    +            // sleep to wait for consume queue has been constructed.
    --- End diff --
    
    Must we use sleep to replace sleep ? Now ,we have polished our unit test within 3 minutes, thoughts @shroman 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-rocketmq pull request #160: [ROCKETMQ-284] ExpressionMessageFilter...

Posted by vongosling <gi...@git.apache.org>.
Github user vongosling commented on a diff in the pull request:

    https://github.com/apache/incubator-rocketmq/pull/160#discussion_r135768040
  
    --- Diff: broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java ---
    @@ -79,24 +79,17 @@
             try {
                 StoreHost = new InetSocketAddress(InetAddress.getLocalHost(), 8123);
             } catch (UnknownHostException e) {
    -            e.printStackTrace();
             }
             try {
                 BornHost = new InetSocketAddress(InetAddress.getByName("127.0.0.1"), 0);
             } catch (UnknownHostException e) {
    --- End diff --
    
    no need to delete  catch part ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---