someone wants to give a hand on the ICQ service?

Hello Damian,

(This is a reply your last 2 posts)

I finally got a chance to reply, sorry for the delay.

First of all - this is really great stuff. I like your work
a lot and it
comes in at a really good time!

Now let's see and merge these into the current source tree.

More inline:

Damian Minkov wrote:
> After registering the user in the server one
SnacCmdFactory is 
> registered, which handles incoming packages in
registerMyFactory() 
> connection. getInfoService(). getOscarConnection(). 
> getSnacProcessor(). getCmdFactoryMgr().
getDefaultFactoryList(). 
> registerAll( new
net.kano.joscar.icqcmd.DefaultCmdFactory());

Do you need to do this upon initialization or only once the
provider has
connected to the AIM servers? The
ProtocolProviderServiceIcqImpl is
initialized inside the initialize method. Another key point
of the
provider setup is located inside the handleStateChange
method of the
AimConnStateListener inner class of the icq provider. You
can use either
of these to register your factories.

> 1. getShortUserDetails - makes the request
MetaShortInfoRequest (snac
>  15,02 with subtypes 2000, 1210) - adds
SnacRequestAdapter, which 
> handles incoming packest 2. getFullUserDetails - makes
request and 
> again adds response handler (request by snac 15,02 with
subtypes 
> 2000, 1202) when packets that are incoming are sequence
of packets 
> which hold different information there is a request id
which must be 
> different for every request and this is the id which
differs packets 
> when they are coming if more than one request is made
at a one time. 
> There is a structure called FullInfoData which holds
all the data for
> one request.


These will be tricky. Right now we have the ContactIcqImpl
class and it
encapsulates the joust sim Buddy interface. Unfortunately
this buddy
interface does not contain all user details. You'll have to
add to
ContactIcqImpl fields for the details not currently handled
by the buddy
interface but which are returned by the short info messages,
and make
sure you update them when necessary.

I believe the same goes for FullUserInfo, unless you think
that it
contains too much information to put it all inside the
ContactIcqImpl
itself. In this case we should create a separate class to
store them.

There's still the problem of making the info available
through the
serfvice interfaces. The ICQ service contains details which
will not be
supported by all protocols so we could not simply add
getXxx() methods
for every one of them. Storing them in a Map like structure
seems like a
better idea.  The keys to this Map could be specified by an
enumeration
interface in:

net.java.sip.communicator.service.icq

This way no explicit icq support would be required for
bundles that use it
like the UI for example. There would always be the
possibility for
them to visualize all details in a table-like structure by
simply
walking through the map and there would still be a way to
provide
explicit support for icq (nicely drawn panels that represent
all details
in appropriate fields) by other icq specific extensions.

Is this making sense?

> 3. sendOfflineMessages - sends the message with packet 
> SendOfflineImIcbm sends packet as the normal message
but adds at the 
> end extra info - TLV with flag 0x0006 4.
getOfflineMessages - makes 
> request OfflineMsgRequest (snac 15,02 with subtype 60)
- adds 
> response handler - when all offline messages have been
retrieved 
> sends OfflineMsgDeleteRequest to be deleted from server

I used to think that sending offline messages would have to
be
transparent outside the PP implementation. In other words
when calling
the sendMessage() method, the user interface would not have
to care
whether or not the message is to leave in the form of  an
OfflineMsgRequest (15.02) or as a regular message.

Well I don't think so any more. I had forgotton to take
into account the
fact that not all protocols support sending offline messages
and that
the user interface needs to know that in order to warn the
user when for
example they are trying to send a message to an offline MSN
buddy.

One way to deal with this would be to add a boolean method (
sth. like
offlineMsgsSupported() ) inside the basic instant messaging
operation
The method would be consulted before sending messages to
offline buddies
and incoming offline messages would be treated as regular
incoming
messages. I think this would be fine.

Another way to handle it, would be to create a supplementary
OperationSet call sth like OperationSetOfflineMessaging
which would
allow sending offline messages and would also give the user
interface a
means of distinguising between incoming offline and online
msgs (I
wonder whether this is necessary though).

WDYT?

> 2. In most of the packets there is a status value which
Keith Lea 
> skips and the comment is "unknown value"
this is actually a value 
> whether the request is successful and if its not this
is the last 
> byte from the packet. For now I'm managing it as Keith
Lea , just 
> skipping it )) Maybe a
exception to be thrown ? What type of 
> exception ?

Meaning that an outgoing message request has been successful
or not. Is
that it? If this is the case it would then be very useful.
Inside the
OperationSetInstantMessaging, and more precisely in the
instant
messaging listeners, we have two methods called

 
net.java.sip.communicator.service.protocol.event.MessageList
ener.messageDeliveryFailed();
 
net.java.sip.communicator.service.protocol.event.MessageList
ener.messageDelivered();

These are quite important since they give the user interface
the
indication whether a message has been successfully delivered
to its
recipient and could hence be moved to the top, uneditable,
message area,
or whether a failure should be signalled to the user. I am
currently
always firing a message-delivered event because  I couldn't
find a way
inside the joustsim stack to state with certainty that
message was or
was not delivered.

If this flag helps you do so, then it would be just great.

OK, now off to your other mail:

> Now I'm sending the sources of the authorization
issue. Here are 
> solved the following things : 1. When adding the user
and we need his
>  authorization  this is handled (we must send
authorization request 
> in this case) 2. Sending authorization request

I didn't quite catch the difference between 1 and 2. Do
they both
pertain to the case where we're adding to our contact list
a user that
has specified they need authorization in order to be added
to someone
else's contact list? (That's how I understand it anyway.)

Here's how this case should be handled. When the user wants
someone
added to their contact list they'd always do it the same
way, regardless
of whether an authorization will be required or not (in many
cases the
user won't know that before actually trying to add the
user). All this
to say that the authorization process will always be
initiated by the
provider itself, once it has discovered (possibly by
receiving an error
message) that an authorization is needed.

There is an interface inside the protocol package called:

 
net.java.sip.communicator.service.protocol.AuthorizationHand
ler

It will be implemented by the user interface which will set
a valid
instance inside the presence operation set using the
setAuthorizationHandler() method.

Whenever adding a contact to the contact list requires
authorization,
the createAuthorizationRequest() method of this interface
will get
called by the provider. The user interface would on its turn
ask the
user for a reason phrase and whether or not an advance
authorization is
to be granted, and it will then construct an
AuthorizationRequest object
which the provider will use to create the actual
authorization request
message.

> 3. Re-request authorization

Hm ... I don't know about this one. It is very ICQ specific
and not
really a proof of a sound protocol. If possible I'd prefer
it if we
handle it the same way we handle the first request. Would
this be feasible?

> 4. Sending future authorization grant to other user
(this is send in
>  case our account also needs authorization and we are
adding some
> body to our list)

Yes, as I've mentioned above - this would be indicated
inside the
AuthorizationRequest object created by the user interface
and returned
through the createAuthorizationRequest() method.

> 5. Handle incoming Future authorization

We could fire an informative event here, through the
processAuthorizationResponse() method so that the UI would
notify the
user that they could now add the source countact to the
contact list. Do
we need to do something else about it? In the case of icq.
for example,
isn't it the AIM server the one to keep a trace of this
grant so that it
won't return an error next time we try.

> 6. Handle incoming authorization requests (Deny or
Grant)

Would result in a call to the processAuthorizationRequest()
method of
the AuthorizationHandler instance. The UI would then
generate an
AuthorizationReponse upon consulting the user.

> 7.  Handle incoming info for authorization granted or
denied

Would result in a call to processAuthorizationResponse().

> 8. Handle incoming info that user has added us to his
contact list

Another ICQ specific thing. Let's keep this silent for the
time being.

> The last 3 are handled in AuthOldMsgCmd. Here is the
special thing. 
> This type of info is send like a normal message (family
4 , command 
> 7) but this messages has three channels - channel 1 -
plain-text 
> messages - channel 2 - rtf messages, rendezvous -
channel 4 - typed 
> old-style messages , which has very different data in
it like 
> Authorization denied message, Authorization given
message, File 
> request / file ok message,  URL message ..... look at 
> h
ttp://iserverd.khstu.ru/oscar/lists.html#msg_types So I
had to 
> extend the factory creating text messages and to handle
this type of 
> channel 4 messages. So this factory must be inserted in
the service 
> after the original one . So the original will be
overridden.

OK, then I guess that adding it inside the
AimConnStateListener, upon
signon will do the deal.

All right. I guess that's all.

Once again, congratulations for the good job!

Emil

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Emil Ivov wrote:
> Hello Damian,
>
> (This is a reply your last 2 posts)
>
> I finally got a chance to reply, sorry for the delay.
>
> First of all - this is really great stuff. I like your
work a lot and it
> comes in at a really good time!
>
> Now let's see and merge these into the current source
tree.
>
> More inline:
>
> Damian Minkov wrote:
>> After registering the user in the server one
SnacCmdFactory is 
>> registered, which handles incoming packages in
registerMyFactory() 
>> connection. getInfoService(). getOscarConnection().

>> getSnacProcessor(). getCmdFactoryMgr().
getDefaultFactoryList(). 
>> registerAll( new
net.kano.joscar.icqcmd.DefaultCmdFactory());
>
> Do you need to do this upon initialization or only once
the provider has
> connected to the AIM servers? The
ProtocolProviderServiceIcqImpl is
> initialized inside the initialize method. Another key
point of the
> provider setup is located inside the handleStateChange
method of the
> AimConnStateListener inner class of the icq provider.
You can use either
> of these to register your factories.
>
Initialize method in ProtocolProviderServiceIcqImpl is good
point I 
think, as this factories are initialization of added
features and are 
not connected to the state of the connection.
>> 1. getShortUserDetails - makes the request
MetaShortInfoRequest (snac
>>  15,02 with subtypes 2000, 1210) - adds
SnacRequestAdapter, which 
>> handles incoming packest 2. getFullUserDetails -
makes request and 
>> again adds response handler (request by snac 15,02
with subtypes 
>> 2000, 1202) when packets that are incoming are
sequence of packets 
>> which hold different information there is a request
id which must be 
>> different for every request and this is the id
which differs packets 
>> when they are coming if more than one request is
made at a one time. 
>> There is a structure called FullInfoData which
holds all the data for
>> one request.
>
>
> These will be tricky. Right now we have the
ContactIcqImpl class and it
> encapsulates the joust sim Buddy interface.
Unfortunately this buddy
> interface does not contain all user details. You'll
have to add to
> ContactIcqImpl fields for the details not currently
handled by the buddy
> interface but which are returned by the short info
messages, and make
> sure you update them when necessary.
>
> I believe the same goes for FullUserInfo, unless you
think that it
> contains too much information to put it all inside the
ContactIcqImpl
> itself. In this case we should create a separate class
to store them.
>
> There's still the problem of making the info available
through the
> serfvice interfaces. The ICQ service contains details
which will not be
> supported by all protocols so we could not simply add
getXxx() methods
> for every one of them. Storing them in a Map like
structure seems like a
> better idea.  The keys to this Map could be specified
by an enumeration
> interface in:
>
> net.java.sip.communicator.service.icq
>
> This way no explicit icq support would be required for
bundles that 
> use it
> like the UI for example. There would always be the
possibility for
> them to visualize all details in a table-like structure
by simply
> walking through the map and there would still be a way
to provide
> explicit support for icq (nicely drawn panels that
represent all details
> in appropriate fields) by other icq specific
extensions.
>
> Is this making sense?
>
The Map idea is good. The information is too much to put it
in fields. 
As you can see in FullInfoData test class.
But where to put the requests for those info? Or it will be
requested on 
demand?
 Is it better to cache the info somewhere ( this is how I
think is 
handled in other clients and when you request it, it just
get updated).

>> 3. sendOfflineMessages - sends the message with
packet 
>> SendOfflineImIcbm sends packet as the normal
message but adds at the 
>> end extra info - TLV with flag 0x0006 4.
getOfflineMessages - makes 
>> request OfflineMsgRequest (snac 15,02 with subtype
60) - adds 
>> response handler - when all offline messages have
been retrieved 
>> sends OfflineMsgDeleteRequest to be deleted from
server
>
> I used to think that sending offline messages would
have to be
> transparent outside the PP implementation. In other
words when calling
> the sendMessage() method, the user interface would not
have to care
> whether or not the message is to leave in the form of 
an
> OfflineMsgRequest (15.02) or as a regular message.
>
> Well I don't think so any more. I had forgotton to
take into account the
> fact that not all protocols support sending offline
messages and that
> the user interface needs to know that in order to warn
the user when for
> example they are trying to send a message to an offline
MSN buddy.
>
> One way to deal with this would be to add a boolean
method ( sth. like
> offlineMsgsSupported() ) inside the basic instant
messaging operation
> The method would be consulted before sending messages
to offline buddies
> and incoming offline messages would be treated as
regular incoming
> messages. I think this would be fine.
>
> Another way to handle it, would be to create a
supplementary
> OperationSet call sth like OperationSetOfflineMessaging
which would
> allow sending offline messages and would also give the
user interface a
> means of distinguising between incoming offline and
online msgs (I
> wonder whether this is necessary though).
>
> WDYT?
If it is like OperationSet you mean it will be only for the
ICQ PP ? As 
it is common only or mostly for ICQ I think this is the
better decision.
>
>> 2. In most of the packets there is a status value
which Keith Lea 
>> skips and the comment is "unknown
value" this is actually a value 
>> whether the request is successful and if its not
this is the last 
>> byte from the packet. For now I'm managing it as
Keith Lea , just 
>> skipping it )) Maybe a
exception to be thrown ? What type of 
>> exception ?
>
> Meaning that an outgoing message request has been
successful or not. Is
> that it? If this is the case it would then be very
useful. Inside the
> OperationSetInstantMessaging, and more precisely in the
instant
> messaging listeners, we have two methods called
>
>
>
net.java.sip.communicator.service.protocol.event.MessageList
ener.messageDeliveryFailed(); 
>
>
>
net.java.sip.communicator.service.protocol.event.MessageList
ener.messageDelivered(); 
>
>
> These are quite important since they give the user
interface the
> indication whether a message has been successfully
delivered to its
> recipient and could hence be moved to the top,
uneditable, message area,
> or whether a failure should be signalled to the user. I
am currently
> always firing a message-delivered event because  I
couldn't find a way
> inside the joustsim stack to state with certainty that
message was or
> was not delivered.
>
> If this flag helps you do so, then it would be just
great.
Yes this is flag whether the request is successful, I think.
As there is 
no much documentation it is not pointed precisely.
Here is the info : (If success byte doesn't equal 0x0A - it
is last byte 
of the snac. ). Actually  I have not seen till now packet
with this byte 
different from 0x0A.
When there is something wrong the server just don't send us
appropriate 
packet or no packet at all. But we can use this
messageDeliveryFailed()
as it can come one day.
>
> OK, now off to your other mail:
>
>> Now I'm sending the sources of the authorization
issue. Here are 
>> solved the following things : 1. When adding the
user and we need his
>>  authorization  this is handled (we must send
authorization request 
>> in this case) 2. Sending authorization request
>
> I didn't quite catch the difference between 1 and 2.
Do they both
> pertain to the case where we're adding to our contact
list a user that
> has specified they need authorization in order to be
added to someone
> else's contact list? (That's how I understand it
anyway.)
>
> Here's how this case should be handled. When the user
wants someone
> added to their contact list they'd always do it the
same way, regardless
> of whether an authorization will be required or not (in
many cases the
> user won't know that before actually trying to add the
user). All this
> to say that the authorization process will always be
initiated by the
> provider itself, once it has discovered (possibly by
receiving an error
> message) that an authorization is needed.
Yes in 1 is mentioned this error message. You are adding the
buddy as 
usual but when auth is needed an error from the server is
received. This 
is mentioned in 1, this type of error is handled.
And in 2 is actual sending the authorization request.
>
> There is an interface inside the protocol package
called:
>
> 
net.java.sip.communicator.service.protocol.AuthorizationHand
ler
>
> It will be implemented by the user interface which will
set a valid
> instance inside the presence operation set using the
> setAuthorizationHandler() method.
>
> Whenever adding a contact to the contact list requires
authorization,
> the createAuthorizationRequest() method of this
interface will get
> called by the provider. The user interface would on its
turn ask the
> user for a reason phrase and whether or not an advance
authorization is
> to be granted, and it will then construct an
AuthorizationRequest object
> which the provider will use to create the actual
authorization request
> message.
>
>> 3. Re-request authorization
>
> Hm ... I don't know about this one. It is very ICQ
specific and not
> really a proof of a sound protocol. If possible I'd
prefer it if we
> handle it the same way we handle the first request.
Would this be 
> feasible?
This is actually as 2. The difference is that 2 is triggered
after 
response from the server, but this is triggered from the
gui.
>
>> 4. Sending future authorization grant to other user
(this is send in
>>  case our account also needs authorization and we
are adding some
>> body to our list)
>
> Yes, as I've mentioned above - this would be indicated
inside the
> AuthorizationRequest object created by the user
interface and returned
> through the createAuthorizationRequest() method.
>
>> 5. Handle incoming Future authorization
>
> We could fire an informative event here, through the
> processAuthorizationResponse() method so that the UI
would notify the
> user that they could now add the source countact to the
contact list. Do
> we need to do something else about it? In the case of
icq. for example,
> isn't it the AIM server the one to keep a trace of
this grant so that it
> won't return an error next time we try.
Yes this is just info from the AIM server. It really keeps
track of this 
and this was the reason of testing to be so difficult I had
to create 
more accounts )
>
>> 6. Handle incoming authorization requests (Deny or
Grant)
>
> Would result in a call to the
processAuthorizationRequest() method of
> the AuthorizationHandler instance. The UI would then
generate an
> AuthorizationReponse upon consulting the user.
>
>> 7.  Handle incoming info for authorization granted
or denied
>
> Would result in a call to
processAuthorizationResponse().
>
>> 8. Handle incoming info that user has added us to
his contact list
>
> Another ICQ specific thing. Let's keep this silent for
the time being.
>
>> The last 3 are handled in AuthOldMsgCmd. Here is
the special thing. 
>> This type of info is send like a normal message
(family 4 , command 
>> 7) but this messages has three channels - channel 1
- plain-text 
>> messages - channel 2 - rtf messages, rendezvous -
channel 4 - typed 
>> old-style messages , which has very different data
in it like 
>> Authorization denied message, Authorization given
message, File 
>> request / file ok message,  URL message ..... look
at 
>> h
ttp://iserverd.khstu.ru/oscar/lists.html#msg_types So I
had to 
>> extend the factory creating text messages and to
handle this type of 
>> channel 4 messages. So this factory must be
inserted in the service 
>> after the original one . So the original will be
overridden.
>
> OK, then I guess that adding it inside the
AimConnStateListener, upon
> signon will do the deal.
I take a look now the factories are registered in the
constructor of 
OscarConnection. So I think this can also be done in
Initialize method in ProtocolProviderServiceIcqImpl
>
> All right. I guess that's all.
>
> Once again, congratulations for the good job!
>
> Emil
>
>
------------------------------------------------------------
---------
> To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
> For additional commands, e-mail: dev-helpsip-communicator.dev.java.net
>
>

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Hello

Damian Minkov wrote:
> Initialize method in ProtocolProviderServiceIcqImpl is
good point I 
> think, as this factories are initialization of added
features and are
>  not connected to the state of the connection.

All right.

> The Map idea is good. The information is too much to
put it in 
> fields. As you can see in FullInfoData test class. But
where to put 
> the requests for those info? Or it will be requested on
demand? Is it
>  better to cache the info somewhere ( this is how I
think is handled
>  in other clients and when you request it, it just get
updated).

Yes I agree this would be the way to go, but it'll be
better not to
cache inside the protocol provider. I am currently working
on storing
the meta contact list and I think this'll be a better place
to store
this kind of details. In the mean time you could simply
start buy
implementing it as a separate method that would be triggered
by the gui.

> If it is like OperationSet you mean it will be only for
the ICQ PP ?
>  As it is common only or mostly for ICQ I think this is
the better 
> decision.

I didn't express myself clearly here. Since some protocols
support
offline msgs (jabber, icq, yahoo) and others don't (msn,
sip, and even
gmail's jabber server) we need some way of indicating the
UI which do
and which don't, so that it could allow or not the user to
send them.

The two ways of making this would be:
1) create a bool method inside the IM op set, which says -
yes or no, I
support offline msgs. Attempts to send an offline message
where it is
not supported would result in an exception.
2) create a separate operation set named
OperationSetOfflineMessaging
which would only be supported by protocols that support
offline msgs.

The solution in 1) is simple and easy to implement. The one
in 2) would
only make sense if we would like to support setting options
for offline
messages like for example, expiration date or sth. else.

I personally prefer 1) since I don't believe we need fine
grained
control over offline messages.

> Yes this is flag whether the request is successful, I
think. As there
>  is no much documentation it is not pointed precisely.
Here is the 
> info : (If success byte doesn't equal 0x0A - it is
last byte of the 
> snac. ). Actually  I have not seen till now packet with
this byte 
> different from 0x0A. When there is something wrong the
server just 
> don't send us appropriate packet or no packet at all.
But we can use
>  this messageDeliveryFailed() as it can come one day.

Alright ... and we could also add a timer after sending
messages and
announce failure if no response to the request is received
before the
timer triggers. Hm .. I would have thought that this would
be inside
joustsim. Could you have a look at this?

Guess that's it.

Good luck and let me know if you encounter other questions.

Emil

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net

Sending and receiving offline messages is implemented and
commited to CVS.
The calls to the implementation is in 
net.java.sip.communicator.impl.protocol.icq.OperationSetBasi
cInstantMessagingIcqImpl
the helper classes are in package 
net.java.sip.communicator.impl.protocol.icq.message.
The factories for receiving the implemented commands are
registered in  
net.java.sip.communicator.impl.protocol.icq.ProtocolProvider
ServiceIcqImpl.AimConnStateListener.
When the connection is established the all new incoming
command types 
are registered.
Tests are being implemented and will be commited soon.

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net