Add localusers contacts to core

[LEDfan]

In this PR the currently implemented Localusers backend for the contacts app will be moved to core.

@DeepDiver1975 is this how I should implement it? The \OC\Contacts\Backend\LocalUsers class will be almost the same as \OCA\Contacts\Backend\Localusers. (which will be removed of course)

Some questions:

• every contacts backend extends the \OCA\Contacts\Backend\AbstractBackend class. This should stay the same for the localusers class in core. Should I move the AbstractBackend class to the public namespace of core, and remove it out of the contacts app?
• To update the DB with the contacts should I use hooks on user regristration, deleteing etc, or check if something has changed every time the contacts are fetched. (This is the current implementation.)
• where should the DB schemes go?

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5483/

[LEDfan]

This class/file is needed to let the LocalUsers backend work. Is it okay to move this to core?
Should I remove these classes from the Contacts app? (If so maybe move them to \OCP\?)

[LEDfan]

🎆 it's working :)
I had to add a handfull classes from the Contacts app to core, is this okay?

TODO's

• if the displayname of an user is updated, update it in the contacts refs file management broken on a fresh instance using postgres #474 Show error message to prevent adding a shared folder in the root dir fix... #482
  What should be the behaviour when foo changes the fullname of bar to Barry but then bar changes it's own displayname via owncloud:
  1. override changed fullname in the contacts app of foo by the one bar set via Personal settings
  2. leave the fullanme in the contacts app of foo unchanged
     refs the discussion in file management broken on a fresh instance using postgres #474
• remove LocalUsers from the contacts app
• avatar of contact must be the same as from the ownCloud user
  • a problem with this is that there is no hook for an updated avatar and again the same problem as with overriding like the avatars

16/06

• delete avatar from contact when it's deleted from the user
• support JPG images
• move classess from the contacts app to core
  • \OCA\Contacts\Utils\TemporaryPhoto
  • \OCA\Contacts\VObject\VCard

17/]06
- [ ] add email from user to contact
- [ ] update email to contact when changed

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5604/

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5605/

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5611/

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5612/

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5613/

[LEDfan]

Avatars are now automatically uploaded to the contacts, even when they are changed.

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5618/

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5632/

[LEDfan]

Displaynames are now automatically uploaded to the contacts, even when they are changed.

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5652/

[LEDfan]

I think the work here is done.
I understand oC7 has higher prio, so I'll just leave this here open until oC7 is released?

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5682/

[LEDfan]

@owncloud-bot retest this please

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5824/

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5825/

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5826/

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/5827/

[LEDfan]

I'm ready with this and this can be tested, can someone add a 5 To Review tag to this?

@jancborchardt about the implemantation:

• when a user changes his avatar the avatar of the contact is updated for all ownCloud users
• when a user changes his displaynames, there are 2 scenarios:
  • the contact's displayname is differnet than the user's old displayname -> the displayname of the contact is changed by the owner of the contact -> nothing is done
  • the contact's displayname euqals the old use's displayname -> the displayname of the contact is changed
• the email address of the user isn't added to the contact nor updated
• all contacts are stored in a special group in the contacts app
  

This can be reviewed.

[LEDfan]

Refs http://stackoverflow.com/questions/16229004/oracle-cannot-insert-a-null-value-to-a-non-primary-key

It looks like the query needs to insert null as id instead of nothing, because the column is set as NOT NULL.

[scrutinizer-notifier]

The inspection completed: 33 new issues, 67 updated code elements

[LEDfan]

Now it fails on pgsql:

03:49:27 SQLSTATE[23502]: Not null violation: 7 ERROR:  null value in column "id" violates not-null constraint"<br />Offending command was: INSERT INTO `*PREFIX*vcategory` (`uid`,`type`,`category`,`id`) SELECT ?,?,?,? FROM `*PREFIX*vcategory` WHERE `uid` = ? AND `type` = ? AND `category` = ? AND `id` = ? HAVING COUNT(*) = 0<br />

[LEDfan]

I was wrong with the id. It's trying to insert NULL into uid. However the query says it's inserting false..... I'll revert the latest commit, but I have no idea what's causing this problem.

[DeepDiver1975]

@LEDfan okay - I'm back on this pull request - sorry too much other things going on here

so we have still more than 1000 LOC? Still a big bunch - I'll have a closer look ...

[scrutinizer-notifier]

A new inspection was created.

[scrutinizer-notifier]

The inspection completed: 33 new issues, 67 updated code elements

[LEDfan]

@DeepDiver1975 I checkout this branch with an oracle DB server. Everything works as expected.
However when I create an user "false" I get the same error. I even can't log in with that user. EDIT: I used the wrong password If I create a "false" user on a MySQL database everything works.
It guess that owncloud internally send the username as boolean (bc there are no " around false in the errors) and this doesn't work with oracle.

[scrutinizer-notifier]

A new inspection was created.

[LEDfan]

@jancborchardt @DeepDiver1975
Because with #9641 the contacts doesn't show up in the Contacts app I tried to remove the abstractpimcollection abstractpimobject and IPIMObject files. It works fine without changing the code :)

TODO (for me)

• rebase
• test
• oracle issue: Add localusers contacts to core #8951 (comment)

[scrutinizer-notifier]

A new inspection was created.

[scrutinizer-notifier]

The inspection completed: 47 new issues, 48 updated code elements

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/6316/

[ghost]

💣 Test Failed. 💣
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/6317/

[DeepDiver1975]

Because with #9641 the contacts doesn't show up in the Contacts app I tried to remove the abstractpimcollection abstractpimobject and IPIMObject files. It works fine without changing the code :)

if we walk th path of #9641 this pull request is no longer needed.
the ability for contacts to read all addressbooks has to be implemented in the contacts app.

[jancborchardt]

the ability for contacts to read all addressbooks has to be implemented in the contacts app.

Yeah, that’s what I meant. The core should only expose the local users through some kind of API. It’s up to the apps to implement the API then.

[jancborchardt]

So @LEDfan should we close this then and focus on the lightweight solution in #9641? The Contacts app of course has to talk with the backend there.

[LEDfan]

@jancborchardt yup.