From 976fbee877c97362f770e57830633d68e1e1e93f Mon Sep 17 00:00:00 2001 From: Austin McKinley Date: Mon, 17 Apr 2017 17:27:03 -0700 Subject: [PATCH] Implement ngram search for File objects Summary: Follows the outline in D15656 for implementing ngram search for names of File objects. Also created FileFullTextEngine, because without implementing `PhabricatorFulltextInterface`, `./bin/search` complains that `File` is not an indexable type. Test Plan: - ran `./bin/storage upgrade` to apply the schema change - confirmed the presence of a new `file_filename_ngrams` table - added a couple file objects - ran `bin/search index --type file --force` - confirmed the presence of rows in `file_filename_ngrams` - did a few keyword searches and saw expected results Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T8788 Differential Revision: https://secure.phabricator.com/D17702 --- .../sql/autopatches/20170417.files.ngrams.sql | 7 +++++++ src/__phutil_library_map__.php | 4 ++++ .../PhabricatorFileDeleteController.php | 3 ++- .../files/editor/PhabricatorFileEditor.php | 2 +- ...habricatorFileTemporaryGarbageCollector.php | 4 +++- .../files/query/PhabricatorFileQuery.php | 6 ++++++ .../query/PhabricatorFileSearchEngine.php | 8 ++++++++ .../files/storage/PhabricatorFile.php | 16 ++++++++++++++-- .../storage/PhabricatorFileNameNgrams.php | 18 ++++++++++++++++++ 9 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 resources/sql/autopatches/20170417.files.ngrams.sql create mode 100644 src/applications/files/storage/PhabricatorFileNameNgrams.php diff --git a/resources/sql/autopatches/20170417.files.ngrams.sql b/resources/sql/autopatches/20170417.files.ngrams.sql new file mode 100644 index 0000000000..988b183323 --- /dev/null +++ b/resources/sql/autopatches/20170417.files.ngrams.sql @@ -0,0 +1,7 @@ +CREATE TABLE {$NAMESPACE}_file.file_filename_ngrams ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + objectID INT UNSIGNED NOT NULL, + ngram CHAR(3) NOT NULL COLLATE {$COLLATE_TEXT}, + KEY `key_object` (objectID), + KEY `key_ngram` (ngram, objectID) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index efb5b65ffe..eb71aaacd3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2781,6 +2781,7 @@ phutil_register_library_map(array( 'PhabricatorFileLightboxController' => 'applications/files/controller/PhabricatorFileLightboxController.php', 'PhabricatorFileLinkView' => 'view/layout/PhabricatorFileLinkView.php', 'PhabricatorFileListController' => 'applications/files/controller/PhabricatorFileListController.php', + 'PhabricatorFileNameNgrams' => 'applications/files/storage/PhabricatorFileNameNgrams.php', 'PhabricatorFileNameTransaction' => 'applications/files/xaction/PhabricatorFileNameTransaction.php', 'PhabricatorFileQuery' => 'applications/files/query/PhabricatorFileQuery.php', 'PhabricatorFileROT13StorageFormat' => 'applications/files/format/PhabricatorFileROT13StorageFormat.php', @@ -7908,6 +7909,8 @@ phutil_register_library_map(array( 'PhabricatorPolicyInterface', 'PhabricatorDestructibleInterface', 'PhabricatorConduitResultInterface', + 'PhabricatorIndexableInterface', + 'PhabricatorNgramsInterface', ), 'PhabricatorFileAES256StorageFormat' => 'PhabricatorFileStorageFormat', 'PhabricatorFileBundleLoader' => 'Phobject', @@ -7954,6 +7957,7 @@ phutil_register_library_map(array( 'PhabricatorFileLightboxController' => 'PhabricatorFileController', 'PhabricatorFileLinkView' => 'AphrontTagView', 'PhabricatorFileListController' => 'PhabricatorFileController', + 'PhabricatorFileNameNgrams' => 'PhabricatorSearchNgrams', 'PhabricatorFileNameTransaction' => 'PhabricatorFileTransactionType', 'PhabricatorFileQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorFileROT13StorageFormat' => 'PhabricatorFileStorageFormat', diff --git a/src/applications/files/controller/PhabricatorFileDeleteController.php b/src/applications/files/controller/PhabricatorFileDeleteController.php index acca7c9b1c..1077625bb1 100644 --- a/src/applications/files/controller/PhabricatorFileDeleteController.php +++ b/src/applications/files/controller/PhabricatorFileDeleteController.php @@ -25,7 +25,8 @@ final class PhabricatorFileDeleteController extends PhabricatorFileController { } if ($request->isFormPost()) { - $file->delete(); + $engine = new PhabricatorDestructionEngine(); + $engine->destroyObject($file); return id(new AphrontRedirectResponse())->setURI('/file/'); } diff --git a/src/applications/files/editor/PhabricatorFileEditor.php b/src/applications/files/editor/PhabricatorFileEditor.php index 28b781fb37..6a2b797b40 100644 --- a/src/applications/files/editor/PhabricatorFileEditor.php +++ b/src/applications/files/editor/PhabricatorFileEditor.php @@ -71,7 +71,7 @@ final class PhabricatorFileEditor } protected function supportsSearch() { - return false; + return true; } } diff --git a/src/applications/files/garbagecollector/PhabricatorFileTemporaryGarbageCollector.php b/src/applications/files/garbagecollector/PhabricatorFileTemporaryGarbageCollector.php index c79bb9ba99..bbbdfb9850 100644 --- a/src/applications/files/garbagecollector/PhabricatorFileTemporaryGarbageCollector.php +++ b/src/applications/files/garbagecollector/PhabricatorFileTemporaryGarbageCollector.php @@ -18,8 +18,10 @@ final class PhabricatorFileTemporaryGarbageCollector 'ttl < %d LIMIT 100', PhabricatorTime::getNow()); + $engine = new PhabricatorDestructionEngine(); + foreach ($files as $file) { - $file->delete(); + $engine->destroyObject($file); } return (count($files) == 100); diff --git a/src/applications/files/query/PhabricatorFileQuery.php b/src/applications/files/query/PhabricatorFileQuery.php index c2ac083fea..6439f75405 100644 --- a/src/applications/files/query/PhabricatorFileQuery.php +++ b/src/applications/files/query/PhabricatorFileQuery.php @@ -119,6 +119,12 @@ final class PhabricatorFileQuery return $this; } + public function withNameNgrams($ngrams) { + return $this->withNgramsConstraint( + id(new PhabricatorFileNameNgrams()), + $ngrams); + } + public function showOnlyExplicitUploads($explicit_uploads) { $this->explicitUploads = $explicit_uploads; return $this; diff --git a/src/applications/files/query/PhabricatorFileSearchEngine.php b/src/applications/files/query/PhabricatorFileSearchEngine.php index 1f2f2de4b3..0d989000cf 100644 --- a/src/applications/files/query/PhabricatorFileSearchEngine.php +++ b/src/applications/files/query/PhabricatorFileSearchEngine.php @@ -38,6 +38,10 @@ final class PhabricatorFileSearchEngine id(new PhabricatorSearchDateField()) ->setKey('createdEnd') ->setLabel(pht('Created Before')), + id(new PhabricatorSearchTextField()) + ->setLabel(pht('Name Contains')) + ->setKey('name') + ->setDescription(pht('Search for files by name substring.')), ); } @@ -68,6 +72,10 @@ final class PhabricatorFileSearchEngine $query->withDateCreatedBefore($map['createdEnd']); } + if ($map['name'] !== null) { + $query->withNameNgrams($map['name']); + } + return $query; } diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index db15fb43e0..f697be0dc2 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -28,7 +28,9 @@ final class PhabricatorFile extends PhabricatorFileDAO PhabricatorFlaggableInterface, PhabricatorPolicyInterface, PhabricatorDestructibleInterface, - PhabricatorConduitResultInterface { + PhabricatorConduitResultInterface, + PhabricatorIndexableInterface, + PhabricatorNgramsInterface { const METADATA_IMAGE_WIDTH = 'width'; const METADATA_IMAGE_HEIGHT = 'height'; @@ -87,7 +89,7 @@ final class PhabricatorFile extends PhabricatorFileDAO 'metadata' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text255?', + 'name' => 'sort255?', 'mimeType' => 'text255?', 'byteSize' => 'uint64', 'storageEngine' => 'text32', @@ -1585,4 +1587,14 @@ final class PhabricatorFile extends PhabricatorFileDAO return array(); } +/* -( PhabricatorNgramInterface )------------------------------------------ */ + + + public function newNgrams() { + return array( + id(new PhabricatorFileNameNgrams()) + ->setValue($this->getName()), + ); + } + } diff --git a/src/applications/files/storage/PhabricatorFileNameNgrams.php b/src/applications/files/storage/PhabricatorFileNameNgrams.php new file mode 100644 index 0000000000..6e97e8f0e4 --- /dev/null +++ b/src/applications/files/storage/PhabricatorFileNameNgrams.php @@ -0,0 +1,18 @@ +