XDataFormImpl Bugs

Openfire Openfire Dev
1

Hi All,

I’'ve attached a revised version of the XDataFormImpl class that addresses a couple of bugs in the addItemFields() and asXMLElement() methods.

The addItemFields() was using the Collections.add() method instead of the Collections.addAll() method to populate reportedItems ivar (a mistake I’‘ve made myself a lot ). And, the asXMLElement() wasn’'t building up the item form fields properly.

Thanks,

Ryan
XDataFormImpl.java (8195 Bytes)

2

Hey Ryan,

I’'m not quite sure about these changes. The idea of the reportedItems instance variable is to be a collection that holds “lists of fields”. Each “lists of fields” represent an item that is being returned as part of a search reply.

I think that your changes are assuming that a reported item will have only one field. Whilst according to the JEP it’'s possible that a reported item may have many fields (i.e. columns). Therefore, we should keep lists of fields in reportedItems.

I wrote that code but I never had a chance to test it using searches. Were you having a problem with the current implementation? Do you have a test case that I may use to see the generated XML?

Thanks,

– Gato

3

Hi Gato,

Thanks for looking over my proposed code changes. Unfortunately, I’'m not able to access the code I wrote to exercise the XDataFormImpl class until I get back into the office on Tues. But, I can tell you about the xml I was seeing being returned from Messenger prior to my changes to the XDataFormImpl class.

The xml I was seeing looked like:

<iq type=''result''
    from=''characters.shakespeare.lit''
    to=''juliet@capulet.com/balcony''
    id=''search4''
    xml:lang=''en''>
  <query xmlns=''jabber:iq:search''>
    <x xmlns=''jabber:x:data'' type=''result''>
      <field type=''hidden'' var=''FORM_TYPE''>
        <value>jabber:iq:search</value>
      </field>
      <reported>
        <field var=''name'' label=''Name''/>
      </reported>
      <item>
        <field var=''name''><value>jill</value></field>
        <field var=''name''><value>jack</value></field>
      </item>
  </query>
</iq>

instead of:

<iq type=''result''
    from=''characters.shakespeare.lit''
    to=''juliet@capulet.com/balcony''
    id=''search4''
    xml:lang=''en''>
  <query xmlns=''jabber:iq:search''>
    <x xmlns=''jabber:x:data'' type=''result''>
      <field type=''hidden'' var=''FORM_TYPE''>
        <value>jabber:iq:search</value>
      </field>
      <reported>
        <field var=''name'' label=''Name''/>
      </reported>
      <item>
        <field var=''name''><value>jill</value></field>
      </item>
      <item>
        <field var=''name''><value>jack</value></field>
      </item>
  </query>
</iq>

The code I posted fixed the problem, but as you pointed out, it would appear that multiple fields (columns) wouldn’‘t be handled properly. While it’‘s highly unlikely that the original XDataFormImpl class was working properly and that my test code wasn’‘t correct , I’'ll take a look at all of it again when I return.

Speaking of tests, is there a test suite for Messenger? I didn’'t see much in the way of tests in the project directory.

Thanks,

Ryan

4

Hey Ryan,

I now understand a bit more the problem. It seems that you are calling #addItemFields(ArrayList) once with all the items. What you should need to do is iterate on your array and send a new ArrayList with only one field to #addItemFields. This is so since you need to return several items with only one field and not one item with several fields.

Let me know what you think.

Thanks,

– Gato

5

Hi Gato,

You were right, I was calling #addItemFields(ArrayList) once with all the items.

It could be that my morning coffee hasn’‘t kicked in yet but, now I’‘m running into a problem when I’‘m trying to return multiple fields for the same item; each field seems like it’‘s being treated as it’'s own item.

I’‘ve attached the code that I’‘m using to exercise the XDataFormImpl class so you can see what I’'m talking about.

Thanks,

Ryan
IQSearchHandler.java (6524 Bytes)

6

Hey Ryan,

I found the problem and it’'s been fixed now. You can use the latest code in CVS or wait until the next nightly build. The issue created for this problem is: http://www.jivesoftware.org/issues/browse/JM-136.

Thanks,

– Gato

7

Hi Gato,

Wow! I was going fix that after I got confirmation that it wasn’'t just my code that was the problem, but I guess you beat me to it.

Thanks,

Ryan