[#652][#653] changed BookmarkVar into a namedtuple & applied some min…

…or refactoring

bukuserver/api.py

@@ -111,13 +111,13 @@ def get(self, rec_id: Union[int, None]):
             result = {'bookmarks': []}  # type: Dict[str, Any]
             for bookmark in all_bookmarks:
                 result_bookmark = {
-                    'url': bookmark[1],
-                    'title': bookmark[2],
-                    'tags': [x for x in bookmark[3].split(',') if x],
-                    'description': bookmark[4]
+                    'url': bookmark.url,
+                    'title': bookmark.title,
+                    'tags': bookmark.taglist,
+                    'description': bookmark.desc
                 }
                 if not request.path.startswith('/api/'):
-                    result_bookmark['id'] = bookmark[0]
+                    result_bookmark['id'] = bookmark.id
                 result['bookmarks'].append(result_bookmark)
             res = jsonify(result)
         else:
@@ -127,10 +127,10 @@ def get(self, rec_id: Union[int, None]):
                 res = response_bad()
             else:
                 res = jsonify({
-                    'url': bookmark[1],
-                    'title': bookmark[2],
-                    'tags': [x for x in bookmark[3].split(',') if x],
-                    'description': bookmark[4]
+                    'url': bookmark.url,
+                    'title': bookmark.title,
+                    'tags': bookmark.taglist,
+                    'description': bookmark.desc,
                 })
         return res
 
@@ -178,10 +178,10 @@ def get(self, starting_id: int, ending_id: int):
         for i in range(starting_id, ending_id + 1, 1):
             bookmark = bukudb.get_rec_by_id(i)
             result['bookmarks'][i] = {
-                'url': bookmark[1],
-                'title': bookmark[2],
-                'tags': [x for x in bookmark[3].split(',') if x],
-                'description': bookmark[4]
+                'url': bookmark.url,
+                'title': bookmark.title,
+                'tags': bookmark.taglist,
+                'description': bookmark.desc,
             }
         return jsonify(result)
 
@@ -236,11 +236,11 @@ def get(self):
         res = None
         for bookmark in bukudb.searchdb(keywords, all_keywords, deep, regex):
             result_bookmark = {
-                'id': bookmark[0],
-                'url': bookmark[1],
-                'title': bookmark[2],
-                'tags': list(filter(lambda x: x, bookmark[3].split(','))),
-                'description': bookmark[4]
+                'id': bookmark.id,
+                'url': bookmark.url,
+                'title': bookmark.title,
+                'tags': bookmark.taglist,
+                'description': bookmark.desc,
             }
             result['bookmarks'].append(result_bookmark)
         current_app.logger.debug('total bookmarks:{}'.format(len(result['bookmarks'])))
@@ -266,7 +266,7 @@ def delete(self):
         bukudb = getattr(flask.g, 'bukudb', get_bukudb())
         res = None
         for bookmark in bukudb.searchdb(keywords, all_keywords, deep, regex):
-            if not bukudb.delete_rec(bookmark[0]):
+            if not bukudb.delete_rec(bookmark.id):
                 res = response_bad()
         return res or response_ok()
 

bukuserver/views.py

@@ -259,15 +259,7 @@ def get_list(self, page, sort_field, sort_desc, _, filters, page_size=None):
         for bookmark in bookmarks:
             bm_sns = types.SimpleNamespace(id=None, url=None, title=None, tags=None, description=None)
             for field in list(BookmarkField):
-                if field == BookmarkField.TAGS:
-                    value = bookmark[field.value]
-                    if value.startswith(","):
-                        value = value[1:]
-                    if value.endswith(","):
-                        value = value[:-1]
-                    setattr(bm_sns, field.name.lower(), value)
-                else:
-                    setattr(bm_sns, field.name.lower(), bookmark[field.value])
+                setattr(bm_sns, field.name.lower(), format_value(field, bookmark))
             data.append(bm_sns)
         return count, data
 
@@ -277,17 +269,8 @@ def get_one(self, id):
             return None
         bm_sns = types.SimpleNamespace(id=None, url=None, title=None, tags=None, description=None)
         for field in list(BookmarkField):
-            if field == BookmarkField.TAGS and bookmark[field.value].startswith(","):
-                value = bookmark[field.value]
-                if value.startswith(","):
-                    value = value[1:]
-                if value.endswith(","):
-                    value = value[:-1]
-                setattr(bm_sns, field.name.lower(), value.replace(',', ', '))
-            else:
-                setattr(bm_sns, field.name.lower(), bookmark[field.value])
-            if field == BookmarkField.URL:
-                session['netloc'] = urlparse(bookmark[field.value]).netloc
+            setattr(bm_sns, field.name.lower(), format_value(field, bookmark, spacing=' '))
+        session['netloc'] = urlparse(bookmark.url).netloc
         return bm_sns
 
     def get_pk_value(self, model):
@@ -739,3 +722,7 @@ def page_of(items, size, idx):
 
 def filter_key(flt, idx=''):
     return 'flt' + str(idx) + '_' + BookmarkModelView._filter_arg(flt)
+
+def format_value(field, bookmark, spacing=''):
+    s = bookmark[field.value]
+    return s if field != BookmarkField.TAGS else s.strip(',').replace(',', ','+spacing)

requirements.txt

@@ -5,4 +5,4 @@ cryptography>=1.2.3
 html5lib>=1.0.1
 setuptools
 urllib3>=1.23
-pyreadline; sys_platform == 'windows'
+pyreadline3; sys_platform == 'win32'

setup.py

@@ -54,8 +54,8 @@
     'certifi',
     'cryptography>=1.2.3',
     'html5lib>=1.0.1',
-    'pyreadline; sys_platform == \'windows\'',
     'urllib3>=1.23',
+    'pyreadline3; sys_platform == \'win32\'',
 ]
 
 setup(

tests/test_buku.py

@@ -11,7 +11,11 @@
 
 import pytest
 
-from buku import DELIM, is_int, prep_tag_search
+from buku import DELIM, FIELD_FILTER, ALL_FIELDS, is_int, prep_tag_search, print_rec_with_filter
+
+def strip_query(s):
+    if isinstance(s, str) and '?' in s:
+        return s.split('?')[0] + '~'
 
 only_python_3_5 = pytest.mark.skipif(sys.version_info < (3, 5), reason="requires Python 3.5 or later")
 
@@ -133,86 +137,25 @@ def test_parse_tags_no_args():
     assert buku.parse_tags() == DELIM
 
 
-@pytest.mark.parametrize(
-    "records, field_filter, exp_res",
-    [
-        [
-            [
-                (1, "http://url1.com", "title1", ",tag1,"),
-                (2, "http://url2.com", "title2", ",tag1,tag2,"),
-            ],
-            1,
-            ["1\thttp://url1.com", "2\thttp://url2.com"],
-        ],
-        [
-            [
-                (1, "http://url1.com", "title1", ",tag1,"),
-                (2, "http://url2.com", "title2", ",tag1,tag2,"),
-            ],
-            2,
-            ["1\thttp://url1.com\ttag1", "2\thttp://url2.com\ttag1,tag2"],
-        ],
-        [
-            [
-                (1, "http://url1.com", "title1", ",tag1,"),
-                (2, "http://url2.com", "title2", ",tag1,tag2,"),
-            ],
-            3,
-            ["1\ttitle1", "2\ttitle2"],
-        ],
-        [
-            [
-                (1, "http://url1.com", "title1", ",tag1,"),
-                (2, "http://url2.com", "title2", ",tag1,tag2,"),
-            ],
-            4,
-            [
-                "1\thttp://url1.com\ttitle1\ttag1",
-                "2\thttp://url2.com\ttitle2\ttag1,tag2",
-            ],
-        ],
-        [
-            [
-                (1, "http://url1.com", "title1", ",tag1,"),
-                (2, "http://url2.com", "title2", ",tag1,tag2,"),
-            ],
-            10,
-            ["http://url1.com", "http://url2.com"],
-        ],
-        [
-            [
-                (1, "http://url1.com", "title1", ",tag1,"),
-                (2, "http://url2.com", "title2", ",tag1,tag2,"),
-            ],
-            20,
-            ["http://url1.com\ttag1", "http://url2.com\ttag1,tag2"],
-        ],
-        [
-            [
-                (1, "http://url1.com", "title1", ",tag1,"),
-                (2, "http://url2.com", "title2", ",tag1,tag2,"),
-            ],
-            30,
-            ["title1", "title2"],
-        ],
-        [
-            [
-                (1, "http://url1.com", "title1", ",tag1,"),
-                (2, "http://url2.com", "title2", ",tag1,tag2,"),
-            ],
-            40,
-            ["http://url1.com\ttitle1\ttag1", "http://url2.com\ttitle2\ttag1,tag2"],
-        ],
-    ],
-)
-def test_print_rec_with_filter(records, field_filter, exp_res):
-    """test func."""
-    with mock.patch("buku.print", create=True) as m_print:
-        import buku
-
-        buku.print_rec_with_filter(records, field_filter)
-        for res in exp_res:
-            m_print.assert_any_call(res)
+@pytest.mark.parametrize("field_filter, exp_res", [
+    (0, ["1. title1\n   > http://url1.com\n   + desc1\n   # tag1\n",
+         "2. title2\n   > http://url2.com\n   + desc2\n   # tag1,tag2\n"]),
+    (1, ["1\thttp://url1.com", "2\thttp://url2.com"]),
+    (2, ["1\thttp://url1.com\ttag1", "2\thttp://url2.com\ttag1,tag2"]),
+    (3, ["1\ttitle1", "2\ttitle2"]),
+    (4, ["1\thttp://url1.com\ttitle1\ttag1", "2\thttp://url2.com\ttitle2\ttag1,tag2"]),
+    (5, ["1\ttitle1\ttag1", "2\ttitle2\ttag1,tag2"]),
+    (10, ["http://url1.com", "http://url2.com"]),
+    (20, ["http://url1.com\ttag1", "http://url2.com\ttag1,tag2"]),
+    (30, ["title1", "title2"]),
+    (40, ["http://url1.com\ttitle1\ttag1", "http://url2.com\ttitle2\ttag1,tag2"]),
+    (50, ["title1\ttag1", "title2\ttag1,tag2"]),
+])
+def test_print_rec_with_filter(capfd, field_filter, exp_res):
+    records = [(1, "http://url1.com", "title1", ",tag1,", "desc1"),
+               (2, "http://url2.com", "title2", ",tag1,tag2,", "desc2")]
+    print_rec_with_filter(records, field_filter)
+    assert capfd.readouterr().out == ''.join(f'{s}\n' for s in exp_res)
 
 
 @pytest.mark.parametrize(
@@ -263,24 +206,22 @@ def test_edit_at_prompt(nav, is_editor_valid_retval, edit_rec_retval):
             obj.add_rec(*edit_rec_retval)
 
 
-@pytest.mark.parametrize("field_filter, single_record", product(list(range(4)), [True, False]))
+@pytest.mark.parametrize('single_record', [True, False])
+@pytest.mark.parametrize('field_filter', [0, 1, 2, 3, 4, 5, 10, 20, 30, 40, 50])
 def test_format_json(field_filter, single_record):
-    """test func."""
-    resultset = [["row{}".format(x) for x in range(5)]]
-    if field_filter == 1:
-        marks = {"uri": "row1"}
-    elif field_filter == 2:
-        marks = {"uri": "row1", "tags": "row3"[1:-1]}
-    elif field_filter == 3:
-        marks = {"title": "row2"}
-    else:
-        marks = {
-            "index": "row0",
-            "uri": "row1",
-            "title": "row2",
-            "description": "row4",
-            "tags": "row3"[1:-1],
-        }
+    resultset = [[f'<row{x}>' for x in range(5)]]
+    fields = FIELD_FILTER.get(field_filter, ALL_FIELDS)
+    marks = {}
+    if 'id' in fields:
+        marks['index'] = '<row0>'
+    if 'url' in fields:
+        marks['uri'] = '<row1>'
+    if 'title' in fields:
+        marks['title'] = '<row2>'
+    if 'tags' in fields:
+        marks['tags'] = 'row3'
+    if 'desc' in fields:
+        marks['description'] = '<row4>'
     if not single_record:
         marks = [marks]
 
@@ -591,6 +532,7 @@ def test_sigint_handler(capsys):
             ),
         ],
     ],
+    ids=strip_query,
 )
 def test_network_handler_with_url(url, exp_res):
     """test func."""

tests/test_bukuDb.py

@@ -1084,34 +1084,34 @@ def test_add_rec_add_invalid_url(caplog, url):
 @pytest.mark.parametrize(
     "kwargs, exp_arg",
     [
-        [{"url": "example.com"}, ("example.com", "Example Domain", ",", "", 0)],
+        [{"url": "example.com"}, ("example.com", "Example Domain", ",", "", False)],
         [
             {"url": "http://example.com"},
-            ("http://example.com", "Example Domain", ",", "", 0),
+            ("http://example.com", "Example Domain", ",", "", False),
         ],
         [
-            {"url": "http://example.com", "immutable": 1},
-            ("http://example.com", "Example Domain", ",", "", 1),
+            {"url": "http://example.com", "immutable": True},
+            ("http://example.com", "Example Domain", ",", "", True),
         ],
         [
             {"url": "http://example.com", "desc": "randomdesc"},
-            ("http://example.com", "Example Domain", ",", "randomdesc", 0),
+            ("http://example.com", "Example Domain", ",", "randomdesc", False),
         ],
         [
             {"url": "http://example.com", "title_in": "randomtitle"},
-            ("http://example.com", "randomtitle", ",", "", 0),
+            ("http://example.com", "randomtitle", ",", "", False),
         ],
         [
             {"url": "http://example.com", "tags_in": "tag1"},
-            ("http://example.com", "Example Domain", ",tag1,", "", 0),
+            ("http://example.com", "Example Domain", ",tag1,", "", False),
         ],
         [
             {"url": "http://example.com", "tags_in": ",tag1"},
-            ("http://example.com", "Example Domain", ",tag1,", "", 0),
+            ("http://example.com", "Example Domain", ",tag1,", "", False),
         ],
         [
             {"url": "http://example.com", "tags_in": ",tag1,"},
-            ("http://example.com", "Example Domain", ",tag1,", "", 0),
+            ("http://example.com", "Example Domain", ",tag1,", "", False),
         ],
     ],
 )