issue2551189 - increase size of words in full text index.

Increased indexed word maxlength to 50

DB migration code is written and tests work.

Restructured some tests to allow for code reuse.

Docs.

If this passes CI without errors 2551189 should be done.  However,
testing on my system generates errors. Encoding (indexer unicode
russian unicode string invalid) and collation errors (utf8_bin not
valid) when running under python2. No issues with python3 and I
haven't changed code that should cause these since the last successful
build in CI. So if this fails in CI we will have more checkins.

Author: rouilj

CHANGES.txt

@@ -127,6 +127,11 @@ Features:
 - Add better error display to the user. Needed to expose errors in fts5
   search syntax to the user while also displaying the template page
   structure. (John Rouillard)
+- issue2551189 - increase size of words in full text index.
+  Many terms (like exception names or symbolic constants) are larger
+  than 25. Also German words are long. Since there is little chance
+  of fixing German to shorten their words, change indexer maxlength to
+  50. (Thomas Hein provided patch; patch reworked John Rouillard)
 
 2021-07-13 2.1.0
 

doc/upgrading.txt

@@ -38,8 +38,15 @@ Migrating from 2.1.0 to 2.x.y
 Rdbms version change from 6 to 7 (required)
 -------------------------------------------
 
-To add support for database supported full-text search (FTS), the
-rdbms database schema has to be upgraded.
+This release includes two changes that require updates to the database
+schema:
+
+  1. The size of words included in the Roundup FTS indexers have been
+     increased from 25 to 50. This requires changes to the database
+     columns used by the native indexer.  This also affect the whoosh
+     and xapian indexers.
+  2. Some databases that include native full-text search (native-fts
+     indexer) searching are now supported.
 
 You should run the ``roundup-admin migrate`` command for your
 tracker once you've installed the latest codebase.
@@ -52,6 +59,16 @@ updated" (if you've not previously run it on an RDBMS backend) or
 "No migration action required" (if you have run it, or have used
 another interface to the tracker, or are using anydbm).
 
+See `below for directions on enabling native-fts`_.
+
+.. _below for directions on enabling native-fts: \
+    #enhanced-full-text-search-optional
+
+The increase in indexed word length also affects whoosh and xapian
+backends. You may want to run ``roundup-admin -i tracker_home
+reindex`` if you want to index or search for longer words in your full
+text searches. Re-indexing make take some time.
+
 Check compression settings (optional)
 -------------------------------------
 

roundup/backends/back_mysql.py

@@ -412,6 +412,15 @@ def fix_version_5_tables(self):
         else:
             self.log_info('No changes needed.')
 
+    def fix_version_6_tables(self):
+        # Modify length for __words._word column.
+        c = self.cursor
+        sql = "alter table __words change column _word _word varchar(%s)" % (
+                                                                   self.arg)
+        # Why magic number 5? It was the original offset between
+        #   column length and maxlength.
+        c.execute(sql, (self.indexer.maxlength + 5,))
+
     def __repr__(self):
         return '<myroundsql 0x%x>'%id(self)
 

roundup/backends/back_postgresql.py

@@ -264,6 +264,15 @@ def fix_version_3_tables(self):
         self.sql('''CREATE INDEX words_both_idx ON public.__words
             USING btree (_word, _textid)''')
 
+    def fix_version_6_tables(self):
+        # Modify length for __words._word column.
+        c = self.cursor
+        sql = 'alter table __words alter column _word type varchar(%s)' % (
+                                                                  self.arg)
+        # Why magic number 5? It was the original offset between
+        #   column length and maxlength.
+        c.execute(sql, (self.indexer.maxlength + 5,))
+
     def add_actor_column(self):
         # update existing tables to have the new actor column
         tables = self.database_schema['tables']

roundup/backends/back_sqlite.py

@@ -219,6 +219,8 @@ def _add_fts5_table(self):
         )
 
     def fix_version_6_tables(self):
+        # note sqlite has no limit on column size so v6 fixes
+        # to __words._word length are not needed.
         # Add native full-text indexing table
         self._add_fts5_table()
 

roundup/backends/indexer_common.py

@@ -19,10 +19,11 @@ def __init__(self, db):
         self.stopwords = set(STOPWORDS)
         for word in db.config[('main', 'indexer_stopwords')]:
             self.stopwords.add(word)
-        # Do not index anything longer than 25 characters since that'll be
-        # gibberish (encoded text or somesuch) or shorter than 2 characters
+        # Do not index anything longer than maxlength characters since
+        # that'll be gibberish (encoded text or somesuch) or shorter
+        # than 2 characters
         self.minlength = 2
-        self.maxlength = 25
+        self.maxlength = 50
         self.language = db.config[('main','indexer_language')]
         # Some indexers have a query language. If that is the case,
         # we don't parse the user supplied query into a wordlist.

roundup/backends/rdbms_common.py

@@ -416,9 +416,9 @@ def fix_version_5_tables(self):
         pass
 
     def fix_version_6_tables(self):
-        # Default (used by mysql): NOOP
-        # sqlite/postgres override this to add fts
-        # full text search tables.
+        # Default (used by nobody): NOOP
+        # Each backend mysql, postgres, sqlite overrides this
+        # You would think ALTER commands would be the same but nooo.
         pass
 
     def _convert_journal_tables(self):

test/test_cgi.py

@@ -100,7 +100,66 @@ def testAddMessageNoEscape(self):
         self.assertEqual(cm([],'<i>x</i>\n<b>x</b>',False),
             ['<i>x</i><br />\n<b>x</b>'])
 
-class FormTestCase(FormTestParent, StringFragmentCmpHelper, unittest.TestCase):
+class testCsvExport(object):
+
+    def testCSVExport(self):
+        cl = self._make_client(
+            {'@columns': 'id,title,status,keyword,assignedto,nosy'},
+            nodeid=None, userid='1')
+        cl.classname = 'issue'
+
+        demo_id=self.db.user.create(username='demo', address='[email protected]',
+            roles='User', realname='demo')
+        key_id1=self.db.keyword.create(name='keyword1')
+        key_id2=self.db.keyword.create(name='keyword2')
+        self.db.issue.create(title='foo1', status='2', assignedto='4', nosy=['3',demo_id])
+        self.db.issue.create(title='bar2', status='1', assignedto='3', keyword=[key_id1,key_id2])
+        self.db.issue.create(title='baz32', status='4')
+        output = io.BytesIO()
+        cl.request = MockNull()
+        cl.request.wfile = output
+        # call export version that outputs names
+        actions.ExportCSVAction(cl).handle()
+        should_be=(s2b('"id","title","status","keyword","assignedto","nosy"\r\n'
+                       '"1","foo1","deferred","","Contrary, Mary","Bork, Chef;Contrary, Mary;demo"\r\n'
+                       '"2","bar2","unread","keyword1;keyword2","Bork, Chef","Bork, Chef"\r\n'
+                       '"3","baz32","need-eg","","",""\r\n'))
+        #print(should_be)
+        print(output.getvalue())
+        self.assertEqual(output.getvalue(), should_be)
+        output = io.BytesIO()
+        cl.request = MockNull()
+        cl.request.wfile = output
+        # call export version that outputs id numbers
+        actions.ExportCSVWithIdAction(cl).handle()
+        should_be = s2b('"id","title","status","keyword","assignedto","nosy"\r\n'
+                        "\"1\",\"foo1\",\"2\",\"[]\",\"4\",\"['3', '4', '5']\"\r\n"
+                        "\"2\",\"bar2\",\"1\",\"['1', '2']\",\"3\",\"['3']\"\r\n"
+                        '\"3\","baz32",\"4\","[]","None","[]"\r\n')
+        #print(should_be)
+        print(output.getvalue())
+        self.assertEqual(output.getvalue(), should_be)
+
+        # test full text search
+        cl = self._make_client(
+            {'@columns': 'id,title,status,keyword,assignedto,nosy',
+             "@search_text": "bar2"}, nodeid=None, userid='1')
+        cl.classname = 'issue'
+        output = io.BytesIO()
+        cl.request = MockNull()
+        cl.request.wfile = output
+
+        # call export version that outputs names
+        actions.ExportCSVAction(cl).handle()
+        should_be=(s2b('"id","title","status","keyword","assignedto","nosy"\r\n'
+                       '"2","bar2","unread","keyword1;keyword2","Bork, Chef","Bork, Chef"\r\n'))
+
+        # call export version that outputs id numbers
+        actions.ExportCSVWithIdAction(cl).handle()
+        should_be = s2b('"id","title","status","keyword","assignedto","nosy"\r\n'
+                        "\"2\",\"bar2\",\"1\",\"['1', '2']\",\"3\",\"['3']\"\r\n")
+
+class FormTestCase(FormTestParent, StringFragmentCmpHelper, testCsvExport, unittest.TestCase):
 
     def setUp(self):
         FormTestParent.setUp(self)
@@ -1810,44 +1869,6 @@ def testRoles(self):
         self.db.user.set('1', roles='')
         self.assertTrue(not item.hasRole(''))
 
-    def testCSVExport(self):
-        cl = self._make_client(
-            {'@columns': 'id,title,status,keyword,assignedto,nosy'},
-            nodeid=None, userid='1')
-        cl.classname = 'issue'
-
-        demo_id=self.db.user.create(username='demo', address='[email protected]',
-            roles='User', realname='demo')
-        key_id1=self.db.keyword.create(name='keyword1')
-        key_id2=self.db.keyword.create(name='keyword2')
-        self.db.issue.create(title='foo1', status='2', assignedto='4', nosy=['3',demo_id])
-        self.db.issue.create(title='bar2', status='1', assignedto='3', keyword=[key_id1,key_id2])
-        self.db.issue.create(title='baz32', status='4')
-        output = io.BytesIO()
-        cl.request = MockNull()
-        cl.request.wfile = output
-        # call export version that outputs names
-        actions.ExportCSVAction(cl).handle()
-        should_be=(s2b('"id","title","status","keyword","assignedto","nosy"\r\n'
-                       '"1","foo1","deferred","","Contrary, Mary","Bork, Chef;Contrary, Mary;demo"\r\n'
-                       '"2","bar2","unread","keyword1;keyword2","Bork, Chef","Bork, Chef"\r\n'
-                       '"3","baz32","need-eg","","",""\r\n'))
-        #print(should_be)
-        print(output.getvalue())
-        self.assertEqual(output.getvalue(), should_be)
-        output = io.BytesIO()
-        cl.request = MockNull()
-        cl.request.wfile = output
-        # call export version that outputs id numbers
-        actions.ExportCSVWithIdAction(cl).handle()
-        should_be = s2b('"id","title","status","keyword","assignedto","nosy"\r\n'
-                        "\"1\",\"foo1\",\"2\",\"[]\",\"4\",\"['3', '4', '5']\"\r\n"
-                        "\"2\",\"bar2\",\"1\",\"['1', '2']\",\"3\",\"['3']\"\r\n"
-                        '\"3\","baz32",\"4\","[]","None","[]"\r\n')
-        #print(should_be)
-        print(output.getvalue())
-        self.assertEqual(output.getvalue(), should_be)
-
     def testCSVExportCharset(self):
         cl = self._make_client(
             {'@columns': 'id,title,status,keyword,assignedto,nosy'},
@@ -2364,7 +2385,7 @@ def testTemplateSubdirectory(self):
         r = t.selectTemplate("user", "subdir/item")
         self.assertEqual("subdir/user.item", r)
 
-class SqliteNativeFtsCgiTest(unittest.TestCase, testFtsQuery):
+class SqliteNativeFtsCgiTest(unittest.TestCase, testFtsQuery, testCsvExport):
     """All of the rest of the tests use anydbm as the backend.
        In addtion to normal fts test, this class tests renderError
        when renderContext fails.
@@ -2382,6 +2403,11 @@ def setUp(self):
         # open the database
         self.db = self.instance.open('admin')
         self.db.tx_Source = "web"
+        self.db.user.create(username='Chef', address='[email protected]',
+            realname='Bork, Chef', roles='User')
+        self.db.user.create(username='mary', address='[email protected]',
+            roles='User', realname='Contrary, Mary')
+        self.db.post_init()
 
         # create a client instance and hijack write_html
         self.client = client.Client(self.instance, "user",
@@ -2429,6 +2455,30 @@ def testRenderContextBadFtsQuery(self):
         self.assertEqual(result, expected)
         self.assertEqual(self.client.response_code, 400)
 
+    #
+    # SECURITY
+    #
+    # XXX test all default permissions
+    def _make_client(self, form, classname='user', nodeid='1',
+           userid='2', template='item'):
+        cl = client.Client(self.instance, None, {'PATH_INFO':'/',
+            'REQUEST_METHOD':'POST'}, db_test_base.makeForm(form))
+        cl.classname = classname
+        if nodeid is not None:
+            cl.nodeid = nodeid
+        cl.db = self.db
+        #cl.db.Otk = MockNull()
+        #cl.db.Otk.data = {}
+        #cl.db.Otk.getall = self.data_get
+        #cl.db.Otk.set = self.data_set
+        cl.userid = userid
+        cl.language = ('en',)
+        cl._error_message = []
+        cl._ok_message = []
+        cl.template = template
+        return cl
+
+
 class SqliteNativeCgiTest(unittest.TestCase, testFtsQuery):
     """All of the rest of the tests use anydbm as the backend.
        This class tests renderContext for fulltext search.

test/test_indexer.py

@@ -123,7 +123,7 @@ def test_extremewords(self):
             pytest.skip("extremewords not tested for native FTS backends")
 
         short = "b"
-        long = "abcdefghijklmnopqrstuvwxyz"
+        long = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz"
         self.dex.add_text(('test', '1', 'a'), '%s hello world' % short)
         self.dex.add_text(('test', '2', 'a'), 'blah a %s world' % short)
         self.dex.add_text(('test', '3', 'a'), 'blah Blub river')

test/test_mysql.py

@@ -65,6 +65,44 @@ def setUp(self):
         mysqlOpener.setUp(self)
         DBTest.setUp(self)
 
+    def testUpgrade_6_to_7(self):
+
+        # load the database
+        self.db.issue.create(title="flebble frooz")
+        self.db.commit()
+
+        if self.db.database_schema['version'] != 7:
+            self.skipTest("This test only runs for database version 7")
+
+        self.db.database_schema['version'] = 6
+
+        # test by shrinking _words and trying to insert a long value
+        #    it should fail.
+        # run post-init
+        #    same test should succeed.
+
+        self.db.sql("alter table __words change column "
+                    "_word _word varchar(10)")
+
+        long_string = "a" * (self.db.indexer.maxlength + 5)
+
+        with self.assertRaises(MySQLdb.DataError) as ctx:
+            # DataError : Data too long for column '_word' at row 1
+            self.db.sql("insert into __words VALUES('%s',1)" % long_string)
+
+        self.assertIn("Data too long for column '_word'",
+                      ctx.exception.args[1])
+ 
+        # test upgrade altering row
+        self.db.post_init()
+
+        # This insert with text of expected column size should succeed
+        self.db.sql("insert into __words VALUES('%s',1)" % long_string)
+
+        # Verify it fails at one more than the expected column size
+        too_long_string = "a" * (self.db.indexer.maxlength + 6)
+        with self.assertRaises(MySQLdb.DataError) as ctx:
+            self.db.sql("insert into __words VALUES('%s',1)" % too_long_string)
 
 @skip_mysql
 class mysqlROTest(mysqlOpener, ROTest, unittest.TestCase):