Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing of custom message attributes #69

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Parsing of custom message attributes #69

wants to merge 7 commits into from

Conversation

Judas
Copy link

@Judas Judas commented Feb 12, 2016

Custom arguments are then stored via an extension
@MarcFRICOU
Copy link

👍

@Flowdalic
Copy link
Member

I've replied at https://community.igniterealtime.org/message/255393#255393
Please keep the discussion there. Thank you.

@@ -194,4 +198,17 @@ public static URI getUriFromNextText(XmlPullParser parser) throws XmlPullParserE
return uri;
}

public static Map<String, String> getCustomAttributes(XmlPullParser parser) {
Map<String, String> customAttributes = new LinkedHashMap<>();
List<String> standardAttributes = Arrays.asList(new String[] {"xml:lang", "id", "to", "from", "type"});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why construct this every time the method is called? And why is it a List? This should be a static Set.

@Flowdalic
Copy link
Member

I've just looked at this again, and I think that

    public static Map<String, String> getCustomAttributes(XmlPullParser parser) {
        Map<String, String> customAttributes = new LinkedHashMap<>();
        for (int i = 0; i < parser.getAttributeCount(); i++) {
            String attributeName = parser.getAttributeName(i);
            boolean isLangAttribute = "lang".equals(attributeName) && "xml".equals(parser.getAttributePrefix(i));
            if (!Stanza.REGISTERED_ATTRIBUTES.contains(attributeName) && !isLangAttribute) {
                String attributeValue = parser.getAttributeValue(i);
                customAttributes.put(attributeName, attributeValue);
            }
        }
        return customAttributes;
    }

should take the attribute prefix into account and eventually prefix it. This is untested, but I believe that if we have

<element xmlns='foo' xmlns:myns='https://my.org' attr1='x' myns:attr2='y'>

then the current code would produce a map of

attr1 → x
myns → https://my.org
attr2 → y

while it should produce a map like

attr1 → x
xmlns:myns → https://my.org
myns:attr2 → y

If that's the case, then this should be fixed and a unit test for this should be created.

@Flowdalic
Copy link
Member

Are you still up to pursue this further? Let me know if not, if so I probably squash it myself and add the missing features.

@Judas
Copy link
Author

Judas commented May 31, 2016

Hi Flow,
I didn't took the time to push it but I've added the unit test, and it seems the "good" result is returned by using your example. I'll try to update/push this week.

@FGRibreau
Copy link

FGRibreau commented Oct 19, 2016

We definitely need this 👍!

@IgniteRealtime-Bot
Copy link

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/support-for-custom-top-level-stanza-attributes/81376/2

@IgniteRealtime-Bot
Copy link

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/unable-to-get-message-data/85920/3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants