jersey
  1. jersey
  2. JERSEY-1150

Jersey-Multipart: BodyPart and its subclasses don't implement equals or hashCode

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: 1.12
    • Fix Version/s: 1.17
    • Component/s: None
    • Labels:
      None

      Description

      I am using the jersey-multipart module and went to unit test some of my work. I need to pass in some MultiPart objects for test data and the tests failed because it was comparing references rather than the values of the data in the object. I examined the code for BodyPart and its subclasses and noticed that equals and hashCode were not implemented. Seeing as how these objects are data containers/value objects, they should definitely implement these methods.

        Activity

        Hide
        Michal Gajdos added a comment -

        Hi, do you have a simple test-case that you can share with us?

        Show
        Michal Gajdos added a comment - Hi, do you have a simple test-case that you can share with us?
        Hide
        ttmrb added a comment -

        I can't give my actual unit tests as it tests proprietary code but I have created the simplest example to demonstrate the issue.

        public class JerseyMultipartTest {
            @Test
            public void testEquals() {
                InputStream is1 = this.getClass().getResourceAsStream("test-image.jpg");
                MultiPart mp1 = new MultiPart();
                StreamDataBodyPart bodyPart1 = new StreamDataBodyPart("image", is1);
                mp1.bodyPart(bodyPart1);
        
                MultiPart mp2 = new MultiPart();
                InputStream is2 = this.getClass().getResourceAsStream("test-image.jpg");
                StreamDataBodyPart bodyPart2 = new StreamDataBodyPart("image", is2);
                mp1.bodyPart(bodyPart2);
                
                Assert.assertEquals(mp1, mp2);
            }
        }
        

        This test fails with the following message:

        java.lang.AssertionError: expected:<com.sun.jersey.multipart.MultiPart@7854a328> but was:<com.sun.jersey.multipart.MultiPart@7ca3d4cf>
        	at org.junit.Assert.fail(Assert.java:93)
        	at org.junit.Assert.failNotEquals(Assert.java:647)
        	at org.junit.Assert.assertEquals(Assert.java:128)
        	at org.junit.Assert.assertEquals(Assert.java:147)
        	at com.mattbertolini.scratch.JerseyMultipartTest.testEquals(JerseyMultipartTest.java:24)
                ...
        

        Based on the assertion error, its clear that the MultiPart objects are being checked using Object.equals() and checking memory reference.

        Show
        ttmrb added a comment - I can't give my actual unit tests as it tests proprietary code but I have created the simplest example to demonstrate the issue. public class JerseyMultipartTest { @Test public void testEquals() { InputStream is1 = this .getClass().getResourceAsStream( "test-image.jpg" ); MultiPart mp1 = new MultiPart(); StreamDataBodyPart bodyPart1 = new StreamDataBodyPart( "image" , is1); mp1.bodyPart(bodyPart1); MultiPart mp2 = new MultiPart(); InputStream is2 = this .getClass().getResourceAsStream( "test-image.jpg" ); StreamDataBodyPart bodyPart2 = new StreamDataBodyPart( "image" , is2); mp1.bodyPart(bodyPart2); Assert.assertEquals(mp1, mp2); } } This test fails with the following message: java.lang.AssertionError: expected:<com.sun.jersey.multipart.MultiPart@7854a328> but was:<com.sun.jersey.multipart.MultiPart@7ca3d4cf> at org.junit.Assert.fail(Assert.java:93) at org.junit.Assert.failNotEquals(Assert.java:647) at org.junit.Assert.assertEquals(Assert.java:128) at org.junit.Assert.assertEquals(Assert.java:147) at com.mattbertolini.scratch.JerseyMultipartTest.testEquals(JerseyMultipartTest.java:24) ... Based on the assertion error, its clear that the MultiPart objects are being checked using Object.equals() and checking memory reference.
        Hide
        Michal Gajdos added a comment -

        Hi, unfortunately I don't think there is much we can do in this case. It seems to me that if the methods equals and hashCode were implemented correctly they would be too expensive in the runtime in this case. MultiPart object can be rather complex structure and even if the BodyPart's lists of two of these objects were ordered in the same order then there is a case where BodyPart object can contain an entity that does not override equals and hashCode. This would make the MultiPart instances incomparable. Even in the use case you've posted the objects is1 and is2 are different (when is1.equals(is2) is called) and comparing them would mean to read both streams and compare them byte per byte. Considering this I am resolving this issue as 'Invalid'.

        Show
        Michal Gajdos added a comment - Hi, unfortunately I don't think there is much we can do in this case. It seems to me that if the methods equals and hashCode were implemented correctly they would be too expensive in the runtime in this case. MultiPart object can be rather complex structure and even if the BodyPart 's lists of two of these objects were ordered in the same order then there is a case where BodyPart object can contain an entity that does not override equals and hashCode . This would make the MultiPart instances incomparable. Even in the use case you've posted the objects is1 and is2 are different (when is1.equals(is2) is called) and comparing them would mean to read both streams and compare them byte per byte. Considering this I am resolving this issue as 'Invalid'.
        Hide
        ttmrb added a comment -

        First, thank you for taking a look at the issue. It is much appreciated.

        While I am disappointed that this will not be able to be fixed, the explanation given raises some good points. Since I was only encountering the issue in my unit tests, I have coded an appropriate workaround (hooray for testable code! )

        At the very least, we now have a well documented explanation that other developers can now find and understand.

        Thanks again.

        Show
        ttmrb added a comment - First, thank you for taking a look at the issue. It is much appreciated. While I am disappointed that this will not be able to be fixed, the explanation given raises some good points. Since I was only encountering the issue in my unit tests, I have coded an appropriate workaround (hooray for testable code! ) At the very least, we now have a well documented explanation that other developers can now find and understand. Thanks again.

          People

          • Assignee:
            Michal Gajdos
            Reporter:
            ttmrb
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: