Use URL to indicate POST action

Instead of passing 'action' as a field like a dummy.

etiquette/constants.py

@@ -102,11 +102,11 @@ SQL_USER = _sql_dictify(SQL_USER_COLUMNS)
 # Errors and warnings
 ERROR_DATABASE_OUTOFDATE = 'Database is out-of-date. {current} should be {new}. Please use utilities\\etiquette_upgrader.py'
 ERROR_INVALID_ACTION = 'Invalid action'
-ERROR_NO_SUCH_TAG = 'Doesn\'t exist'
+ERROR_NO_SUCH_TAG = 'Tag "{tag}" does not exist'
 ERROR_NO_TAG_GIVEN = 'No tag name supplied'
 ERROR_SYNONYM_ITSELF = 'Cant apply synonym to itself'
-ERROR_TAG_TOO_LONG = '{tag} is too long'
+ERROR_TAG_TOO_LONG = '"{tag}" is too long'
-ERROR_TAG_TOO_SHORT = '{tag} has too few valid chars'
+ERROR_TAG_TOO_SHORT = '"{tag}" has too few valid chars'
 ERROR_RECURSIVE_GROUPING = 'Recursive grouping'
 WARNING_MINMAX_INVALID = 'Field "{field}": "{value}" is not a valid request. Ignored.'
 WARNING_MINMAX_OOO = 'Field "{field}": minimum "{min}" maximum "{max}" are out of order. Ignored.'

etiquette/decorators.py

@@ -7,21 +7,36 @@ import warnings
 from . import jsonify
 
 
-def required_fields(fields):
+def required_fields(fields, forbid_whitespace=False):
     '''
     Declare that the endpoint requires certain POST body fields. Without them,
     we respond with 400 and a message.
+
+    forbid_whitespace:
+        If True, then providing the field is not good enough. It must also
+        contain at least some non-whitespace characters.
     '''
-    def with_required_fields(function):
+    def wrapper(function):
         @functools.wraps(function)
         def wrapped(*args, **kwargs):
-            if not all(field in request.form for field in fields):
+            for requirement in fields:
-                response = {'error': 'Required fields: %s' % ', '.join(fields)}
+                if (
+                    requirement not in request.form or
+                    (
+                        forbid_whitespace and
+                        request.form[requirement].strip() == ''
+                    )
+                ):
+                    response = {
+                        'error_type': 'MISSING_FIELDS',
+                        'error_message': 'Required fields: %s' % ', '.join(fields),
+                    }
                     response = jsonify.make_json_response(response, status=400)
                     return response
+
             return function(*args, **kwargs)
         return wrapped
-    return with_required_fields
+    return wrapper
 
 def not_implemented(function):
     '''

etiquette/objects.py

@@ -851,8 +851,11 @@ class Tag(ObjectBase, GroupableMixin):
         they always resolve to the master tag before application.
         '''
         synname = self.photodb.normalize_tagname(synname)
+        if synname == self.name:
+            raise exceptions.NoSuchSynonym(synname)
+
         cur = self.photodb.sql.cursor()
-        cur.execute('SELECT * FROM tag_synonyms WHERE name == ?', [synname])
+        cur.execute('SELECT * FROM tag_synonyms WHERE mastername == ? AND name == ?', [self.name, synname])
         fetch = cur.fetchone()
         if fetch is None:
             raise exceptions.NoSuchSynonym(synname)

etiquette_site.py

@@ -64,40 +64,53 @@ def delete_tag(tag):
 def delete_synonym(synonym):
     synonym = synonym.split('+')[-1].split('.')[-1]
     synonym = P.normalize_tagname(synonym)
-    try:
+
     master_tag = P.get_tag(synonym)
-    except exceptions.NoSuchTag:
-        flask.abort(404, 'That synonym doesnt exist')
-
-    if synonym not in master_tag.synonyms():
-        flask.abort(400, 'That name is not a synonym')
-
     master_tag.remove_synonym(synonym)
+
     return {'action':'delete_synonym', 'synonym': synonym}
 
+def P_wrapper(function):
+    def P_wrapped(thingid, response_type='html'):
+        ret = function(thingid)
+        if not isinstance(ret, str):
+            return ret
+
+        if response_type == 'html':
+            flask.abort(404, ret)
+        else:
+            response = jsonify.make_json_response({'error': ret})
+            flask.abort(response)
+
+    return P_wrapped
+
+@P_wrapper
 def P_album(albumid):
     try:
         return P.get_album(albumid)
     except exceptions.NoSuchAlbum:
-        flask.abort(404, 'That album doesnt exist')
+        return 'That album doesnt exist'
 
+@P_wrapper
 def P_photo(photoid):
     try:
         return P.get_photo(photoid)
     except exceptions.NoSuchPhoto:
-        flask.abort(404, 'That photo doesnt exist')
+        return 'That photo doesnt exist'
 
+@P_wrapper
 def P_tag(tagname):
     try:
         return P.get_tag(tagname)
     except exceptions.NoSuchTag as e:
-        flask.abort(404, 'That tag doesnt exist: %s' % e)
+        return 'That tag doesnt exist: %s' % e
 
+@P_wrapper
 def P_user(username):
     try:
         return P.get_user(username=username)
     except exceptions.NoSuchUser as e:
-        flask.abort(404, 'That user doesnt exist: %s' % e)
+        return 'That user doesnt exist: %s' % e
 
 def send_file(filepath):
     '''
@@ -615,10 +628,9 @@ def get_user_json(username):
     return user
 
 
-@site.route('/album/<albumid>', methods=['POST'])
+@site.route('/album/<albumid>/add_tag', methods=['POST'])
-@site.route('/album/<albumid>.json', methods=['POST'])
 @session_manager.give_token
-def post_edit_album(albumid):
+def post_album_add_tag(albumid):
     '''
     Edit the album's title and description.
     Apply a tag to every photo in the album.
@@ -626,10 +638,7 @@ def post_edit_album(albumid):
     response = {}
     album = P_album(albumid)
 
-    if 'add_tag' in request.form:
+    tag = request.form['tagname'].strip()
-        action = 'add_tag'
-
-        tag = request.form[action].strip()
     try:
         tag = P_tag(tag)
     except exceptions.NoSuchTag:
@@ -638,97 +647,92 @@ def post_edit_album(albumid):
     recursive = request.form.get('recursive', False)
     recursive = helpers.truthystring(recursive)
     album.add_tag_to_all(tag, nested_children=recursive)
-        response['action'] = action
+    response['action'] = 'add_tag'
     response['tagname'] = tag.name
     return jsonify.make_json_response(response)
 
 
-@site.route('/photo/<photoid>', methods=['POST'])
+def post_photo_add_remove_tag_core(photoid, tagname, add_or_remove):
-@site.route('/photo/<photoid>.json', methods=['POST'])
+    photo = P_photo(photoid, response_type='json')
-@session_manager.give_token
+    tag = P_tag(tagname, response_type='json')
-def post_edit_photo(photoid):
-    '''
-    Add and remove tags from photos.
-    '''
-    response = {}
-    photo = P_photo(photoid)
 
-    if 'add_tag' in request.form:
+    if add_or_remove == 'add':
-        action = 'add_tag'
+        photo.add_tag(tag)
-        method = photo.add_tag
+    elif add_or_remove == 'remove':
-    elif 'remove_tag' in request.form:
+        photo.remove_tag(tag)
-        action = 'remove_tag'
-        method = photo.remove_tag
-    else:
-        flask.abort(400, 'Invalid action')
 
-    tag = request.form[action].strip()
+    response = {'tagname': tagname}
-    if tag == '':
-        flask.abort(400, 'No tag supplied')
-
-    try:
-        tag = P.get_tag(tag)
-    except exceptions.NoSuchTag:
-        response = {'error': 'That tag doesnt exist', 'tagname': tag}
-        return jsonify.make_json_response(response, status=404)
-
-    method(tag)
-    response['action'] = action
-    #response['tagid'] = tag.id
-    response['tagname'] = tag.name
     return jsonify.make_json_response(response)    
 
-
+@site.route('/photo/<photoid>/add_tag', methods=['POST'])
-@site.route('/tags', methods=['POST'])
+@decorators.required_fields(['tagname'], forbid_whitespace=True)
-@session_manager.give_token
+def post_photo_add_tag(photoid):
-def post_edit_tags():
     '''
-    Create and delete tags and synonyms.
+    Add a tag to this photo.
     '''
-    #print(request.form)
+    return post_photo_add_remove_tag_core(photoid, request.form['tagname'], 'add')
-    status = 200
-    if 'create_tag' in request.form:
-        action = 'create_tag'
-        method = create_tag
-    elif 'delete_tag_synonym' in request.form:
-        action = 'delete_tag_synonym'
-        method = delete_synonym
-    elif 'delete_tag' in request.form:
-        action = 'delete_tag'
-        method = delete_tag
-    else:
-        status = 400
-        response = {'error': constants.ERROR_INVALID_ACTION}
 
-    if status == 200:
+@site.route('/photo/<photoid>/remove_tag', methods=['POST'])
-        tag = request.form[action].strip()
+@decorators.required_fields(['tagname'], forbid_whitespace=True)
-        if tag == '':
+def post_photo_remove_tag(photoid):
-            response = {'error': constants.ERROR_NO_TAG_GIVEN}
+    '''
-            status = 400
+    Remove a tag from this photo.
+    '''
+    return post_photo_add_remove_tag_core(photoid, request.form['tagname'], 'remove')
 
-    if status == 200:
+
-        # expect the worst
+def post_tag_create_delete_core(tagname, function):
-        status = 400
     try:
-            response = method(tag)
+        response = function(tagname)
     except exceptions.TagTooLong:
-            response = {'error': constants.ERROR_TAG_TOO_LONG.format(tag=tag), 'tagname': tag}
+        error_type = 'TAG_TOO_LONG'
+        error_message = constants.ERROR_TAG_TOO_LONG.format(tag=tagname)
     except exceptions.TagTooShort:
-            response = {'error': constants.ERROR_TAG_TOO_SHORT.format(tag=tag), 'tagname': tag}
+        error_type = 'TAG_TOO_SHORT'
+        error_message = constants.ERROR_TAG_TOO_SHORT.format(tag=tagname)
     except exceptions.CantSynonymSelf:
-            response = {'error': constants.ERROR_SYNONYM_ITSELF, 'tagname': tag}
+        error_type = 'TAG_SYNONYM_ITSELF'
-        except exceptions.NoSuchTag as e:
+        error_message = constants.ERROR_SYNONYM_ITSELF
-            response = {'error': constants.ERROR_NO_SUCH_TAG, 'tagname': tag}
     except exceptions.RecursiveGrouping as e:
-            response = {'error': constants.ERROR_RECURSIVE_GROUPING, 'tagname': tag}
+        error_type = 'RECURSIVE_GROUPING'
-        except ValueError as e:
+        error_message = constants.ERROR_RECURSIVE_GROUPING
-            response = {'error': e.args[0], 'tagname': tag}
+    except exceptions.NoSuchTag as e:
+        error_type = 'NO_SUCH_TAG'
+        error_message = constants.ERROR_NO_SUCH_TAG.format(tag=tagname)
+    else:
+        error_type = None
+    
+    if error_type is not None:
+        status = 400
+        response = {'error_type': error_type, 'error_message': error_message}
     else:
         status = 200
 
-    response = json.dumps(response)
+    return jsonify.make_json_response(response, status=status)
-    response = flask.Response(response, status=status)
+
-    return response
+@site.route('/tags/create_tag', methods=['POST'])
+@decorators.required_fields(['tagname'], forbid_whitespace=True)
+def post_tag_create():
+    '''
+    Create a tag.
+    '''
+    return post_tag_create_delete_core(request.form['tagname'], create_tag)
+
+@site.route('/tags/delete_tag', methods=['POST'])
+@decorators.required_fields(['tagname'], forbid_whitespace=True)
+def post_tag_delete():
+    '''
+    Delete a tag.
+    '''
+    return post_tag_create_delete_core(request.form['tagname'], delete_tag)
+
+@site.route('/tags/delete_synonym', methods=['POST'])
+@decorators.required_fields(['tagname'], forbid_whitespace=True)
+def post_tag_delete_synonym():
+    '''
+    Delete a synonym.
+    '''
+    return post_tag_create_delete_core(request.form['tagname'], delete_synonym)
 
 
 @site.route('/apitest')

static/common.js

@@ -32,8 +32,12 @@ function post(url, data, callback)
             if (callback != null)
             {
                 var text = request.responseText;
+                console.log(request);
                 console.log(text);
-                callback(JSON.parse(text));
+                var response = JSON.parse(text);
+                response["_request_url"] = url;
+                response["_status"] = status;
+                callback(response);
             }
         }
     };

templates/photo.html

@@ -202,17 +202,17 @@ add_tag_box.onkeydown = function(){entry_with_history_hook(add_tag_box, add_tag_
 function add_photo_tag(photoid, tagname, callback)
 {
     if (tagname === ""){return}
-    var url = "/photo/" + photoid;
+    var url = "/photo/" + photoid + "/add_tag";
     data = new FormData();
-    data.append("add_tag", tagname);
+    data.append("tagname", tagname);
     return post(url, data, callback);
 }
 function remove_photo_tag(photoid, tagname, callback)
 {
     if (tagname === ""){return}
-    var url = "/photo/" + photoid;
+    var url = "/photo/" + photoid + "/remove_tag";
     data = new FormData();
-    data.append("remove_tag", tagname);
+    data.append("tagname", tagname);
     return post(url, data, callback);
 }
 function submit_tag(callback)
@@ -228,18 +228,22 @@ function receive_callback(response)
         message_positivity = "message_negative";
         message_text = '"' + tagname + '" ' + response["error"];
     }
-    else if ("action" in response)
+    else
     {
-        var action = response["action"];
+        var action;
         message_positivity = "message_positive";
-        if (action == "add_tag")
+        if (response["_request_url"].includes("add_tag"))
         {
             message_text = "Added tag " + tagname;
         }
-        else if (action == "remove_tag")
+        else if (response["_request_url"].includes("remove_tag"))
         {
             message_text = "Removed tag " + tagname;
         }
+        else
+        {
+            return;
+        }
     }
     create_message_bubble(message_area, message_positivity, message_text, 8000);
 }

templates/tags.html

@@ -102,14 +102,14 @@ function submit_tag(callback)
 function edit_tags(action, tagname, callback)
 {
     if (tagname === ""){return}
-    var url = "/tags";
+    var url = "/tags/" + action;
     data = new FormData();
-    data.append(action, tagname);
+    data.append("tagname", tagname);
     return post(url, data, callback);
 }
 function delete_tag_synonym(tagname, callback)
 {
-    return edit_tags("delete_tag_synonym", tagname, callback);
+    return edit_tags("delete_synonym", tagname, callback);
 }
 function delete_tag(tagname, callback)
 {

test_etiquette_site.py

@@ -0,0 +1,110 @@
+import json
+import os
+import unittest
+import random
+import requests
+import string
+
+URL = 'http://localhost:5000'
+
+def randstring(length):
+    return ''.join(random.choice(string.ascii_letters) for x in range(length))
+
+class EtiquetteSiteTest(unittest.TestCase):
+    pass
+
+
+class TagTest(EtiquetteSiteTest):
+    '''
+    Test the tag editor.
+    '''
+    def _helper(self, action, tagname):
+        url = URL + '/tags/' + action
+        data = {'tagname': tagname}
+        print(action, data)
+        response = requests.post(url, data=data)
+        print(response.status_code)
+        #print(response.text)
+        j = response.json()
+        print(json.dumps(j, indent=4, sort_keys=True))
+        print()
+        return
+
+        
+    def _create_helper(self, tagname):
+        return self._helper('create_tag', tagname)
+
+    def _delete_helper(self, tagname):
+        return self._helper('delete_tag', tagname)
+
+    def _delete_synonym_helper(self, tagname):
+        return self._helper('delete_synonym', tagname)
+
+    def test_create_tag(self):
+        self._create_helper('1')
+
+        # new tag
+        tagname = randstring(10)
+        self._create_helper(tagname)
+
+        # new tags with grouping
+        tagname = '.'.join(randstring(3) for x in range(2))
+        self._create_helper(tagname)
+
+        # new tag with new synonym
+        tagname = '+'.join(randstring(3) for x in range(2))
+        self._create_helper(tagname)
+
+        # existing tag with new synonym
+        tagname = randstring(10)
+        self._create_helper('1.' + tagname)
+
+        # renaming
+        self._create_helper('testing')
+        self._create_helper('testing=tester')
+        self._create_helper('tester=testing')
+
+        # trying to rename nonexisting
+        tagname = randstring(10)
+        self._create_helper(tagname + '=nonexist')
+        self._create_helper(tagname + '+nonexist')
+
+        # length errors
+        tagname = randstring(100)
+        self._create_helper(tagname)
+        self._create_helper('')
+        self._create_helper('*?%$')
+
+        # regrouping.
+        self._create_helper('test1.test2')
+        self._create_helper('test3.test2')
+        self._create_helper('test1.test2')
+        self._create_helper('test3.test1.test2')
+
+    def test_delete_tag(self):
+        self._create_helper('1')
+        self._delete_helper('1')
+
+        self._create_helper('1.2')
+        self._delete_helper('2')
+
+        self._create_helper('1.2')
+        self._delete_helper('1')
+
+    def test_delete_synonym(self):
+        tagname = randstring(5)
+        self._create_helper('testing+' + tagname)
+        self._create_helper('testing+' + tagname)
+        self._delete_synonym_helper(tagname)
+
+        self._create_helper('testing+' + tagname)
+        self._delete_synonym_helper('testing+' + tagname)
+
+        self._create_helper('tester.testing+' + tagname)
+        self._delete_synonym_helper('tester.testing+' + tagname)
+
+
+
+
+if __name__ == '__main__':
+    unittest.main()