Address collector #1276

ChristophWurst · 2016-02-07T18:14:15Z

Adds all addresses of sent mails (to, cc, bcc) to a db table, so that those addresses can then be used for auto-completion.

This is a very simple implementation, it only saves the bare email address. Adding more information (label, recipient name) can be done in another PR as that might need some advanced update-logic.

We should also think about a way for users to delete one of those entries…

TODO:

fix sql matching
update/add unit tests

implements #879
fixes #1254

ChristophWurst · 2016-02-07T18:16:19Z

appinfo/database.xml

+ <index>
+ <name>mail_collected_addr_email_index</name>
+ <field>
+ <name>user_id</name>


this should be 'email'

ChristophWurst · 2016-02-08T15:59:00Z

lib/db/collectedaddressmapper.php

+ }
+
+ public function exists($userId, $email) {
+ $sql = 'SELECT * FROM *PREFIX*mail_collected_addresses WHERE `user_id` = ? AND `email` = ?';


use ILIKE here too

ChristophWurst · 2016-02-08T18:49:43Z

Dang it. Some test cases still fail

ChristophWurst · 2016-02-12T13:20:54Z

This is weird:

exception 'OCP\AppFramework\QueryException' with message 'Could not resolve OCP\IDBConnection! Class can not be instantiated' in /home/travis/build/owncloud/core/lib/private/appframework/utility/simplecontainer.php:80

Stack trace:

#0 /home/travis/build/owncloud/core/lib/private/appframework/utility/simplecontainer.php(98): OC\AppFramework\Utility\SimpleContainer->resolve('OCP\\IDBConnecti...')

#1 /home/travis/build/owncloud/core/lib/private/appframework/utility/simplecontainer.php(59): OC\AppFramework\Utility\SimpleContainer->query('OCP\\IDBConnecti...')

#2 /home/travis/build/owncloud/core/lib/private/appframework/utility/simplecontainer.php(78): OC\AppFramework\Utility\SimpleContainer->buildClass(Object(ReflectionClass))

#3 /home/travis/build/owncloud/core/lib/private/appframework/utility/simplecontainer.php(98): OC\AppFramework\Utility\SimpleContainer->resolve('OCA\\Mail\\Db\\Col...')

#4 /home/travis/build/owncloud/core/lib/private/appframework/utility/simplecontainer.php(59): OC\AppFramework\Utility\SimpleContainer->query('OCA\\Mail\\Db\\Col...')

#5 /home/travis/build/owncloud/core/lib/private/appframework/utility/simplecontainer.php(78): OC\AppFramework\Utility\SimpleContainer->buildClass(Object(ReflectionClass))

#6 /home/travis/build/owncloud/core/lib/private/appframework/utility/simplecontainer.php(98): OC\AppFramework\Utility\SimpleContainer->resolve('OCA\\Mail\\Servic...')

#7 /home/travis/build/owncloud/core/apps/mail/lib/account.php(90): OC\AppFramework\Utility\SimpleContainer->query('OCA\\Mail\\Servic...')

#8 /home/travis/build/owncloud/core/apps/mail/tests/imap/abstracttest.php(71): OCA\Mail\Account->__construct(Object(OCA\Mail\Db\MailAccount))

#9 [internal function]: OCA\Mail\Tests\Imap\AbstractTest::setUpBeforeClass()

#10 phar:///home/travis/.phpenv/versions/5.4.37/bin/phpunit/phpunit/Framework/TestSuite.php(673): call_user_func(Array)

#11 phar:///home/travis/.phpenv/versions/5.4.37/bin/phpunit/phpunit/Framework/TestSuite.php(716): PHPUnit_Framework_TestSuite->run(Object(PHPUnit_Framework_TestResult))

#12 phar:///home/travis/.phpenv/versions/5.4.37/bin/phpunit/phpunit/TextUI/TestRunner.php(398): PHPUnit_Framework_TestSuite->run(Object(PHPUnit_Framework_TestResult))

#13 phar:///home/travis/.phpenv/versions/5.4.37/bin/phpunit/phpunit/TextUI/Command.php(152): PHPUnit_TextUI_TestRunner->doRun(Object(PHPUnit_Framework_TestSuite), Array)

#14 phar:///home/travis/.phpenv/versions/5.4.37/bin/phpunit/phpunit/TextUI/Command.php(104): PHPUnit_TextUI_Command->run(Array, true)

#15 /home/travis/.phpenv/versions/5.4.37/bin/phpunit(722): PHPUnit_TextUI_Command::main()

#16 {main}

It's only reproducible on php5.4 and I have no idea what might cause that exception. @DeepDiver1975 is there any difference between php5.4 and php5.6 in terms of owncloud dependency injection?

Gomez · 2016-02-15T14:20:03Z

I dropping 5.4 a option for us (i would say yes)? The news app dropped it and 5.4 is EOL.

DeepDiver1975 · 2016-02-15T15:40:53Z

lib/account.php

@@ -83,6 +87,7 @@ public function __construct(MailAccount $account) {
 $this->crypto = \OC::$server->getCrypto();
 $this->config = \OC::$server->getConfig();
 $this->memcacheFactory = \OC::$server->getMemcacheFactory();
+ $this->addressCollector = \OC::$server->query('OCA\Mail\Service\AutoCompletion\AddressCollector');


@ChristophWurst this is the reason why this is falling apart.

In previous versions of owncloud you cannot query the server container for services which are located in an app ... let me fix that ...

ChristophWurst · 2016-02-16T08:31:22Z

@DeepDiver1975 thanks for fixing

@owncloud/mail @ErikPel @jospoortvliet review please ;-)

jancborchardt · 2016-02-16T11:11:05Z

Nice @ChristophWurst, awesome on that functionality! :)

Do Gmail and others also collect addresses of people you receive emails from? (As well as people in cc there.)

And of course as you mention – saving the name would be cool, but for a follow-up PR.

enoch85 · 2016-02-16T11:14:45Z

I know this is for another PR, but I'd love this to be synced with my CardDav as well so that when I write on mobile, the address is there as well.

ChristophWurst · 2016-02-16T11:47:19Z

I dropping 5.4 a option for us (i would say yes)? The news app dropped it and 5.4 is EOL.

If we want to become an "official" app we have to support all platforms supported by core, see https://doc.owncloud.org/server/8.2/developer_manual/app/publishing.html#official

Gomez · 2016-02-16T14:49:07Z

@ChristophWurst Cool! Checked & Tested

@enoch85 Would you open a issue? This PR will be closed.

Address collector

ChristophWurst added enhancement 2 - developing labels Feb 7, 2016

ChristophWurst added this to the 0.4 – small innovations! milestone Feb 7, 2016

DeepDiver1975 added the in progress label Feb 7, 2016

ChristophWurst reviewed Feb 7, 2016
View reviewed changes

ChristophWurst force-pushed the address-collector branch from 5c86661 to ee07ac2 Compare February 8, 2016 15:10

ChristophWurst reviewed Feb 8, 2016
View reviewed changes

ChristophWurst force-pushed the address-collector branch from 5bdbbd3 to bc29b32 Compare February 8, 2016 16:27

ChristophWurst added 3 - to review 2 - developing and removed 2 - developing 3 - to review labels Feb 8, 2016

ChristophWurst force-pushed the address-collector branch from 42f58d9 to f05a00f Compare February 11, 2016 14:10

ChristophWurst added 8 commits February 15, 2016 16:34

add basic class structure

aaaaeec

add ability to search collected addresses

c5a0181

collect addresses of sent mails

7ac1260

update/add unit tests

39b7880

fix index column

d02b47e

add database tests

a105cde

fix wildcard matching

61cdc13

fix unit tests

8638311

DeepDiver1975 force-pushed the address-collector branch from f05a00f to 8638311 Compare February 15, 2016 15:35

DeepDiver1975 reviewed Feb 15, 2016
View reviewed changes

Move the address collector outside of class Account

c6a8e67

DeepDiver1975 force-pushed the address-collector branch from b2fe19b to c6a8e67 Compare February 15, 2016 16:14

More fault tolerant tearDown

61b0ed7

Fix CollectedAddressMapper ctor

2faacd9

DeepDiver1975 force-pushed the address-collector branch from ba1d248 to 2faacd9 Compare February 16, 2016 07:12

ChristophWurst added 3 - to review and removed 2 - developing labels Feb 16, 2016

Gomez added a commit that referenced this pull request Feb 16, 2016

Merge pull request #1276 from owncloud/address-collector

fba6a34

Address collector

Gomez merged commit fba6a34 into master Feb 16, 2016

Gomez deleted the address-collector branch February 16, 2016 14:49

DeepDiver1975 removed the in progress label Feb 16, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address collector #1276

Address collector #1276

ChristophWurst commented Feb 7, 2016

ChristophWurst Feb 7, 2016

ChristophWurst Feb 8, 2016

ChristophWurst commented Feb 8, 2016

ChristophWurst commented Feb 12, 2016

Gomez commented Feb 15, 2016

DeepDiver1975 Feb 15, 2016

ChristophWurst commented Feb 16, 2016

jancborchardt commented Feb 16, 2016

enoch85 commented Feb 16, 2016

ChristophWurst commented Feb 16, 2016

Gomez commented Feb 16, 2016

Address collector #1276

Address collector #1276

Conversation

ChristophWurst commented Feb 7, 2016

ChristophWurst Feb 7, 2016

Choose a reason for hiding this comment

ChristophWurst Feb 8, 2016

Choose a reason for hiding this comment

ChristophWurst commented Feb 8, 2016

ChristophWurst commented Feb 12, 2016

Gomez commented Feb 15, 2016

DeepDiver1975 Feb 15, 2016

Choose a reason for hiding this comment

ChristophWurst commented Feb 16, 2016

jancborchardt commented Feb 16, 2016

enoch85 commented Feb 16, 2016

ChristophWurst commented Feb 16, 2016

Gomez commented Feb 16, 2016