You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by ppatierno <gi...@git.apache.org> on 2016/11/16 10:08:50 UTC

[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

GitHub user ppatierno opened a pull request:

    https://github.com/apache/qpid-proton/pull/89

    [PROTON-1352] Trivial casting from UnsignedByte to SenderSettleMode and ReceiverSettleMode

    Refactored ReceiverSettleMode and SenderSettleMode for having trivial casting from/to UnsignedByte
    Added message annotations to the String visualization of AMQP message

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

    $ git pull https://github.com/ppatierno/qpid-proton master

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

    https://github.com/apache/qpid-proton/pull/89.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 #89
    
----
commit eb2961096f02f70890aad5b7575cec058b72718d
Author: ppatierno <pp...@live.com>
Date:   2016-11-16T10:03:35Z

    Added message annotations to the String visualization of AMQP message
    Refactored ReceiverSettleMode and SenderSettleMode for having trivial casting from/to UnsignedByte

----


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

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

    https://github.com/apache/qpid-proton/pull/89#discussion_r88248771
  
    --- Diff: proton-j/src/main/java/org/apache/qpid/proton/amqp/transport/SenderSettleMode.java ---
    @@ -25,13 +25,33 @@
     
     import org.apache.qpid.proton.amqp.UnsignedByte;
     
    +import java.util.HashMap;
    +import java.util.Map;
    +
     public enum SenderSettleMode
     {
    -    UNSETTLED, SETTLED, MIXED;
    +    UNSETTLED(0),
    +    SETTLED(1),
    +    MIXED(2);
    +
    +    private UnsignedByte value;
    +    private static Map<UnsignedByte, SenderSettleMode> map = new HashMap<>();
    +
    +    private SenderSettleMode(int value) {
    +        this.value = UnsignedByte.valueOf((byte)value);
    +    }
     
    -    public UnsignedByte getValue()
    -    {
    -        return UnsignedByte.valueOf((byte)ordinal());
    +    static {
    +        for (SenderSettleMode mode: SenderSettleMode.values()) {
    +            map.put(mode.value, mode);
    +        }
         }
     
    +    public SenderSettleMode valueof(UnsignedByte value) {
    +        return map.get(value);
    --- End diff --
    
    As with the other mode.


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-proton issue #89: [PROTON-1352] Trivial casting from UnsignedByte to Se...

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

    https://github.com/apache/qpid-proton/pull/89
  
    Including a message as to why the IAE is thrown wouldn't go amiss :)
    
    I think you missed my comment about raising a separate JIRA for the unrelated annotations change, then including its key in the commit too. Can you use a simple "key(s): message" format for the log, e.g. "PROTON-1234, PROTON-5678: foo" (see the previous commits for more: https://github.com/apache/qpid-proton/commits/master).


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

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

    https://github.com/apache/qpid-proton/pull/89#discussion_r88249149
  
    --- Diff: proton-j/src/main/java/org/apache/qpid/proton/message/impl/MessageImpl.java ---
    @@ -769,6 +769,10 @@ public String toString()
                 sb.append("properties=");
                 sb.append(_properties);
             }
    +        if (_messageAnnotations != null) {
    --- End diff --
    
    This should have its own JIRA. We can skip the separate PR, this time ;)


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

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

    https://github.com/apache/qpid-proton/pull/89


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

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

    https://github.com/apache/qpid-proton/pull/89#discussion_r88259929
  
    --- Diff: proton-j/src/main/java/org/apache/qpid/proton/amqp/transport/ReceiverSettleMode.java ---
    @@ -25,13 +25,32 @@
     
     import org.apache.qpid.proton.amqp.UnsignedByte;
     
    +import java.util.HashMap;
    +import java.util.Map;
    +
     public enum ReceiverSettleMode
     {
    -    FIRST, SECOND;
    +    FIRST(0),
    +    SECOND(1);
    +
    +    private UnsignedByte value;
    +    private static Map<UnsignedByte, ReceiverSettleMode> map = new HashMap<>();
    +
    +    private ReceiverSettleMode(int value) {
    +        this.value = UnsignedByte.valueOf((byte)value);
    +    }
     
    -    public UnsignedByte getValue()
    -    {
    -        return UnsignedByte.valueOf((byte)ordinal());
    +    static {
    +        for (ReceiverSettleMode mode: ReceiverSettleMode.values()) {
    +            map.put(mode.value, mode);
    +        }
         }
     
    +    public static ReceiverSettleMode valueOf(UnsignedByte value) {
    +        return map.get(value);
    --- End diff --
    
    I would probably just make it a switch statement. Of course, you could also use such a switch statement wherever this new method is going to be called from :)


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

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

    https://github.com/apache/qpid-proton/pull/89#discussion_r88262982
  
    --- Diff: proton-j/src/main/java/org/apache/qpid/proton/amqp/transport/ReceiverSettleMode.java ---
    @@ -25,13 +25,32 @@
     
     import org.apache.qpid.proton.amqp.UnsignedByte;
     
    +import java.util.HashMap;
    +import java.util.Map;
    +
     public enum ReceiverSettleMode
     {
    -    FIRST, SECOND;
    +    FIRST(0),
    +    SECOND(1);
    +
    +    private UnsignedByte value;
    +    private static Map<UnsignedByte, ReceiverSettleMode> map = new HashMap<>();
    +
    +    private ReceiverSettleMode(int value) {
    +        this.value = UnsignedByte.valueOf((byte)value);
    +    }
     
    -    public UnsignedByte getValue()
    -    {
    -        return UnsignedByte.valueOf((byte)ordinal());
    +    static {
    +        for (ReceiverSettleMode mode: ReceiverSettleMode.values()) {
    +            map.put(mode.value, mode);
    +        }
         }
     
    +    public static ReceiverSettleMode valueOf(UnsignedByte value) {
    +        return map.get(value);
    --- End diff --
    
    I'm actually fine with adding the method here, which is why I didn't say that originally ;)


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-proton issue #89: [PROTON-1352] Trivial casting from UnsignedByte to Se...

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

    https://github.com/apache/qpid-proton/pull/89
  
    I hadn't realised you had done separate commits...can you add the relevant JIRA key to the first commit, then squash the other two together as a single commit and give it the other key.


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

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

    https://github.com/apache/qpid-proton/pull/89#discussion_r88260293
  
    --- Diff: proton-j/src/main/java/org/apache/qpid/proton/amqp/transport/ReceiverSettleMode.java ---
    @@ -25,13 +25,32 @@
     
     import org.apache.qpid.proton.amqp.UnsignedByte;
     
    +import java.util.HashMap;
    +import java.util.Map;
    +
     public enum ReceiverSettleMode
     {
    -    FIRST, SECOND;
    +    FIRST(0),
    +    SECOND(1);
    +
    +    private UnsignedByte value;
    +    private static Map<UnsignedByte, ReceiverSettleMode> map = new HashMap<>();
    +
    +    private ReceiverSettleMode(int value) {
    +        this.value = UnsignedByte.valueOf((byte)value);
    +    }
     
    -    public UnsignedByte getValue()
    -    {
    -        return UnsignedByte.valueOf((byte)ordinal());
    +    static {
    +        for (ReceiverSettleMode mode: ReceiverSettleMode.values()) {
    +            map.put(mode.value, mode);
    +        }
         }
     
    +    public static ReceiverSettleMode valueOf(UnsignedByte value) {
    +        return map.get(value);
    --- End diff --
    
    It's true but it works in our case just because enum values go from 0,1 to 2 (so they are good as indexes in the array). If one of the value was 100 for example, using value.intValue() as index for the values() array will throw an out of bound exception. Your suggestion was the way I was using outside this enum in my application before opening the PR for doing the cast. :-) 


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

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

    https://github.com/apache/qpid-proton/pull/89#discussion_r88247744
  
    --- Diff: proton-j/src/main/java/org/apache/qpid/proton/amqp/transport/ReceiverSettleMode.java ---
    @@ -25,13 +25,32 @@
     
     import org.apache.qpid.proton.amqp.UnsignedByte;
     
    +import java.util.HashMap;
    +import java.util.Map;
    +
     public enum ReceiverSettleMode
     {
    -    FIRST, SECOND;
    +    FIRST(0),
    +    SECOND(1);
    +
    +    private UnsignedByte value;
    +    private static Map<UnsignedByte, ReceiverSettleMode> map = new HashMap<>();
    +
    +    private ReceiverSettleMode(int value) {
    +        this.value = UnsignedByte.valueOf((byte)value);
    +    }
     
    -    public UnsignedByte getValue()
    -    {
    -        return UnsignedByte.valueOf((byte)ordinal());
    +    static {
    +        for (ReceiverSettleMode mode: ReceiverSettleMode.values()) {
    +            map.put(mode.value, mode);
    +        }
         }
     
    +    public static ReceiverSettleMode valueOf(UnsignedByte value) {
    +        return map.get(value);
    --- End diff --
    
    I think its worth throwing IAE if an out of bound value is provided, rather than just returning null.


---
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.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org