Merge pull request #63 from geoadmin/bug-bgdiinf-sb-3129-randomize #m…

…ajor

BGDIINF_SB-3129: Replaced short_id generator

.pylintrc

@@ -522,6 +522,6 @@ min-public-methods=2
 
 # Exceptions that will emit a warning when being caught. Defaults to
 # "BaseException, Exception".
-overgeneral-exceptions=BaseException,
-                       Exception,
-                       StandardError
+overgeneral-exceptions=builtins.BaseException,
+                       builtins.Exception,
+                       builtins.StandardError

Pipfile

@@ -4,21 +4,22 @@ verify_ssl = true
 name = "pypi"
 
 [packages]
-boto3 = "~=1.23.0"
-logging-utilities = "~=3.0"
-Flask = "~=2.1.0"
-gevent = "~=21.12.0"
-gunicorn = "~=20.1.0"
-PyYAML = ">=5.4"
-python-dotenv = "~=0.20.0"
-validators = "~=0.19.0"
+boto3 = "~=1.28"
+logging-utilities = "~=4.0"
+Flask = "~=3.0"
+gevent = "~=23.9"
+gunicorn = "~=21.2"
+PyYAML = "~=6.0"
+python-dotenv = "~=1.0"
+validators = "~=0.22"
+nanoid = "~=2.0"
 
 [dev-packages]
-yapf = "~=0.30.0"
-moto = "~=3.1.9"
+yapf = "~=0.40"
+moto = "~=4.2"
 nose2 = "*"
 pylint = "*"
 pylint-flask = "*"
 
 [requires]
-python_version = "3.9"
+python_version = "3.11"

README.md

@@ -194,15 +194,17 @@ The service is configured by Environment Variable:
 
 | Env Variable                | Default                    | Description                                                                                                                                                                      |
 | --------------------------- | -------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| LOGGING_CFG                 | logging-cfg-local.yml      | Logging configuration file to use.                                                                                                                                               |
-| AWS_ACCESS_KEY_ID           | None                       | Necessary credential to access dynamodb                                                                                                                                          |
-| AWS_SECRET_ACCESS_KEY       | None                       | AWS_SECRET_ACCESS_KEY                                                                                                                                                            |  |
-| AWS_DYNAMODB_TABLE_NAME     |                            | The dynamodb table name                                                                                                                                                          |
-| AWS_DEFAULT_REGION          | eu-central-1               | The AWS region in which the table is hosted.                                                                                                                                     |
-| AWS_ENDPOINT_URL            |                            | The AWS endpoint url to use                                                                                                                                                      |
-| ALLOWED_DOMAINS             | `.*`                       | A comma separated list of allowed domains names                                                                                                                                  |
-| FORWARED_ALLOW_IPS          | `*`                        | Sets the gunicorn `forwarded_allow_ips` (see https://docs.gunicorn.org/en/stable/settings.html#forwarded-allow-ips). This is required in order to `secure_scheme_headers` works. |
-| FORWARDED_PROTO_HEADER_NAME | `X-Forwarded-Proto`        | Sets gunicorn `secure_scheme_headers` parameter to `{FORWARDED_PROTO_HEADER_NAME: 'https'}`, see https://docs.gunicorn.org/en/stable/settings.html#secure-scheme-headers.        |
-| CACHE_CONTROL               | `public, max-age=31536000` | Cache Control header value of the `GET /<shortlink>` endpoint                                                                                                                    |
-| CACHE_CONTROL_4XX           | `public, max-age=3600`     | Cache Control header for 4XX responses                                                                                                                                           |
-| GUNICORN_WORKER_TMP_DIR     | `None`                     | This should be set to an tmpfs file system for better performance. See https://docs.gunicorn.org/en/stable/settings.html#worker-tmp-dir.                                         |
+| LOGGING_CFG                   | `logging-cfg-local.yml`                   | Logging configuration file to use.                                                                                                                                               |
+| AWS_ACCESS_KEY_ID             |                                           | Necessary credential to access dynamodb                                                                                                                                          |
+| AWS_SECRET_ACCESS_KEY         |                                           | AWS_SECRET_ACCESS_KEY                                                                                                                                                            |  |
+| AWS_DYNAMODB_TABLE_NAME       |                                           | The dynamodb table name                                                                                                                                                          |
+| AWS_DEFAULT_REGION            | eu-central-1                              | The AWS region in which the table is hosted.                                                                                                                                     |
+| AWS_ENDPOINT_URL              |                                           | The AWS endpoint url to use                                                                                                                                                      |
+| ALLOWED_DOMAINS               | `.*`                                      | A comma separated list of allowed domains names                                                                                                                                  |
+| FORWARED_ALLOW_IPS            | `*`                                       | Sets the gunicorn `forwarded_allow_ips` (see https://docs.gunicorn.org/en/stable/settings.html#forwarded-allow-ips). This is required in order to `secure_scheme_headers` works. |
+| FORWARDED_PROTO_HEADER_NAME   | `X-Forwarded-Proto`                       | Sets gunicorn `secure_scheme_headers` parameter to `{FORWARDED_PROTO_HEADER_NAME: 'https'}`, see https://docs.gunicorn.org/en/stable/settings.html#secure-scheme-headers.        |
+| CACHE_CONTROL                 | `public, max-age=31536000`                | Cache Control header value of the `GET /<shortlink>` endpoint                                                                                                                    |
+| CACHE_CONTROL_4XX             | `public, max-age=3600`                    | Cache Control header for 4XX responses                                                                                                                                           |
+| GUNICORN_WORKER_TMP_DIR       |                                           | This should be set to an tmpfs file system for better performance. See https://docs.gunicorn.org/en/stable/settings.html#worker-tmp-dir.                                         |
+| SHORT_ID_SIZE                 | `12`                                      | The size (number of characters) of the shortloink id's 
+| SHORT_ID_ALPHABET             | `0123456789abcdefghijklmnopqrstuvwxyz`    | The alphabet (characters) used by the shortlink. Allowed chars `[0-9][A-Z][a-z]-_`

app/__init__.py

@@ -152,4 +152,4 @@ def handle_exception(err):
     return make_error_msg(500, "Internal server error, please consult logs")
 
 
-from app import routes  # isort:skip pylint: disable=ungrouped-imports, wrong-import-position
+from app import routes  # isort:skip pylint: disable=ungrouped-imports, wrong-import-position, cyclic-import

app/helpers/utils.py

@@ -2,20 +2,21 @@
 import logging.config
 import os
 import re
-import time
-from distutils.util import strtobool
 from itertools import chain
 from pathlib import Path
 
 import validators
 import yaml
+from nanoid import generate
 
 from flask import abort
 from flask import jsonify
 from flask import make_response
 from flask import request
 
 from app.settings import ALLOWED_DOMAINS_PATTERN
+from app.settings import SHORT_ID_ALPHABET
+from app.settings import SHORT_ID_SIZE
 
 logger = logging.getLogger(__name__)
 
@@ -70,8 +71,7 @@ def get_redirect_param(ignore_errors=False):
 
 
 def generate_short_id():
-    # datetime.datetime(2001, 9, 9, 3, 46, 40) * 1000 = 1000000000000
-    return f'{int(time.time() * 1000) - 1000000000000:x}'
+    return generate(SHORT_ID_ALPHABET, SHORT_ID_SIZE)
 
 
 def make_error_msg(code, msg):
@@ -117,3 +117,17 @@ def get_url():
         abort(400, 'URL given as a parameter is not allowed.')
 
     return url
+
+
+def strtobool(value) -> bool:
+    """Convert a string representation of truth to true (1) or false (0).
+    True values are 'y', 'yes', 't', 'true', 'on', and '1'; false values
+    are 'n', 'no', 'f', 'false', 'off', and '0'.  Raises ValueError if
+    'val' is anything else.
+    """
+    value = value.lower()
+    if value in ('y', 'yes', 't', 'true', 'on', '1'):
+        return True
+    if value in ('n', 'no', 'f', 'false', 'off', '0'):
+        return False
+    raise ValueError(f"invalid truth value \'{value}\'")

app/routes.py

@@ -51,16 +51,16 @@ def create_shortlink():
 @app.route('/<shortlink_id>', methods=['GET'])
 def get_shortlink(shortlink_id):
     """
-    This route checks the shortened url id  and redirect the user to the full url.
+    This route checks the shortened url id and redirect the user to the full url.
     When the redirect query parameter is set to false, it will return a json containing
     the information about the shortlink.
     """
-    should_redirect = get_redirect_param()
+
     db_entry = get_db().get_entry_by_shortlink(shortlink_id)
     if db_entry is None:
         abort(404, f'No short url found for {shortlink_id}')
 
-    if should_redirect:
+    if get_redirect_param():
         logger.debug("redirecting to the following url : %s", db_entry['url'])
         return redirect(db_entry['url'], code=301)
 

app/settings.py

@@ -26,6 +26,8 @@
 STAGING = os.environ['STAGING']
 
 COLLISION_MAX_RETRY = 10
-SHORT_ID_SIZE = 10
+
+SHORT_ID_SIZE = int(os.getenv('SHORT_ID_SIZE', '12'))
+SHORT_ID_ALPHABET = os.getenv('SHORT_ID_ALPHABET', '0123456789abcdefghijklmnopqrstuvwxyz')
 
 GUNICORN_WORKER_TMP_DIR = os.getenv("GUNICORN_WORKER_TMP_DIR", None)

tests/unit_tests/__init__.py

@@ -1,6 +1,6 @@
 # The mocking is placed here to make sure it is started earliest possible.
 # see: https://github.com/spulec/moto#very-important----recommended-usage
-from moto import mock_dynamodb2
+from moto import mock_dynamodb
 
-dynamodb = mock_dynamodb2()
+dynamodb = mock_dynamodb()
 dynamodb.start()

tests/unit_tests/test_routes.py

@@ -6,6 +6,7 @@
 
 from flask import url_for
 
+from app.settings import SHORT_ID_ALPHABET
 from app.settings import SHORT_ID_SIZE
 from app.version import APP_VERSION
 from tests.unit_tests.base import BaseShortlinkTestCase
@@ -35,9 +36,16 @@ def test_create_shortlink_ok(self):
         shorturl = response.json.get('shorturl')
         self.assertEqual('http://localhost/' in shorturl, True)
         short_id = shorturl.replace('http://localhost/', '')
+        self.assertEqual(
+            len(short_id),
+            SHORT_ID_SIZE,
+            msg=f"Length of short_id '{short_id}' does not match configured size of "\
+                f"{SHORT_ID_SIZE} characters"
+        )
+        # Check if all characters of short_id are allowed characters as defined in SHORT_ID_ALPHABET
         self.assertIsNotNone(
-            re.search("^[0-9A-Za-z-_]{" + str(SHORT_ID_SIZE) + "}$", short_id),
-            msg=f'Short ID {short_id} doesn\'t match regex'
+            re.fullmatch(f"[{SHORT_ID_ALPHABET}]+", short_id),
+            f"Invalid characters found in short-id '{short_id}'. Allowed '{SHORT_ID_ALPHABET}'"
         )
         # Check that second call returns 200 and the same short url
         response = self.app.post(