Opened 13 years ago

Closed 13 years ago

#28 closed enhancement (fixed)

Add client support for Service Discovery

Reported by: caedes Owned by: tofu
Priority: normal Milestone:
Component: generic Keywords:
Cc: caedes, tofu, ralphm

Description

This patch provides a DiscoClientProtocol?, implementing item and information requests and acompaining unit tests.

Note there is a little overlap with the muc branch, where DiscoQuery? is defined as DiscoRequest?. I think this name is more consistent with others in the file, but the detail is not so important to me either, so it could be named back to DiscoRequest?.

Other considerations... about the request functions, i wasn't very sure if i should return items, the iq, or even filtered items as the pubsub.items() does. I thought filtering them can be counter productive, but found the convenience of returning the items compelling enough as it abstracts away some complexity, so i took this middle alternative. I'm also happy with a different approach.

Attachments (1)

disco.patch (5.5 KB) - added by caedes 13 years ago.
DiscoClientProtocol? implementation and unit tests.

Download all attachments as: .zip

Change History (12)

Changed 13 years ago by caedes

DiscoClientProtocol? implementation and unit tests.

comment:1 Changed 13 years ago by tofu

(In [137]) Creating a branch to apply and review patch. re #28

comment:2 Changed 13 years ago by tofu

(In [138]) adding patch from caedes re #28

comment:3 Changed 13 years ago by tofu

This patch looks ok. The tests pass. I think the tests need comments. I also think the tests need to test the returned items.

I also think the items returned should be a list of wokkel.disco.DiscoItems? instead of a list of domish.Elements

comment:4 Changed 13 years ago by tofu

I retract my statment about the return values, I think domish.Element is fine, since there really is not much more to a DiscoItem? and it very simple. I do not think we should filter.

I also think that the muc branch can be changed to work these changes in. I will leave it for ralph to decide to merge this or not.

comment:5 Changed 13 years ago by ralphm

  • author set to ralphm
  • branch set to branches/disco-client-28-2

(In [143]) Branching to 'disco-client-28-2'

comment:6 Changed 13 years ago by ralphm

  • Cc caedes tofu ralphm added
  • Status changed from new to assigned
  • Summary changed from DiscoClientProtocol xmpphandler patch to Add client support for Service Discovery
  • Type changed from defect to enhancement

I branched forward after closing #7 and am going to work on this before merging.

Besides more docstrings and better tests, I agree with tofu's original statement, We should have generic objects for items, features and entities and maybe a container for info to hold features, entities and Service Discovery Extensions (XEP-0128) forms. Each of these would provide toElement and fromElement methods to render to an Element and parse from an Element respectively. I started this pattern for Data Forms as part of #13 and made objects for publish-subscribe events, too.

comment:7 Changed 13 years ago by ralphm

(In [145]) Implement proper objects to hold service discovery information and items.

This changes the existing objects that derived from domish.Element, to objects in their own right that can be created from an Element using fromElement or rendered to an Element with toElement`.

Re #28.

comment:8 Changed 13 years ago by ralphm

  • Keywords review added
  • Owner changed from ralphm to tofu
  • Status changed from assigned to new

I think my work for providing nice container objects for parsing from Elements and rendering to Elements is done. I modified to the client code to use it. For example requestInfo returns a DiscoInfo object with features, identities and extensions attributes for easy access to the parsed response.

Please review.

comment:9 Changed 13 years ago by tofu

  • Status changed from new to assigned

comment:10 Changed 13 years ago by tofu

  • Keywords review removed

I do not see any reason not to merge this. I will merge it.

comment:11 Changed 13 years ago by tofu

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.