7
\$\begingroup\$

I'm learning how to use DynamoDB and various AWS services, so I decided to write a simple hit counter application for my education of how to use DynamoDB. This application deduplicates hits from the same client for the same page by storing a hash of their user agent and ip address. It also stores 1 entry of hits per day, and accumulates them over the time domain. I'm looking for feedback on how I structured the dynamo calls and potential issues that it has. I suspect that I might be using a consistent read when I don't need to. I'm also curious if I used the right pattern to handle an upsert in dynamodb.

The dynamo db table has a hash key of the "url" and a sort field called "as_of_when" which says roughly when the hits occured.

"""
A simple hit counter stuff to learn the various apis of dynamodb.
"""

import datetime
import os
import hashlib
import base64

import boto3
from botocore.exceptions import ClientError
from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.core import patch_all
from pybadges import badge

patch_all()

TABLE_NAME = os.getenv("TABLE_NAME", "hit_counts")

dynamo = boto3.client('dynamodb')


def get_today() -> str:
    """
    Gets a formatted date string
    """
    return datetime.datetime.now().strftime("%Y-%m-%d")

def get_full_timestamp() -> str:
    """
    Gets a complete date string
    """
    return datetime.datetime.now().strftime("%Y%m%d%H%M%S")

def get_previous_total(url:str):
    """
    Gets the most recent known total. Should be used before inserting a new instance
    """
    response = dynamo.query(
        TableName=TABLE_NAME,
        Select='ALL_ATTRIBUTES',
        Limit=1,
        KeyConditionExpression="the_url=:urlval",
        ScanIndexForward=False,
        ExpressionAttributeValues={
            ":urlval": {
                "S": url
            }
        }
    )
    if response['Count'] == 0:
        return 0
    return response['Items'][0]['accumulated_count']['N']

@xray_recorder.capture('insert_new_entry')
def insert_new_entry(url:str, as_of_when : str, user_hash : str, user_id : str):
    the_count = str(int(get_previous_total(url)) + 1)

    result = dynamo.put_item(
        TableName=TABLE_NAME,
        Item={
            'the_url': {
                'S': url
            },
            'as_of_when': {
                'S': as_of_when
            },
            'today_count': {
                'N': '1'
            },
            'accumulated_count': {
                'N': the_count
            },
            'user_id_hashes': {
                'SS': [user_hash]
            },
            'user_hashes': {
                'SS': [user_id]
            }
        },
        ReturnValues='ALL_OLD',
        ReturnItemCollectionMetrics='SIZE',
        ReturnConsumedCapacity='TOTAL',
        ConditionExpression='attribute_not_exists(the_url) and attribute_not_exists(as_of_when)'
    )
    print('insert_result', result)

    
    return result

@xray_recorder.capture('update_existing_entry')
def update_existing_entry(url:str, as_of_when : str, user_hash : str, user_id : str):
    result = dynamo.execute_statement(
        Statement=f"""
            UPDATE {TABLE_NAME}
            SET today_count = today_count + 1
            SET accumulated_count = accumulated_count + 1
            SET user_hashes = set_add(user_hashes, ?)
            SET user_id_hashes = set_add(user_id_hashes, ?)
            WHERE the_url = ? AND as_of_when = ? RETURNING ALL NEW *
        """,
        Parameters=[
            {
                "SS": [user_id]
            },
            {
                "SS": [user_hash]
            },
            {
                "S": url
            },
            {
                "S": as_of_when
            }
        ]
    )
    return result

@xray_recorder.capture('get_todays_entry')
def get_todays_entry(url:str, as_of_when : str):
    result = dynamo.get_item(
        TableName=TABLE_NAME,
        Key={
            'the_url': {
                'S': url
            },
            'as_of_when': {
                'S': as_of_when
            }
        },
        AttributesToGet=[
            'today_count',
            'accumulated_count',
            'user_hashes',
            'user_id_hashes'
        ],
        ConsistentRead=True
    )

    print('get_todays_entry', result)
    if 'Item' in result:
        return result['Item']
    return None


def increment_hit_count(url:str, as_of_when : str, user_hash : str, user_id : str):
    """
    increments a counter instance in the dynamo table
    """
    current_hits = get_todays_entry(url, as_of_when)

    if current_hits is None:
        # Insert new entry
        x = insert_new_entry(url, as_of_when, user_hash, user_id)
        current_hits = {
            'accumulated_count': {
                'N': "1"
            }
        }
    else:
        # Check for existence in existing object
        print(current_hits['user_id_hashes'])
        print(user_id)
        if user_hash not in current_hits['user_id_hashes']['SS']:
            result = update_existing_entry(url, as_of_when, user_hash, user_id)
            if 'Items' in result:
                current_hits = result['Items'][0]
            print(result)

    
    return  current_hits['accumulated_count']['N']




def hash_api_gateway_event(event : dict):
    reqContext = event['requestContext']['http']
    reqString = ':'.join((reqContext['sourceIp'], reqContext['protocol'] + reqContext['userAgent']))
    
    m = hashlib.md5()
    m.update(reqString.encode('utf-8'))

    return base64.b64encode(m.digest()).decode('utf-8'), reqString


def handler(event : dict, context):
    """
    # Invoked via query string parameters from an image tag
    # Returns a SVG unless text parameter is set.
    """
    print(event)
    print("hello")

    user_hash, og_user_id = hash_api_gateway_event(event)
    print(user_hash, og_user_id)

    if 'queryStringParameters' in event:
        url = event['queryStringParameters']['url']
        result = increment_hit_count(url, get_today(), user_hash, og_user_id)

        print(result)


    body =  badge(left_text="Total Views", right_text=result)

    return {
        "statusCode": 200,
        "isBase64Encoded": False,
        "headers": {
            "Content-Type": "image/svg+xml"
        },
        "body": body
    }
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Alex Waygood covered everything I would have said, so I'll only add that getting into the habit of running pylint, pycodestyle, and mypy really helps get into the groove of things with python, they really did the trick for me \$\endgroup\$ Sep 2, 2021 at 11:55

2 Answers 2

7
\$\begingroup\$

[Edited following discussion in the comments]

A few brief notes on your code:

  1. Your type hints could be more complete/descriptive. You don't have any return annotations; the context parameter in your handler function has no type hint; and annotating your event parameter in handler and hash_api_gateway_event with a bare dict tells the type-checker extremely little. It would be much better to annotate these parameters with a parameterised dict that informs the type-checker about the expected types of the dictionary keys and values. You could also consider typing.TypedDict, which allows you to specify the type of the value you expect to be associated with individual keys in a dictionary.
  2. It would be nice to have docstrings in all your public functions, even if it's just a brief one-liner describing what your function achieves.
  3. "Flat is better than nested", so consider using from datetime import datetime and from base64 import bs4encode instead of import datetime and import base64. In both cases, you only use a single class/function from their respective module; it makes your code more succinct and readable without introducing any ambiguity about what the functions/classes do; and it's marginally more performant to not have to look up the datetime class in the datetime module whenever you want to use it.

I know little about DynamoDB, so can't comment on what your code is trying to accomplish, but with that (large) caveat, and other than the points I've mentioned above, your code appears pretty clean from my perspective.

\$\endgroup\$
0
5
\$\begingroup\$

For your date format,

datetime.datetime.now().strftime("%Y-%m-%d")

is more directly expressed as

datetime.date.today().isoformat()

For this expression:

return response['Items'][0]['accumulated_count']['N']

if you know there's only one item in the response, then

item, = response['Items']
return item['accumulated_count']['N']

will be more explicit, and offer for free an assertion that exactly one item was returned.

This method:

def insert_new_entry(url:str, as_of_when : str, user_hash : str, user_id : str):

has inconsistent spacing around your type hints; it should look like url: str. Also, you're missing a return typehint, which in this case is the same as the type from dynamo.put_item.

print('insert_result', result) and similar should be converted to a logging call.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.