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

Improve / simplify stanza building code #61

Open
mremond opened this issue Jun 20, 2019 · 5 comments
Open

Improve / simplify stanza building code #61

mremond opened this issue Jun 20, 2019 · 5 comments
Assignees
Milestone

Comments

@mremond
Copy link
Member

mremond commented Jun 20, 2019

Currently, building a stanza can lead to quite lengthy code, like the following:

iqResp := xmpp.NewIQ(xmpp.Attrs{Type: "result", From: iq.To, To: iq.From, Id: iq.Id, Lang: "en"})
identity := xmpp.Identity{
	Name:     opts.Name,
	Category: opts.Category,
	Type:     opts.Type,
}
payload := xmpp.DiscoInfo{
	XMLName: xml.Name{
		Space: xmpp.NSDiscoInfo,
		Local: "query",
	},
	Identity: identity,
	Features: []xmpp.Feature{
		{Var: xmpp.NSDiscoInfo},
		{Var: xmpp.NSDiscoItems},
		{Var: "jabber:iq:version"},
		{Var: "urn:xmpp:delegation:1"},
	},
}
iqResp.Payload = &payload

I am exploring ways to make the code more compact and more guided through helpers (usable with code completin) with a code like this:

b := stanza.NewBuilder().Lang("fr") // Create builder and set default language

iq := b.IQ(stanza.Attrs{Type: "get", To: "service.localhost", Id: "disco-get-1"})

payload := b.DiscoInfo()
identity := b.Identity("Test Component", "gateway", "service")
payload.SetFeatures(stanza.NSDiscoInfo, stanza.NSDiscoItems, "jabber:iq:version", "urn:xmpp:delegation:1").
	SetIdentities(identity)

iq.Payload = payload

What do you think?

@mremond
Copy link
Member Author

mremond commented Jun 20, 2019

@genofire Any opinion on this?
Do you like the prototype API to create packets better ?

@genofire
Copy link
Contributor

unnecessary complicated
for me the source code is easier to read, if it is formated like before.

@mremond
Copy link
Member Author

mremond commented Jun 20, 2019

Thanks for feedback.
I committed my exploration in that branch: https://github.com/FluuxIO/go-xmpp/tree/builder-exploration/pkg/stanza to give you a better idea.

I feel that at some point the code may be difficult to navigate, with the pure parsing / packet formatting part, being mixed with client, component workflow and connection manager.

Separating the stanza building, marshalling and unmarshalling maybe help structure the code better as it grows (and I did not add test modules for all the structure generation, yet).

I think it should still be possible to use the full expending stanza version as well.

Anyway, still thinking about it, as you find this complicated. Maybe I will end up doing something in between (move to a separate package but remove the builder interface).

I am also afraid that godoc will become unusable if we do no split the XML marshalling / unmarshalling part: https://godoc.org/gosrc.io/xmpp

@mremond mremond self-assigned this Jun 21, 2019
@mremond mremond added this to the v0.2.0 milestone Jun 21, 2019
@mremond
Copy link
Member Author

mremond commented Jun 27, 2019

Using pointers, I managed to have a prototype of approach that can further reduce building an IQ with payload, as follows:

iq := stanza.NewIQ(stanza.Attrs{Type: "get", To: "service.localhost", Id: "disco-get-1"})
disco := iq.DiscoInfo()
disco.AddIdentity("Test Component", "gateway", "service")
disco.AddFeatures(stanza.NSDiscoInfo, stanza.NSDiscoItems, "jabber:iq:version", "urn:xmpp:delegation:1")

mremond added a commit that referenced this issue Jun 27, 2019
…nzas


- Move parsing and stanza marshalling / unmarshalling to stanza package
- Add pattern & basic helpers to simplify stanza building.
This was requested on #61
@mremond
Copy link
Member Author

mremond commented Jun 27, 2019

@genofire Several things to note for upgrade:

  1. There is now a stanza package to host parsing and stanza structure. It should help make the project structure more obvious.
  2. On DiscoInfo package, you can have several identities. For future reference, see example 2 in XEP-0030: https://xmpp.org/extensions/xep-0030.html#example-2

Other than that, you are not force to use shortcuts and can keep on relying on building the stanza with structs, as before.

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

No branches or pull requests

2 participants