Bug 48155 - Multiple problems / enhancements with JMS protocol classes.
Multiple problems / enhancements with JMS protocol classes.
Status: RESOLVED FIXED
Product: JMeter
Classification: Unclassified
Component: Main
2.3.4
PC Mac OS X 10.4
: P2 major (vote)
: ---
Assigned To: JMeter issues mailing list
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2009-11-06 11:35 UTC by martinrl
Modified: 2009-12-10 18:21 UTC (History)
0 users



Attachments
Proposed patches (10.76 KB, patch)
2009-11-06 19:19 UTC, martinrl
Details | Diff
Patches with original source (21.18 KB, application/zip)
2009-11-13 17:13 UTC, martinrl
Details
Proposed patches (19.65 KB, patch)
2009-11-13 19:50 UTC, martinrl
Details | Diff
Proposed patches against current revision (8.61 KB, patch)
2009-11-14 15:26 UTC, martinrl
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description martinrl 2009-11-06 11:35:47 UTC
1. (ENHANCEMENT) Added method "getMessageHeaders()" to BaseJMSSubscriber class to create String representation of the JMS Message Header variables.

    /**
     * Returns a String with the JMS Message Header values.
     * 
     * @param message JMS Message
     * @return String with message header values.
     */
    public static String getMessageHeaders(Message message) {
        final StringBuffer response = new StringBuffer(256);
        try {
            response.append("JMS Message Header Attributes:");
            response.append("\n   Correlation ID: ");
            response.append(message.getJMSCorrelationID());

            response.append("\n   Delivery Mode: ");
            if (message.getJMSDeliveryMode() == DeliveryMode.PERSISTENT) {
                response.append("PERSISTENT");
            } else {
                response.append("NON-PERSISTENT");
            }

            final Destination destination = message.getJMSDestination();

            response.append("\n   Destination: ");
            response.append((destination == null ? null : destination
                .toString()));

            response.append("\n   Expiration: ");
            response.append(new Date(message.getJMSExpiration()));

            response.append("\n   Message ID: ");
            response.append(message.getJMSMessageID());

            response.append("\n   Priority: ");
            response.append(message.getJMSPriority());

            response.append("\n   Redelivered: ");
            response.append(message.getJMSRedelivered());

            final Destination replyTo = message.getJMSReplyTo();
            response.append("\n   Reply to: ");
            response.append((replyTo == null ? null : replyTo.toString()));

            response.append("\n   Timestamp: ");
            response.append(new Date(message.getJMSTimestamp()));

            response.append("\n   Type: ");
            response.append(message.getJMSType());

            response.append("\n\n");
        } catch (JMSException e) {
            e.printStackTrace();
        }
        return new String(response);
    }

2. (DEFECT) SubscriberSampler.sampleWithListener() does not call

   result.setDataType(SampleResult.TEXT);

so the result data is not displayed in the Response Data pane.

3. (DEFECT) Same problem exists with SubscriberSampler.sampleWithReceive() 

4. (ENHANCEMENT) Add call getMessageHeaders() to SubscriberSampler.onMessage() so that JMS Message Header info can be displayed in Response Data pane:

   this.BUFFER.append(getMessageHeaders(message));
   this.BUFFER.append("JMS Message Text:\n\n");
   this.BUFFER.append(content);

5. (DEFECT) ReceiveSubscriber() constructor is using mix of local and instance variable to try to create TopicSession and TopicSubscriber resulting in NPE.  Use the instance variables instead, remove the local variables.

6. (ENHANCEMENT) modify ReceiveSubscriber.run() to add JMS Header to data buffer:

   synchronized (this) {
      this.buffer.append(BaseJMSSampler.getMessageHeaders(message));
      this.buffer.append("JMS Message Text:\n\n");
      this.buffer.append(text);
      count(1);
   }

7 (DEFECT) PublisherSampler.sample() is missing call to result.setDataType(SampleResult.TEXT) and add calls to set the Request Data and Result values:

    public SampleResult sample() {
        SampleResult result = new SampleResult();
        
        // REQUIRED TO DISPLAY IN REQEST / RESPONSE PANES
        result.setDataType(SampleResult.TEXT);
        result.setSampleLabel(getName());
        if (this.PUB == null) {
            this.initClient();
        }
        int loop = this.getIterationCount();
        if (this.PUB != null) {
            result.sampleStart();
            for (int idx = 0; idx < loop; idx++) {
                String tmsg = this.getMessageContent();
                this.PUB.publish(tmsg);
                this.BUFFER.append(tmsg);
            }
            result.sampleEnd();
            String content = this.BUFFER.toString();
            result.setBytes(content.getBytes().length);
            result.setResponseCode("message published successfully");
            result.setResponseMessage(loop + " messages published");
            result.setSuccessful(true);
            // SET RESPONSE PANE DATA TO N/A
            result.setResponseData("Not Applicable".getBytes());
            // SET REQUEST PANE DATA
            result.setSamplerData(content);
            result.setSampleCount(loop);
            this.BUFFER.setLength(0);
        }
        return result;
    }
Comment 1 martinrl 2009-11-06 19:19:13 UTC
Created attachment 24503 [details]
Proposed patches
Comment 2 Sebb 2009-11-13 14:41:29 UTC
Thanks for the suggestions.

Unfortunately they cannot be applied as they stand, as the source code has changed, and the changes are provided as full source files which no longer provide sensible differences.

Please can you provide the changes as patches in unified difference format?

Ideally against the current version in SVN, but even differences from the source you based your changes on would probably be OK.
Comment 3 martinrl 2009-11-13 17:12:23 UTC
I'm a newbie to submitting patch suggestions.  I am attaching a ZIP file with the original and fixed files in separate folders.  If you need me to do more, please let me know.  I want to contribute in whatever way I can.  Thank you for such a great tool!  I am still learning it, but I see it as a very valuable tool, thank you very much!
Comment 4 martinrl 2009-11-13 17:13:50 UTC
Created attachment 24531 [details]
Patches with original source
Comment 5 martinrl 2009-11-13 17:23:00 UTC
I re-read your response, what is a "unified different format"?  Can you point me to a URL explaining how to do this?

The files I provided can be "diff"d against each other.  They are the original 2.3.4 source and the fixed source files.

Please let me know if you want me to do something different.
Comment 6 Sebb 2009-11-13 18:50:47 UTC
(In reply to comment #5)
> I re-read your response, what is a "unified different format"?  Can you point
> me to a URL explaining how to do this?

http://www.apache.org/dev/contributors.html#patches
Comment 7 martinrl 2009-11-13 19:50:25 UTC
Created attachment 24532 [details]
Proposed patches

Thanks for guide on patches.  Here's the resulting patch file.
Comment 8 Sebb 2009-11-14 14:09:36 UTC
(In reply to comment #7)
> Created an attachment (id=24532) [details]
> Proposed patches
> 
> Thanks for guide on patches.  Here's the resulting patch file.

Unfortunately the patch appears to have been made by comparing your version against the current code in trunk. As your changes were not made to the current code, this means that several recent changes would be lost if the patches were applied.

What's needed is a patch file showing exactly what you changed, i.e. the changes need to be against whatever you started with, which I think was 2.3.4?

Alternatively, redo your changes to the current code in trunk, and produce a new patch for that.
Comment 9 martinrl 2009-11-14 15:26:49 UTC
Created attachment 24538 [details]
Proposed patches against current revision

One more time....  Thanks for your patience with a rookie!
Comment 10 martinrl 2009-12-09 14:43:21 UTC
Is there anything else I need to do to get this patch to be in an acceptable format?

(In reply to comment #8)
> (In reply to comment #7)
> > Created an attachment (id=24532) [details] [details]
> > Proposed patches
> > 
> > Thanks for guide on patches.  Here's the resulting patch file.
> 
> Unfortunately the patch appears to have been made by comparing your version
> against the current code in trunk. As your changes were not made to the current
> code, this means that several recent changes would be lost if the patches were
> applied.
> 
> What's needed is a patch file showing exactly what you changed, i.e. the
> changes need to be against whatever you started with, which I think was 2.3.4?
> 
> Alternatively, redo your changes to the current code in trunk, and produce a
> new patch for that.
Comment 11 Sebb 2009-12-10 14:58:50 UTC
The patch is a lot better, but still makes some unnecessary changes, for example:

-    private final TopicConnection CONN;
+    private TopicConnection CONN = null;

and related changes in the static {} block.

It's important that the variable (and others like it) remains final, otherwise it may not be published correctly across threads.

However, I think I can work round those changes; the other changes look OK.
Comment 12 Sebb 2009-12-10 15:02:47 UTC
Applied to SVN:

URL: http://svn.apache.org/viewvc?rev=889456&view=rev
Log:
Bug 48155 - Multiple problems / enhancements with JMS protocol classes

I made one or two other changes, for example responseCode should be "200" for OK, not a text message.

The code will be in nightly builds after r889456 - please re-open the bug if there are any problems.
Comment 13 martinrl 2009-12-10 18:21:30 UTC
My first open source bug patch!  Woohoo!

Thanks for the free lessons.  Hopefully I'll be able to contribute more in the future.

(In reply to comment #12)
> Applied to SVN:
> 
> URL: http://svn.apache.org/viewvc?rev=889456&view=rev
> Log:
> Bug 48155 - Multiple problems / enhancements with JMS protocol classes
> 
> I made one or two other changes, for example responseCode should be "200" for
> OK, not a text message.
> 
> The code will be in nightly builds after r889456 - please re-open the bug if
> there are any problems.