Summary

Change the way accounts connect while keeping backwards compatibility

Review Request #1967 — Created Oct. 26, 2022 and submitted Nov. 1, 2022, 3:54 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

Blocks

2003

Reviewers

Groups

pidgin

People

Description

This adds a new method to PurpleProtocol named create_connection which allows

a protocol to return a subclassed PurpleConnection so it can avoid using

purple_connection_[gs]et_protocol_data. The default implementation for this

method, just creates a PurpleConnection with the given protocol, account,

and password.
This also adds two new methods to PurpleConnection; connect and

disconnect. The default implementation for these two just call

purple_protocol_login and purple_protocol_close respectively.
The long term intent of this, is to have all protocol plugins subclass

PurpleConnection and then remove the login and close methods from

PurpleProtocol and also remove purple_connection_[gs]et_protocol_data.

Testing Done

Connected the demo account and used the account actions to test reconnecting.
Connected an XMPP account after running and at startup.
Verified that the infinite loop bug on error due to connection failure is now fixed as well.

Commits

Summary	ID
Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	9c59801d841a6dd7ef90fd1c51978ea7beb0b181

Summary

Change the way accounts connect while keeping backwards compability

This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.

9c59801d841a6dd7ef90fd1c51978ea7beb0b181

Issues

Description	From	Last Updated
Shouldn't you check error here? It'll leak if this returns early and is set.	QuLogic	Oct. 29, 2022, 9:40 a.m.
%TRUE if	QuLogic	Oct. 29, 2022, 9:40 a.m.
Since _purple_connection_new is gone, I guess this comment can go too?	QuLogic	Oct. 31, 2022, 8:15 a.m.
Not sure, should we set an error here?	QuLogic	Oct. 29, 2022, 9:41 a.m.
Maybe here also?	QuLogic	Oct. 29, 2022, 9:41 a.m.
Should this not be cleared in dispose now? I suppose it might happen to work because disconnect will set it …	QuLogic	Nov. 1, 2022, 3:50 a.m.
Do you want to document the default disconnect here, as done for connect?	QuLogic	Nov. 1, 2022, 2:54 a.m.
Extra space.	QuLogic	Nov. 1, 2022, 2:55 a.m.
This condition is always true, because you're checking it after purple_connnection_set_state(connection, PURPLE_CONNECTION_STATE_DISCONNECTING).	QuLogic	Nov. 1, 2022, 2:55 a.m.
Isn't the protocol's close method called in klass->disconnect? But that method call's now after the buddy iteration, when it was …	QuLogic	Nov. 1, 2022, 2:55 a.m.

Description:: ~
This adds a new method to PurpleProtocol named create_connection which allows
~
This adds a new method to PurpleProtocol named create_connection which allows
a protocol to return a subclassed PurpleConnection so it can avoid using
purple_connection_[gs]et_protocol_data. The default implementation for this
method, just creates a PurpleConnection with the given protocol, account,
and password.

This also adds two new methods to PurpleConnection; connect and
disconnect. The default implementation for these two just call
purple_protocol_login and purple_protocol_close respectively.

The long term intent of this, is to have all protocol plugins subclass
PurpleConnection and then remove the login and close methods from
PurpleProtocol and also remove purple_connection_[gs]et_protocol_data.

Change Summary:

rebased

Commits:

	Summary	ID
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	ebfa83be1e07e0269e865a5ea88c5064eaaa48f3
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	0140ca1b9749201fd3056df0c30154150b82ce08

Diff:

Revision 2 (+684 -358)

Show changes

	libpurple/account.c
	libpurple/connection.h
	libpurple/connection.c
	libpurple/purpleprivate.h
	libpurple/purpleprotocol.h
	libpurple/purpleprotocol.c

Change Summary:

Fix the infinite loop on exit after a connection entered a failed state.

Testing Done:

		Connected the demo account and used the account actions to test reconnecting.
		Connected an XMPP account after running and at startup.
~		Verified that the infinite loop bug on error due to connection failure is unforunately, still present.
	~	Verified that the infinite loop bug on error due to connection failure is now fixed as well.

Commits:

	Summary	ID
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	0140ca1b9749201fd3056df0c30154150b82ce08
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	e286b383057a498d95c1974853083a9583f9bcb5

Diff:

Revision 3 (+710 -358)

Show changes

	libpurple/account.c
	libpurple/connection.h
	libpurple/connection.c
	libpurple/purpleprivate.h
	libpurple/purpleprotocol.h
	libpurple/purpleprotocol.c

Change Summary:: typo fix
Summary:: Change the way accounts connect while keeping backwards compability
Change the way accounts connect while keeping backwards compatibility

libpurple/account.c (Diff revision 3)

The issue has been resolved. Show all issues

Shouldn't you check error here? It'll leak if this returns early and is set.

libpurple/connection.h (Diff revision 3)
The issue has been resolved. Show all issues
```
%TRUE if
```
libpurple/connection.c (Diff revision 3)
The issue has been resolved. Show all issues
```
Since _purple_connection_new is gone, I guess this comment can go too?
```
1. grim Oct. 30, 2022, 8:29 a.m.
  Did I miss something here?
libpurple/connection.c (Diff revision 3)
The issue has been resolved. Show all issues
```
Not sure, should we set an error here?
```
libpurple/purpleprotocol.c (Diff revision 3)
The issue has been resolved. Show all issues
```
Maybe here also?
```

Change Summary:

address issues and better handle failed connections

Commits:

	Summary	ID
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	e286b383057a498d95c1974853083a9583f9bcb5
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	a77bd5ca8d0a3f5948889c174687127b583d7303

Diff:

Revision 4 (+776 -360)

Show changes

	libpurple/account.c
	libpurple/connection.h
	libpurple/connection.c
	libpurple/purpleprivate.h
	libpurple/purpleprotocol.h
	libpurple/purpleprotocol.c

libpurple/account.c (Diff revision 4)

The issue has been resolved. Show all issues

Should this not be cleared in dispose now?

I suppose it might happen to work because disconnect will set it to NULL.

grim Oct. 31, 2022, 8:31 a.m.

Accounts out live connections so we need to be able to set it to null outside of dispose.

QuLogic Nov. 1, 2022, 2:59 a.m.

Yes, but we should still clear it in dispose just in case something really weird happens, or if we happen to change the lifetimes later (e.g., maybe with PurpleConnectionManager?).

libpurple/connection.h (Diff revision 4)
The issue has been resolved. Show all issues
```
Do you want to document the default disconnect here, as done for connect?
```
libpurple/connection.c (Diff revision 4)
The issue has been resolved. Show all issues
```
Extra space.
```

libpurple/connection.c (Diff revision 4)

The issue has been resolved. Show all issues

This condition is always true, because you're checking it after purple_connnection_set_state(connection, PURPLE_CONNECTION_STATE_DISCONNECTING).

libpurple/connection.c (Diff revision 4)

The issue has been resolved. Show all issues

Isn't the protocol's close method called in klass->disconnect? But that method call's now after the buddy iteration, when it was previously before it in the old code that called purple_protocol_close directly.

Change Summary:

rebase and address issues

Commits:

	Summary	ID
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	a77bd5ca8d0a3f5948889c174687127b583d7303
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	65c2af0eaf070ecf974a6284d8e1036406aed470

Diff:

Revision 5 (+782 -360)

Show changes

	libpurple/account.c
	libpurple/connection.h
	libpurple/connection.c
	libpurple/purpleprivate.h
	libpurple/purpleprotocol.h
	libpurple/purpleprotocol.c

Change Summary:

explicitly clear the connection in purple_account_dispose

Commits:

	Summary	ID
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	65c2af0eaf070ecf974a6284d8e1036406aed470
	Change the way accounts connect while keeping backwards compability This adds a new method to PurpleProtocol named create_connection which allows a protocol to return a subclassed PurpleConnection so it can avoid using purple_connection_set_protocol_[gs]et_protocol_data. The default implementation for this method, just create a PurpleConnection with the given protocol, account, and password. This also adds two new methods to PurpleConnection; connect and disconnect. The default implementation for these two just call purple_protocol_login and purple_protocol_close respectively. The long term intent of this, is to have all protocol plugins subclass PurpleConnection and then remove the login and close methods from PurpleProtocol as well as remove purple_connection_[gs]et_protocol_data.	9c59801d841a6dd7ef90fd1c51978ea7beb0b181

Diff:

Revision 6 (+784 -360)

Show changes

	libpurple/account.c
	libpurple/connection.h
	libpurple/connection.c
	libpurple/purpleprivate.h
	libpurple/purpleprotocol.h
	libpurple/purpleprotocol.c

Ship it!

```
Ship It!
```

Status:: Completed

~		This adds a new method to PurpleProtocol named `create_connection` which allows
	~	This adds a new method to `PurpleProtocol` named `create_connection` which allows
		a protocol to return a subclassed `PurpleConnection` so it can avoid using
		`purple_connection_[gs]et_protocol_data`. The default implementation for this
		method, just creates a `PurpleConnection` with the given protocol, account,
		and password.

		This also adds two new methods to `PurpleConnection`; `connect` and
		`disconnect`. The default implementation for these two just call
		`purple_protocol_login` and `purple_protocol_close` respectively.

		The long term intent of this, is to have all protocol plugins subclass
		`PurpleConnection` and then remove the `login` and `close` methods from
		`PurpleProtocol` and also remove `purple_connection_[gs]et_protocol_data`.