CXF
  1. CXF
  2. CXF-3988

org.apache.cxf.jaxrs.ext.multipart.Attachment object should be mutable

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5.1
    • Fix Version/s: 2.4.6, 2.5.2
    • Component/s: JAX-RS
    • Labels:
      None
    • Estimated Complexity:
      Unknown

      Description

      As currently coded, the headers in an Attachment are immutable after construction. This prevents a construction like:

      • call a convenient constructor
      • add one more header

      Rather than change the contract of getHeaders, I propose to add put/putSingle to the contract of Attachment.

        Activity

        Hide
        Sergey Beryozkin added a comment -

        I'm sorry but I think it's a non-fix, Attachment is used on the server side too so letting add headers to existing Attachments may not be the right thing to do;
        when do you think providing all the headers upfront may be too restrictive ?

        Show
        Sergey Beryozkin added a comment - I'm sorry but I think it's a non-fix, Attachment is used on the server side too so letting add headers to existing Attachments may not be the right thing to do; when do you think providing all the headers upfront may be too restrictive ?
        Hide
        Benson Margulies added a comment -

        When you want to add a content disposition in addition to a content type and an object to map with a provider! Right now, passing in a content disposition forces application/octet-stream.

        I'd be content to add an AttachmentFactory class that obviates the confusing collection of constructors.

        Show
        Benson Margulies added a comment - When you want to add a content disposition in addition to a content type and an object to map with a provider! Right now, passing in a content disposition forces application/octet-stream. I'd be content to add an AttachmentFactory class that obviates the confusing collection of constructors.
        Hide
        Benson Margulies added a comment -

        Sergey,

        In general, a forrest of slightly different ctors is harder to use than a fluid builder. So I'll make an attachment builder, and you can tell me what you think of it.

        --benson

        Show
        Benson Margulies added a comment - Sergey, In general, a forrest of slightly different ctors is harder to use than a fluid builder. So I'll make an attachment builder, and you can tell me what you think of it. --benson
        Hide
        Sergey Beryozkin added a comment - - edited

        Sounds like a good idea indeed;
        Something like

        AttachmentBuilder {
           AttachmentBuilder header(String name, Object value);
           // shortcut for header ("Content-Disposition", cd)
           AttachmentBuilder cd(ContentDisposition cd);   
           AttachmentBuilder data(InputStream is);
           // etc
           Attachment build();
        } 
        

        guess something like that can be handy, it may take a bit of time to make it complete, but I like the idea

        Show
        Sergey Beryozkin added a comment - - edited Sounds like a good idea indeed; Something like AttachmentBuilder { AttachmentBuilder header( String name, Object value); // shortcut for header ( "Content-Disposition" , cd) AttachmentBuilder cd(ContentDisposition cd); AttachmentBuilder data(InputStream is); // etc Attachment build(); } guess something like that can be handy, it may take a bit of time to make it complete, but I like the idea
        Hide
        Benson Margulies added a comment -

        How about what I committed in r1221777 for a start?

        Show
        Benson Margulies added a comment - How about what I committed in r1221777 for a start?
        Hide
        Sergey Beryozkin added a comment -

        Very nice, thanks, I merged to 2.4.x as well

        Show
        Sergey Beryozkin added a comment - Very nice, thanks, I merged to 2.4.x as well
        Hide
        Sergey Beryozkin added a comment -

        Fixed thanks to Benson

        Show
        Sergey Beryozkin added a comment - Fixed thanks to Benson

          People

          • Assignee:
            Unassigned
            Reporter:
            Benson Margulies
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development