- Download Diff

Summary:

Serialize the ContactManager

Review Request #2054 — Created Nov. 9, 2022 and updated Dec. 4, 2022, 5:55 a.m.

Information
Owner:	grim
Repository:	pidgin/pidgin
Branch:	default
Bugs:	PIDGIN-17687
Depends On:
Reviewers
Groups:	pidgin
People:

Description

This is saved in a SQLite database inside the user config directory named
contacts.db. It does its best to synchronize with PurpleBuddy, but not
everything emits a signal when it changes.

Also, stuff like custom avatars are ignored as those are currently store
on the Contact and we don't have a mapping to PurplePerson yet.

Testing Done

Ran with the demo account and verified everything about the contacts was stored via the sqlite3 command line.

Commits

Summary
	Serialize the ContactManager

Screenshots

Issues

Description	From	Last Updated
Figure out how to handle unit tests. This doens't know when we're testing or not, so it always tries to …	grim	Nov. 28, 2022, 10:07 p.m.
Does this need a rebase for PurpleContactInfo stuff?	QuLogic	Dec. 4, 2022, 2:38 a.m.
Why is this in the account manager header?	QuLogic	Dec. 2, 2022, 1:05 a.m.
NULL works here.	QuLogic	Dec. 2, 2022, 1:06 a.m.
name and value are not freed here.	QuLogic	Dec. 2, 2022, 1:06 a.m.
name and value are not freed at the end of the loop.	QuLogic	Dec. 2, 2022, 1:06 a.m.
Not sure why this calls sync_tags but not update_contact_tags? And should everything get a transaction around it?	QuLogic	Dec. 4, 2022, 2:39 a.m.
Ditto.	QuLogic	Dec. 4, 2022, 2:39 a.m.
Don't need the semicolon, I think?	QuLogic	Dec. 2, 2022, 1:07 a.m.
Extra tab?	QuLogic	Dec. 4, 2022, 4:25 p.m.

Change Summary:

Fix typos.

Description:

		This is saved in a SQLite database inside the user config directory named
~		contacts.db. It does it's best to synchronize with PurpleBuddy, but not
	~	`contacts.db`. It does its best to synchronize with `PurpleBuddy`, but not
		everything emits a signal when it changes.

~		Also, stuff like custom avatars are
~		ignore as those are currently store on the Contact and we don't have a mapping
	~	Also, stuff like custom avatars are ignored as those are currently store
	~	on the Contact and we don't have a mapping to `PurplePerson` yet.
-		to PurplePerson yet.

Change Summary:

rebase, just found a bug in the unit tests that needs to be addressed yet.

Commits:

	Summary
-		Serialize the ContactManager
+		Serialize the ContactManager

Diff:

Revision 2 (+974 -10)

Show changes

	libpurple/core.c
	libpurple/purpleaccountmanager.h
	libpurple/purplecontactmanager.h
	libpurple/purplecontactmanager.c
	libpurple/purpleprivate.h
	libpurple/purpletags.h
	libpurple/purpletags.c
	libpurple/resources/libpurple.gresource.xml
	1 more

Change Summary:

add the issue that this is for

Bugs:

+	PIDGIN-17687

libpurple/purpleaccountmanager.h (Diff revision 2)
Show all issues
```
Why is this in the account manager header?
```
1. grim Nov. 28, 2022, 9:46 p.m.
  I must have fixed up my open files...
libpurple/purplecontactmanager.c (Diff revision 2)
Show all issues
```
NULL works here.
```

libpurple/purplecontactmanager.c (Diff revision 2)

Show all issues

name and value are not freed here.

grim Nov. 28, 2022, 10:05 p.m.

They're free'd via the bind method.
I used the following POC to test. Notice the typo in the statement, you can correct it and verify that we still get double free's for both success and failure.

#include <glib.h>
#include <sqlite3.h>

int
main(int argc, char *argv[]) {
    sqlite3 *db = NULL;
    sqlite3_stmt *stmt = NULL;
    char *bar = NULL;

    sqlite3_open(":memory:", &db);

    sqlite3_prepare(db, "select * from foo wher bar=?", -1, &stmt, NULL);

    bar = g_strdup("allocated string");

    sqlite3_bind_text(stmt, 1, bar, -1, g_free);

    sqlite3_step(stmt);

    g_free(bar);

    return 0;
}

grim Nov. 28, 2022, 10:06 p.m.

Derp, this was supposed to be on the other comment below. This block was leaking.

libpurple/purplecontactmanager.c (Diff revision 2)

Show all issues

name and value are not freed at the end of the loop.

grim Nov. 28, 2022, 10:05 p.m.
```
They're free'd via the bind method.
```

grim Nov. 28, 2022, 10:06 p.m.

See the comment above for why this is already free'd.

libpurple/purplecontactmanager.c (Diff revision 2)

Show all issues

Not sure why this calls sync_tags but not update_contact_tags? And should everything get a transaction around it?

libpurple/purplecontactmanager.c (Diff revision 2)
Show all issues
```
Ditto.
```
libpurple/purplecontactmanager.c (Diff revision 2)
Show all issues
```
Don't need the semicolon, I think?
```

Fix it!

Show all issues

Figure out how to handle unit tests. This doens't know when we're testing or not, so it always tries to create purple_user_config_dir() / 'contacts.db' which causes failures.

Change Summary:

rebase and address issues, the unit test failures remain however.

Commits:

	Summary
-		Serialize the ContactManager
+		Serialize the ContactManager

Diff:

Revision 3 (+994 -10)

Show changes

	libpurple/core.c
	libpurple/purplecontactmanager.h
	libpurple/purplecontactmanager.c
	libpurple/purpleprivate.h
	libpurple/purpletags.h
	libpurple/purpletags.c
	libpurple/resources/libpurple.gresource.xml
	libpurple/resources/contactmanager/01-initial.sql

Fix it!

Show all issues

Does this need a rebase for PurpleContactInfo stuff?

libpurple/purplecontactmanager.c (Diff revision 3)
Show all issues
```
Extra tab?
```

Change Summary:

rebase, fix whitespace

Commits:

	Summary
-		Serialize the ContactManager
+		Serialize the ContactManager

Diff:

Revision 4 (+994 -10)

Show changes

	libpurple/core.c
	libpurple/purplecontactmanager.h
	libpurple/purplecontactmanager.c
	libpurple/purpleprivate.h
	libpurple/purpletags.h
	libpurple/purpletags.c
	libpurple/resources/libpurple.gresource.xml
	libpurple/resources/contactmanager/01-initial.sql

You have a pending review.

Review Board 5.0.3

Serialize the ContactManager

Change Summary:

Description:

Change Summary:

Commits:

Diff:

Change Summary:

Bugs:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff: