Extend accounts table for custom search attributes #27832

tomneedham · 2017-05-08T13:35:59Z

Description

Adds an additional column to the accounts table that can be supplied by the user backends to permit improved searching
Adds a find method to user manager to query email, displayname, userid and search_attributes
Adds a findUsersInGroup method to query group backends and filter users using the find method

How Has This Been Tested?

Extended the Database user backend to return a string and observed string in database
Tested searching for users in the sharee dialog box using their email address

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

PVince81 · 2017-05-09T16:19:09Z

Steps:

Enable guest app
Run npm run build if you picked the guest app and are wondering why it doesn't work
Share a folder "test" with "Someone" and wait for the "(guest)" entry appears in the dropdown, select it
Type in an email address
Save
Log out
Open the email
Open the link from the email
Set a password
Login as admin user again (or whatever first user from step 1)
Create a folder "test2"
Type "Someo" to search for the existing guest user

Expected: guest user appears
Actual: "an error occurred"

{"reqId":"nCeGIVUGZ56IHxxMZ6Zu","level":3,"time":"2017-05-09T16:16:54+00:00","remoteAddr":"127.0.0.1","user":"admin","app":"PHP","method":"GET","url":"\/owncloud\/ocs\/v1.php\/apps\/files_sharing\/api\/v1\/sharees?format=json&search=vince&perPage=200&itemType=folder","message":"Call to undefined method OC\\User\\User::getEmail() at \/srv\/www\/htdocs\/owncloud\/apps\/files_sharing\/lib\/Controller\/ShareesController.php#165"}

PVince81 · 2017-05-09T16:22:11Z

note that in my case the email address also started with "someone". Well, actually I used "Vincent" as guest name and "[email protected]" (where hostname.tld is the local computer) when testing. So the bug could also be related to this overlap when searching.

From the error it seems to be an API issue.

tomneedham · 2017-05-10T14:31:05Z

REF: #27850

butonic · 2017-05-11T07:19:49Z

lib/private/User/Backend.php

@@ -46,6 +46,7 @@
 	const SET_DISPLAYNAME	= 1048576;		// 1 << 20
 	const PROVIDE_AVATAR	= 16777216;		// 1 << 24
 	const COUNT_USERS		= 268435456;	// 1 << 28
+	const SEARCH_STRING		= 4294967296;	// 1 << 32


we start exceeding max int here ... why are the constants shifted by 4 anyway? wtf?

butonic · 2017-05-11T09:16:59Z

core/Migrations/Version20170510143952.php

+		$table->addColumn('searchString', 'string', [
+			'notnull' => false,
+			'length' => 64,
+		]);


needs an index

jvillafanez · 2017-05-12T07:07:25Z

lib/private/User/AccountMapper.php

+		$qb->select('*')
+			->from($this->getTableName())
+			->where($qb->expr()->Like('user_id', $qb->createNamedParameter('%' . $this->db->escapeLikeParameter(strtolower($pattern)) . '%')))
+			->orWhere($qb->expr()->iLike('display_name', $qb->createNamedParameter('%' . $this->db->escapeLikeParameter($pattern) . '%')))


Quoting comment from http://stackoverflow.com/a/34029944:

It is in PostgreSQL the keyword ILIKE can be used instead of LIKE to make the match case-insensitive according to the active locale. This is not in the SQL standard but is a PostgreSQL extension.

What is the question here? We use ILIKE from postgres and have adapters for mysql, sqlite and oracle.

On oracle it is currently problematic when the search string contains [ or ] see #26672

if we have hidden magic then it's ok.

jvillafanez · 2017-05-12T07:11:37Z

lib/private/Group/Manager.php

+		foreach($groupUsers as $groupUser) {
+			$matchingUsers[$groupUser->getUID()] = $groupUser;
+		}
+		die(var_dump($matchingUsers));


jvillafanez · 2017-05-12T07:13:06Z

core/Migrations/Version20170510143952.php

+		// Add column to hold additional search attributes
+		$table->addColumn('search_attributes', 'string', [
+			'notnull' => false,
+			'length' => 64,


This is far from being enough

DeepDiver1975 · 2017-05-12T14:22:49Z

core/Migrations/Version20170510143952.php

+		]);
+
+		// Add index for search attributes
+		$table->addIndex(['search_attributes'], 'search_attributes_index');


we need a combined index if one query searches in multiple columns

@tomneedham any update ?

nope. we need an index per or part of a query:

-- with combined index CREATE INDEX find_index ON oc_accounts (lower_user_id, display_name, email); sqlite> EXPLAIN QUERY PLAN select * from oc_accounts WHERE lower_user_id LIKE '%test%' and display_name LIKE '%test%' and email LIKE '%test%'; 0|0|0|SCAN TABLE oc_accounts sqlite> EXPLAIN QUERY PLAN select * from oc_accounts WHERE lower_user_id LIKE '%test%' or display_name LIKE '%test%' or email LIKE '%test%'; 0|0|0|SCAN TABLE oc_accounts sqlite> EXPLAIN QUERY PLAN select * from oc_accounts WHERE lower_user_id LIKE '%test%' or display_name LIKE '%test%' or email LIKE '%test%'; 0|0|0|SCAN TABLE oc_accounts sqlite> EXPLAIN QUERY PLAN select * from oc_accounts WHERE lower_user_id = '%test%' or display_name = '%test%' or email = '%test%'; 0|0|0|SCAN TABLE oc_accounts sqlite> EXPLAIN QUERY PLAN select * from oc_accounts WHERE lower_user_id = '%test%' and display_name = '%test%' and email = '%test%'; 0|0|0|SEARCH TABLE oc_accounts USING INDEX find_index (lower_user_id=? AND display_name=? AND email=?) sqlite> ... -- with individual indexes CREATE UNIQUE INDEX lower_user_id_index ON oc_accounts (lower_user_id); CREATE INDEX display_name_index ON oc_accounts (display_name); CREATE INDEX email_index ON oc_accounts (email); sqlite> EXPLAIN QUERY PLAN select * from oc_accounts WHERE lower_user_id = '%test%' and display_name = '%test%' and email = '%test%'; 0|0|0|SEARCH TABLE oc_accounts USING INDEX lower_user_id_index (lower_user_id=?) sqlite> EXPLAIN QUERY PLAN select * from oc_accounts WHERE lower_user_id = '%test%' or display_name = '%test%' or email = '%test%'; 0|0|0|SEARCH TABLE oc_accounts USING INDEX lower_user_id_index (lower_user_id=?) 0|0|0|SEARCH TABLE oc_accounts USING INDEX display_name_index (display_name=?) 0|0|0|SEARCH TABLE oc_accounts USING INDEX email_index (email=?) sqlite>

then again like kills the indexes on sqlite anyway:

EXPLAIN QUERY PLAN select * from oc_accounts WHERE lower_user_id like '%test%' or display_name like '%test%' or email like '%test%'; 0|0|0|SCAN TABLE oc_accounts

also see http://use-the-index-luke.com/de/sql/where/bereiche/like-filter

PVince81 · 2017-05-15T14:47:30Z

ok guest app part seems to work again

PVince81 · 2017-05-15T14:51:40Z

With the guest app if I type in the email of the previously created guest user it appears in the list with the display name and I can select it for sharing.

PVince81 · 2017-05-15T14:54:37Z

Fixes #27847

PVince81 · 2017-05-15T15:11:59Z

Apart from the index question, is this ready ? @butonic @DeepDiver1975 @jvillafanez

cdamken · 2017-05-15T15:16:43Z

wait, im testing now :)

cdamken · 2017-05-15T15:18:47Z

Looks good for me now 👍 Tested and works!

tomneedham · 2017-05-15T16:30:36Z

185 for the length of the search_attributes column as according to some research by @butonic this is the max indexable length we can achieve - so my understanding is that if we add an index that spans this column as well it won't be effective

tomneedham · 2017-05-16T13:45:03Z

Working on a new branch / PR - closing for now

PVince81 · 2017-05-17T07:51:43Z

the new PR #27906

lock · 2019-08-02T21:01:39Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

tomneedham added the 2 - Developing label May 8, 2017

tomneedham added this to the 10.0.1 milestone May 8, 2017

tomneedham self-assigned this May 8, 2017

tomneedham requested review from PVince81 and DeepDiver1975 May 8, 2017 13:35

tomneedham mentioned this pull request May 10, 2017

[10.0.0] "User Search Attributes" does not search in those attributes owncloud/user_ldap#96

Closed

tomneedham force-pushed the user-account-find branch from d24b379 to dccd8c2 Compare May 10, 2017 09:26

felixboehm mentioned this pull request May 10, 2017

share by email, resolve existing users by email #27847

Closed

1 task

butonic reviewed May 11, 2017

View reviewed changes

tomneedham added 2 commits May 11, 2017 11:53

Add find methods to user manager and group manager

3422d3d

Add indexes for improved search performance and use lower uid in query

8e00ed3

tomneedham force-pushed the user-account-find branch from 49e24c9 to 8e00ed3 Compare May 11, 2017 10:54

tomneedham changed the title ~~Add find methods to user manager and group manager~~ Extend accounts table for custom search attributes May 11, 2017

jvillafanez reviewed May 12, 2017

View reviewed changes

tomneedham force-pushed the user-account-find branch 2 times, most recently from 802798c to 80a4a7d Compare May 12, 2017 07:57

Add search attribute to user interface

9d75572

tomneedham force-pushed the user-account-find branch from 80a4a7d to 9d75572 Compare May 12, 2017 08:04

PVince81 mentioned this pull request May 12, 2017

Add option to restrict share autocomplete to group members #27869

Merged

16 tasks

tomneedham added 3 commits May 12, 2017 13:43

Add search attribute to user interface

29fcb72

Update sharee unit tests for custom search attribute support

25b9ce4

Renamed search attribute backend interface

5ebc7bc

tomneedham force-pushed the user-account-find branch from 26a5275 to 5ebc7bc Compare May 12, 2017 12:50

Add tests for group|user manager find methods

db21e77

tomneedham added 3 - To Review and removed 2 - Developing labels May 12, 2017

DeepDiver1975 requested changes May 12, 2017

View reviewed changes

Update account after changing search attributes string

e010774

tomneedham closed this May 16, 2017

tomneedham mentioned this pull request May 16, 2017

User backend extendable account searching #27906

Merged

9 tasks

DeepDiver1975 deleted the user-account-find branch August 11, 2017 22:53

lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend accounts table for custom search attributes #27832

Extend accounts table for custom search attributes #27832

tomneedham commented May 8, 2017 •

edited

Loading

PVince81 commented May 9, 2017

PVince81 commented May 9, 2017

tomneedham commented May 10, 2017

butonic May 11, 2017 •

edited

Loading

tomneedham May 11, 2017

butonic May 11, 2017

jvillafanez May 12, 2017

butonic May 12, 2017 •

edited

Loading

jvillafanez May 15, 2017

jvillafanez May 12, 2017

jvillafanez May 12, 2017

DeepDiver1975 May 12, 2017

PVince81 May 15, 2017

butonic May 18, 2017 •

edited

Loading

butonic May 18, 2017

butonic May 18, 2017

PVince81 commented May 15, 2017

PVince81 commented May 15, 2017

PVince81 commented May 15, 2017

PVince81 commented May 15, 2017

cdamken commented May 15, 2017

cdamken commented May 15, 2017 •

edited

Loading

tomneedham commented May 15, 2017

tomneedham commented May 16, 2017

PVince81 commented May 17, 2017

lock bot commented Aug 2, 2019

Extend accounts table for custom search attributes #27832

Extend accounts table for custom search attributes #27832

Conversation

tomneedham commented May 8, 2017 • edited Loading

Description

How Has This Been Tested?

Types of changes

Checklist:

PVince81 commented May 9, 2017

PVince81 commented May 9, 2017

tomneedham commented May 10, 2017

butonic May 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

butonic May 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

butonic May 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PVince81 commented May 15, 2017

PVince81 commented May 15, 2017

PVince81 commented May 15, 2017

PVince81 commented May 15, 2017

cdamken commented May 15, 2017

cdamken commented May 15, 2017 • edited Loading

tomneedham commented May 15, 2017

tomneedham commented May 16, 2017

PVince81 commented May 17, 2017

lock bot commented Aug 2, 2019

tomneedham commented May 8, 2017 •

edited

Loading

butonic May 11, 2017 •

edited

Loading

butonic May 12, 2017 •

edited

Loading

butonic May 18, 2017 •

edited

Loading

cdamken commented May 15, 2017 •

edited

Loading