Bugzilla – Full Text Bug Listing
| Summary: | [BB] Improve quality of bulletin board API | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] ECF | Reporter: | Erkki Lindpere <villane> | ||||||||
| Component: | ecf.core | Assignee: | Erkki Lindpere <villane> | ||||||||
| Status: | RESOLVED WONTFIX | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | caniszczyk, remy.suen, slewis | ||||||||
| Version: | 1.1.0 | Keywords: | helpwanted | ||||||||
| Target Milestone: | 2.1.0 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Erkki Lindpere
Created attachment 62829 [details]
Patch: some API naming and JavaDoc improvements
Please review and/or apply the patch. I renamed a class (IMessageBase -> IBBMessage) and some methods in the API classes, also improved JavaDoc for some classes. Updated the example plugin to replace URL-s with URI-s. A few minor bug fixes. Patches are good, I like patches. IBBMessage getFrom() -> getSender(): 'getFrom()' does not sound grammatically correct. setName(String) -> setTitle(String): If the name is the title, might as well use title in my opinion. setBody(String): Perhaps name the parameter as 'body' instead of 'message'. The message as a whole is the interface itself, the body is merely the content. IForum HOLDS_THREAD: 'This forum can contain threads.' would be good enough in my opinion. I didn't review the other projects' changes since it looks like they're just being because of the API changes. If this is not the case, please identify which ones I should comment on, thanks. getSender() doesn't sound good either, because the action is usually called "posting", not "sending" I think. So "poster" would be even more correct... I have to think about this/look up how it's called in actual forums. I also think having good names is important. I agree about the other points, will update the patch hopefully in 4-5 hours or tomorrow at the latest. (In reply to comment #4) > getSender() doesn't sound good either, because the action is usually called > "posting", not "sending" I think. So "poster" would be even more correct... I > have to think about this/look up how it's called in actual forums. I also think > having good names is important. > > I agree about the other points, will update the patch hopefully in 4-5 hours or > tomorrow at the latest. Sorry I haven't been able to comment sooner...other things impinging. A few comments: >> So what I'm thinking now is: >> a) make IMember extend IUser (from ECF Core) >> b) replace usage of IMember in BBAPI with IUser as much as possible >> c) use IMember only where group membership is concerned >> d) IUser has a properties map -- maybe a property should tell whether a >> user is a guest or not? >> >> Does this sound good? Looking at IUser usage, it currently seems to be >> used exclusively by UI bundles. Remy replied >Don't really like A or B, and as you mentioned, it's not really used >much. Only place I can think of is as a representation of a local user >via IRoster's getUser() method, so it's not really appropriate for >"any user". I definitely will give a -1 D since I'm not a big fan of >the whole properties deal. (Scott) I agree with -1 for D. I'm 0 on A and B...as if you need semantics of having a UI visible name, nickname, and properties associated with an ID (identity within certain namespace), you might as well reuse IUser rather than create something duplicating that association...unless you need different things. Remy wrote: >So I think C is the best route because all members are in a group >(certainly any phpbb, invision, and vbulletin forums I have partaken >in) but guests are not. This (C) also seems reasonable to me. >> 2) The Presence API prefers returning ID-s instead of full objects. For >> example, IIMMessage.getFromID() returns ID instead of some kind of User >> object. Bulletin Board API currently returns full objects in most cases. >> This originates from my own usage of the API -- it was/is mostly used in >> a way that usually expects to easily find the posters name and other >> details about a post. For example some Velocity templates could be >> written to directly reference a message author's name if given an >> IMessageBase object: ${message.from.name}. >> However, if BBAPI usage is to increase, such optimizations may not fit >> all use cases and perhaps should be moved out of the API and left to the >> client code? What do you think? On the other hand, returning ID-s and >> expecting the full objects to be looked up separately if needed makes it >> less object oriented IMO. The ID is meant to convey the *minimum* necessary to uniquely address a given entity (process, user, etc) within a certain Namespace. If you want/require more out of an ID in a given circumstance (e.g. in bbapi) there are basically two paths: 1) use the ID.getAdapter(MyID.class) that Remy describes to query for a certain supported interface at runtime (this is what the UI stuff does with the IChatID interface in presence API) 2) define a subinterface type that has the additional semantics for an ID that you want...e.g. the file transfer API has an 'IFileID' that exposes 'getFilename()' and getURL() for file IDs 2 has the advantage/disadvantage that it's static...it's an advantage because then everyone knows about it (and no runtime query's have to be done). It's a disadvantage (IMHO) because it's relatively inflexible. For the bbAPI it might make sense to use #2 rather than #1. Of course it's also possible to do both, but this might make usage a little confusing. >> 3) Break some of the API-s into parts similar to the presence API's >> I*Manager -s. Not sure yet how exactly I would do this. I like the I*Manager concept, because it makes it relatively easy to partition access to multiple 'parts' of a given API (e.g. chat, chatrooms, admin, roster, history, etc) rather than have everything in one big interface. Also, as functionality is added to an API managers allow such additions to be relatively less disruptive. But, I think Remy is right that they should be used with a grain of salt...as they do add a perhaps-unneeded additional level of abstraction for users of the API. I haven't had a chance yet to look closely at the bbapi in this respect...where were you thinking that managers should/would be introduced? Finally...procedurally...as Erkki makes changes and applies these as patches...what's going to be expectation about applying those patches to the checked-in codebase? I would be happy to do it, but it might be better if we split things up (i.e. sometimes I did it, sometimes Remy, sometimes zx, etc...so that I (Scott) don't hold things up). I'm going to add zx to this bug, in case he is interested in participating in both the review, changes, and in patch application. In any event, Erkki...I suggest that when you have a patch that you want applied/checked in to the codebase you say so in the bug comment...and even direct it to a given person (e.g. Scott, Remy, Chris A). Does this work for you (Erkki)? (In reply to comment #5) > 2) define a subinterface type that has the additional semantics for an ID that > you want...e.g. the file transfer API has an 'IFileID' that exposes > 'getFilename()' and getURL() for file IDs > > 2 has the advantage/disadvantage that it's static...it's an advantage because > then everyone knows about it (and no runtime query's have to be done). It's a > disadvantage (IMHO) because it's relatively inflexible. > > For the bbAPI it might make sense to use #2 rather than #1. Of course it's > also possible to do both, but this might make usage a little confusing. Thanx, I'll need to think about this, but #2 sounds good. > I haven't had a chance yet to look closely at the bbapi in this respect...where > were you thinking that managers should/would be introduced? I was thinking of splitting the IBulletinBoardContainerAdapter functionality into IForumManager and IGroupManager. If I would complete the NNTP provider, it could leave out the IGroupManager completely and only implement forums, because there are no user groups or any public info about users at all, so practically everyone would be a Guest. Also an IPollManager could provide optional support for polls. But since there aren't really that many methods in IBulletinBoardContainerAdapter anyway, maybe splitting it would be overkill. > ... > in patch application. In any event, Erkki...I suggest that when you have a > patch that you want applied/checked in to the codebase you say so in the bug > comment...and even direct it to a given person (e.g. Scott, Remy, Chris A). > Does this work for you (Erkki)? Yes, sounds good. (In reply to comment #5) > I suggest that when you have a > patch that you want applied/checked in to the codebase you say so in the bug > comment... This can get a bit out of hand. Can anyone remind me why we chose to push the api and its implementations into Eclipse.org instead of leaving it where it was or just pushing the api alone? (In reply to comment #7) > (In reply to comment #5) > > I suggest that when you have a > > patch that you want applied/checked in to the codebase you say so in the bug > > comment... > > This can get a bit out of hand. Can anyone remind me why we chose to push the > api and its implementations into Eclipse.org instead of leaving it where it was > or just pushing the api alone? Well, we were only pushing for the IP approval to move to incubation so that bbapi could be IP approved as soon as possible. I suppose we technically didn't have to actually check the code into dev.eclipse.org upon approval, but we did...I think at the time I was expecting that Erkki would be able/want to come on as a committer. If this gets too cumbersome, I suggest that what we do is let Erkki create a separate repository, make changes to that one repository and then we'll apply occasional wholesale changes rather than patches. Erkki I leave it up to you as to whether you want to manage with patches or not...if you decide that you would rather not, the just let us know and we'll make sure things are tagged at dev.eclipse.org...and that the rest of us stay away from making changes simultaneously. Created attachment 63118 [details]
Patch: naming and JavaDoc improvements, migration to Arrays
Created attachment 63119 [details]
mylar/context/zip
Updated the patch according to Remy's comments, also attached Mylar task context, in case you use Mylar too. NB! Also migrated away from lists/collections to arrays! If you think this is ok, and the other changes as well, then please apply the patch. Some more thoughts: 1) Perhaps it would be good to remove the getName() method from IBBObject, and instead provide a common ID interface IBBObjectID with a getName() method. The idea behind this is that usually a combination of id and name is used to identify these objects anyway -- people identify them by the name/title/subject, a computer usually by a unique number. But that would probably leave either duplicate methods (in ID and main object) or roundabout ways of getting to a things name/title/subject: ((IBBObjectID) forum.getID()).getName() 2) "From" vs. "Sender" vs. "Poster": I'm not sure about which terms to use, but it seems posting is even mentioned in dictionary in the context of newsgroups: http://dictionary.reference.com/browse/posting (you may have to scroll down a bit) So I think perhaps "posting", "poster" etc. would be good terms to use, but I would still keep "message" and not rename it to "post" or "posting". 3) I really screwed up somewhere by removing IMessageAuthor and I think it should be brought back (renaming it to IPoster) so that guests and members would be separated again, but still using the correct terms (IMember would extend IPoster). Or perhaps I should just rename IMember to IPoster and each bulletin board could have a (fake or real) "All Users" Group, membership in which tells the difference between members and guests. (In reply to comment #8) > If this gets too cumbersome, I suggest that what we do is let Erkki create a > separate repository, make changes to that one repository and then we'll apply > occasional wholesale changes rather than patches. Erkki I leave it up to you > as to whether you want to manage with patches or not...if you decide that you > would rather not, the just let us know and we'll make sure things are tagged at > dev.eclipse.org...and that the rest of us stay away from making changes > simultaneously. On my part, I don't think it's too cumbersome right now, but it would be fine by me to keep my small changes in a different repository and apply only larger changes at Eclipse.org. I'm still thinking about becoming a committer as well, but haven't made a final decision if I want to do that at the moment. Ok, I can actually see now why patches can be too difficult -- managing multiple patches at the same time would be quite difficult. I commited the latest changes back to http://bbapi.tigris.org/svn/bbapi and if you think it's ok, I will keep commiting smaller changes there and occasionally you can apply them to eclipse.org as Scott suggested. I will also commit the extra stuff to that repository, like the NNTP provider for example (currently very basic). (In reply to comment #11) > Updated the patch according to Remy's comments, also attached Mylar task > context, in case you use Mylar too. > > NB! Also migrated away from lists/collections to arrays! If you think this is > ok, and the other changes as well, then please apply the patch. +1, looks fine to me. Does anyone have anything to add? > 1) Perhaps it would be good to remove the getName() method from IBBObject, and > instead provide a common ID interface IBBObjectID with a getName() method. > > The idea behind this is that usually a combination of id and name is used to > identify these objects anyway -- people identify them by the > name/title/subject, a computer usually by a unique number. But that would > probably leave either duplicate methods (in ID and main object) or roundabout > ways of getting to a things name/title/subject: ((IBBObjectID) > forum.getID()).getName() An ID already has a getName() method which, in this context, is likely to return a different value then calling IBBObject's getName(), I think it's fine the way it is right now. > 2) "From" vs. "Sender" vs. "Poster": I'm not sure about which terms to use, but > it seems posting is even mentioned in dictionary in the context of newsgroups: > http://dictionary.reference.com/browse/posting (you may have to scroll down a > bit) > So I think perhaps "posting", "poster" etc. would be good terms to use, but I > would still keep "message" and not rename it to "post" or "posting". While I agree that 'posting' is the appropriate way to convey the action of adding a message to a thread, I think people generally say "author" instead of "poster" to refer to the person that wrote the message. > 3) I really screwed up somewhere by removing IMessageAuthor and I think it > should be brought back (renaming it to IPoster) so that guests and members > would be separated again, but still using the correct terms (IMember would > extend IPoster). Or perhaps I should just rename IMember to IPoster and each > bulletin board could have a (fake or real) "All Users" Group, membership in > which tells the difference between members and guests. Alternatively, you could have getPoster()/getAuthor() return null to indicate that it's a guest. There is no identifying information that you can use if the person is a guest anyway (well, I guess you might be able to get the IP). By the way, I think IBBMessage should be renamed to IMessage, but I don't know if that's just me. (In reply to comment #14) > adding a message to a thread, I think people generally say "author" instead of > "poster" to refer to the person that wrote the message. Although "Author" is good as well, I am personally a tiny bit more happy with IPoster right now, so I would keep that. > Alternatively, you could have getPoster()/getAuthor() return null to indicate > that it's a guest. There is no identifying information that you can use if the > person is a guest anyway (well, I guess you might be able to get the IP). Actually, there may be some info for a guest: some boards allow guests to enter names as well, and it's possible that some boards may allow other information in the future -- like maybe avatar images, which could be persisted through sessions without the user having to register. At the moment I made IMember extend IPoster. IPoster has only one method #isGuest() and will be used instead of IMember where appropriate. The code is currently in the bbapi.tigris.org repo, but I want to make some more small improvements and then post a patch here, I think. > By the way, I think IBBMessage should be renamed to IMessage, but I don't know > if that's just me. I would keep IBBMessage, similarly to IIMMessage, both are base interfaces for different kinds of messages. Reassiging to Erkki and changing version target to 1.1.0 Just letting you know, there should be some updates coming soon as I've already commited some changes to the tigris.org repository and more improvements should coming as I now actually have deployed a live service based on this and bugs and faults will come out (I'll tell more about it in a blog entry or in the ECF mailing list). Setting target milestone to 1.1.0 and changing severity. Would 1.1.0 work for you Erkki? Sorry for the delay in responding. I was mostly away from Eclipse stuff due to a busy period in my day job. That period still continues actually, but I will try to do this for 1.1.0. Erkki any progress on this? I would like to try to get bbapi into ECF distribution in 1.2 timeframe. Setting target milestone. Sorry, I haven't followed the ecf-dev list closely in the last month or so, what time is 1.2 planned for? I have not made any progress, but I intend to resume work on bbapi next week or the week after (some time in mid September). (In reply to comment #21) > Sorry, I haven't followed the ecf-dev list closely in the last month or so, > what time is 1.2 planned for? I have not made any progress, but I intend to > resume work on bbapi next week or the week after (some time in mid September). > Hi Erkki. Here's the current plan/schedule: http://wiki.eclipse.org/ECF_Ganymede_Roadmap Don't mean to be a nag, but I would like to get the latest of bb apis into ECF distribution. setting target milestone to 2.1 I'm continuing the work on this, but I would like to make some bigger changes: 1) rename org.eclipse.ecf.bulletinboard.commons -> org.eclipse.ecf.bulletinboard.web Since this contains helpers/ base classes that can be used to create HTTP + HTML scraping based providers, I think "web" describes it bettern than "commons". Also, all of this bundle used to be internal, but actually I think it should be *provisional* API, because it contains useful classes that implementors would probably want to use. I should also improve the overall quality of this package to make it more suitable as API for implementors. 2) reduce scope of the API (ecf.bulletinboard bundle). Being realistic, I don't have the time to implement all of it in the providers. The scope reduction would not be that big. I would remove: - Polls API, this leaves the following types of objects to deal with: forums, threads, messages, members, member groups, private messages - Distinguishing between read only and writable objects (not very useful in hindsight, and was never implemented properly) - Additional attributes on threads (subscription status, type) and possibly some minor details from other objects. - IBBCredentials interface and IBulletinBoardContainerAdapter's login/logout methods - there is no real need for this to be client-oriented API, I would move it to the commons/web bundle as a concrete class that simplifies implementations (reading the username/password from connect context). Is that ok? (In reply to comment #24) > I'm continuing the work on this, but I would like to make some bigger changes: > > 1) rename org.eclipse.ecf.bulletinboard.commons -> > org.eclipse.ecf.bulletinboard.web > > Since this contains helpers/ base classes that can be used to create HTTP + > HTML scraping based providers, I think "web" describes it bettern than > "commons". Also, all of this bundle used to be internal, but actually I think > it should be *provisional* API, because it contains useful classes that > implementors would probably want to use. > > I should also improve the overall quality of this package to make it more > suitable as API for implementors. > > 2) reduce scope of the API (ecf.bulletinboard bundle). > > Being realistic, I don't have the time to implement all of it in the providers. > The scope reduction would not be that big. I would remove: > - Polls API, this leaves the following types of objects to deal with: forums, > threads, messages, members, member groups, private messages > - Distinguishing between read only and writable objects (not very useful in > hindsight, and was never implemented properly) > - Additional attributes on threads (subscription status, type) and possibly > some minor details from other objects. > - IBBCredentials interface and IBulletinBoardContainerAdapter's login/logout > methods - there is no real need for this to be client-oriented API, I would > move it to the commons/web bundle as a concrete class that simplifies > implementations (reading the username/password from connect context). > > Is that ok? > Yes. I have some comments about the above, but in general this sounds reasonable to me. One possibility. We have access to a CVS server at ecf1.osuosl.org, and could give you (Erkki) read-write access. Then work on the nbb APIs could proceed there, and most of the ECF committers would have access...and we could avoid the difficulties of doing things through patches (especially the larger-scale changes that you identify above). How would this be? A couple of comments below: > 1) rename org.eclipse.ecf.bulletinboard.commons -> > org.eclipse.ecf.bulletinboard.web > > Since this contains helpers/ base classes that can be used to create HTTP + > HTML scraping based providers, I think "web" describes it bettern than > "commons". Also, all of this bundle used to be internal, but actually I think > it should be *provisional* API, because it contains useful classes that > implementors would probably want to use. > Both sound right to me (name change of bundle as well as provisional API. The convention to use for the provisional API is to name the packages containing provisional API like the following: org.eclipse.ecf.internal.provisional.package.name the 'internal' part is so that the PDE will insert the proper markup in the manifest.mf to warn users using these internal APIs. The 'provisional' part is obviously to indicate that that status might/will change. (In reply to comment #25) > One possibility. We have access to a CVS server at ecf1.osuosl.org, and could > give you (Erkki) read-write access. Then work on the nbb APIs could proceed > there, and most of the ECF committers would have access...and we could avoid > the difficulties of doing things through patches (especially the larger-scale > changes that you identify above). How would this be? That would be fine to me. I commited the initial version of the refactored API, web-based bulletin board implementation support bundle and the phpBB 2 provider. The web-based stuff uses generics and requires Java 1.5. The bulletinboard.web bundle is now a lot cleaner, I think. It is divided into 4 packages: web -- some abstract and concrete implementations for IBBObject, IContainer and IBulletinBoardContainerAdapter web.identity.uri -- URI based ID's, with URI Descriptors (describe the form of an URI and validate them), namespace implementation based on these ID's web.parsing.regex -- helpers for parsing HTML using regular expressions web.request.http -- wrappers for http client methods to support a more uniform API The phpBB 2 provider does not yet support all features of the API. Missing are: private messaging and member group membership checking. I will implement these later. I will now start working on refactoring the vBulletin and NNTP providers. Oops... forgot to mention. I commited the plugins to this location: ecf1.osuosl.org:/home/cvs/ecf/bbapi Should we cancel this or leave in helpwanted state? There doesn't seem to be enough demand for this and my own project that uses BBAPI is also not one of my priorities at the moment (and likely will not be). Also, the world has changed and I no longer really believe something good/lasting can be built on screen-scraping based approaches. It seems outdated in an age where many service providers provide APIs for programmatic acces and mash-ups. This doesn't seem to have happened with forums, but if there would be enough value in it, it should happen. There is a move afoot to create an rcp newsreader and have it live in the ECF project: http://dev.eclipse.org/mhonarc/lists/ecf-dev/msg02530.html It might be appropriate to have this work (bb api) move in those directions. Regrettably, there are no ECF resources for further work...or even maintenance...on the ECF bulletin board API...and it seems likely that we will have to discontinue work on it unless more contributions can be made. If resources and interest do become available, please reopen. |