Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(idempotency): include decorated fn name in hash #869

Merged
merged 3 commits into from
Dec 7, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion aws_lambda_powertools/utilities/idempotency/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def __init__(
self.fn_args = function_args
self.fn_kwargs = function_kwargs

persistence_store.configure(config)
persistence_store.configure(config, self.function.__name__)
self.persistence_store = persistence_store

def handle(self) -> Any:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class BasePersistenceLayer(ABC):

def __init__(self):
"""Initialize the defaults"""
self.function_name = None
self.configured = False
self.event_key_jmespath: Optional[str] = None
self.event_key_compiled_jmespath = None
Expand All @@ -124,7 +125,7 @@ def __init__(self):
self._cache: Optional[LRUDict] = None
self.hash_function = None

def configure(self, config: IdempotencyConfig) -> None:
def configure(self, config: IdempotencyConfig, function_name: str) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heitorlessa - should this be optional? or required?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional. Only because it's a public method and we have no way of knowing if someone took a dependency on it via tests or something - if they did, we will break them and take another release to fix it

"""
Initialize the base persistence layer from the configuration settings

Expand All @@ -133,6 +134,7 @@ def configure(self, config: IdempotencyConfig) -> None:
config: IdempotencyConfig
Idempotency configuration settings
"""
self.function_name = function_name
if self.configured:
# Prevent being reconfigured multiple times
return
Expand Down Expand Up @@ -178,7 +180,7 @@ def _get_hashed_idempotency_key(self, data: Dict[str, Any]) -> str:
warnings.warn(f"No value found for idempotency_key. jmespath: {self.event_key_jmespath}")

generated_hash = self._generate_hash(data=data)
function_name = os.getenv(constants.LAMBDA_FUNCTION_NAME_ENV, "test-func")
function_name = os.getenv(constants.LAMBDA_FUNCTION_NAME_ENV, "test-func") + "." + self.function_name
michaelbrewer marked this conversation as resolved.
Show resolved Hide resolved
return f"{function_name}#{generated_hash}"

@staticmethod
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/idempotency/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,15 @@ def expected_params_put_item_with_validation(hashed_idempotency_key, hashed_vali
def hashed_idempotency_key(lambda_apigw_event, default_jmespath, lambda_context):
compiled_jmespath = jmespath.compile(default_jmespath)
data = compiled_jmespath.search(lambda_apigw_event)
return "test-func#" + hashlib.md5(serialize(data).encode()).hexdigest()
return "test-func.lambda_handler#" + hashlib.md5(serialize(data).encode()).hexdigest()


@pytest.fixture
def hashed_idempotency_key_with_envelope(lambda_apigw_event):
event = extract_data_from_envelope(
data=lambda_apigw_event, envelope=envelopes.API_GATEWAY_HTTP, jmespath_options={}
)
return "test-func#" + hashlib.md5(serialize(event).encode()).hexdigest()
return "test-func.lambda_handler#" + hashlib.md5(serialize(event).encode()).hexdigest()


@pytest.fixture
Expand Down
72 changes: 56 additions & 16 deletions tests/functional/idempotency/test_idempotency.py
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ def test_in_progress_never_saved_to_cache(
):
# GIVEN a data record with status "INPROGRESS"
# and persistence_store has use_local_cache = True
persistence_store.configure(idempotency_config)
persistence_store.configure(idempotency_config, "handler")
data_record = DataRecord("key", status="INPROGRESS")

# WHEN saving to local cache
Expand All @@ -661,7 +661,7 @@ def test_in_progress_never_saved_to_cache(
@pytest.mark.parametrize("idempotency_config", [{"use_local_cache": False}], indirect=True)
def test_user_local_disabled(idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer):
# GIVEN a persistence_store with use_local_cache = False
persistence_store.configure(idempotency_config)
persistence_store.configure(idempotency_config, "")

# WHEN calling any local cache options
data_record = DataRecord("key", status="COMPLETED")
Expand All @@ -683,7 +683,7 @@ def test_delete_from_cache_when_empty(
idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer
):
# GIVEN use_local_cache is True AND the local cache is empty
persistence_store.configure(idempotency_config)
persistence_store.configure(idempotency_config, "handler")

try:
# WHEN we _delete_from_cache
Expand Down Expand Up @@ -735,15 +735,16 @@ def test_default_no_raise_on_missing_idempotency_key(
idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer, lambda_context
):
# GIVEN a persistence_store with use_local_cache = False and event_key_jmespath = "body"
persistence_store.configure(idempotency_config)
function_name = "foo"
persistence_store.configure(idempotency_config, function_name)
assert persistence_store.use_local_cache is False
assert "body" in persistence_store.event_key_jmespath

# WHEN getting the hashed idempotency key for an event with no `body` key
hashed_key = persistence_store._get_hashed_idempotency_key({})

# THEN return the hash of None
expected_value = "test-func#" + md5(serialize(None).encode()).hexdigest()
expected_value = f"test-func.{function_name}#" + md5(serialize(None).encode()).hexdigest()
assert expected_value == hashed_key


Expand All @@ -754,7 +755,7 @@ def test_raise_on_no_idempotency_key(
idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer, lambda_context
):
# GIVEN a persistence_store with raise_on_no_idempotency_key and no idempotency key in the request
persistence_store.configure(idempotency_config)
persistence_store.configure(idempotency_config, "handler")
persistence_store.raise_on_no_idempotency_key = True
assert persistence_store.use_local_cache is False
assert "body" in persistence_store.event_key_jmespath
Expand All @@ -781,7 +782,7 @@ def test_jmespath_with_powertools_json(
idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer, lambda_context
):
# GIVEN an event_key_jmespath with powertools_json custom function
persistence_store.configure(idempotency_config)
persistence_store.configure(idempotency_config, "handler")
sub_attr_value = "cognito_user"
static_pk_value = "some_key"
expected_value = [sub_attr_value, static_pk_value]
Expand All @@ -794,7 +795,7 @@ def test_jmespath_with_powertools_json(
result = persistence_store._get_hashed_idempotency_key(api_gateway_proxy_event)

# THEN the hashed idempotency key should match the extracted values generated hash
assert result == "test-func#" + persistence_store._generate_hash(expected_value)
assert result == "test-func.handler#" + persistence_store._generate_hash(expected_value)


@pytest.mark.parametrize("config_with_jmespath_options", ["powertools_json(data).payload"], indirect=True)
Expand All @@ -803,7 +804,7 @@ def test_custom_jmespath_function_overrides_builtin_functions(
):
# GIVEN an persistence store with a custom jmespath_options
# AND use a builtin powertools custom function
persistence_store.configure(config_with_jmespath_options)
persistence_store.configure(config_with_jmespath_options, "handler")

with pytest.raises(jmespath.exceptions.UnknownFunctionError, match="Unknown function: powertools_json()"):
# WHEN calling _get_hashed_idempotency_key
Expand Down Expand Up @@ -871,7 +872,9 @@ def _delete_record(self, data_record: DataRecord) -> None:
def test_idempotent_lambda_event_source(lambda_context):
# Scenario to validate that we can use the event_source decorator before or after the idempotent decorator
mock_event = load_event("apiGatewayProxyV2Event.json")
persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest())
persistence_layer = MockPersistenceLayer(
"test-func.lambda_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()
)
expected_result = {"message": "Foo"}

# GIVEN an event_source decorator
Expand All @@ -891,7 +894,9 @@ def lambda_handler(event, _):
def test_idempotent_function():
# Scenario to validate we can use idempotent_function with any function
mock_event = {"data": "value"}
persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest())
persistence_layer = MockPersistenceLayer(
"test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()
)
expected_result = {"message": "Foo"}

@idempotent_function(persistence_store=persistence_layer, data_keyword_argument="record")
Expand All @@ -908,7 +913,9 @@ def test_idempotent_function_arbitrary_args_kwargs():
# Scenario to validate we can use idempotent_function with a function
# with an arbitrary number of args and kwargs
mock_event = {"data": "value"}
persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest())
persistence_layer = MockPersistenceLayer(
"test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()
)
expected_result = {"message": "Foo"}

@idempotent_function(persistence_store=persistence_layer, data_keyword_argument="record")
Expand All @@ -923,7 +930,9 @@ def record_handler(arg_one, arg_two, record, is_record):

def test_idempotent_function_invalid_data_kwarg():
mock_event = {"data": "value"}
persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest())
persistence_layer = MockPersistenceLayer(
"test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()
)
expected_result = {"message": "Foo"}
keyword_argument = "payload"

Expand All @@ -940,7 +949,9 @@ def record_handler(record):

def test_idempotent_function_arg_instead_of_kwarg():
mock_event = {"data": "value"}
persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest())
persistence_layer = MockPersistenceLayer(
"test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()
)
expected_result = {"message": "Foo"}
keyword_argument = "record"

Expand All @@ -958,13 +969,19 @@ def record_handler(record):
def test_idempotent_function_and_lambda_handler(lambda_context):
# Scenario to validate we can use both idempotent_function and idempotent decorators
mock_event = {"data": "value"}
persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest())
persistence_layer = MockPersistenceLayer(
"test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()
)
expected_result = {"message": "Foo"}

@idempotent_function(persistence_store=persistence_layer, data_keyword_argument="record")
def record_handler(record):
return expected_result

persistence_layer = MockPersistenceLayer(
"test-func.lambda_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()
)

@idempotent(persistence_store=persistence_layer)
def lambda_handler(event, _):
return expected_result
Expand All @@ -986,7 +1003,9 @@ def test_idempotent_data_sorting():
data_two = {"more_data": "more data 1", "data": "test message 1"}

# Assertion will happen in MockPersistenceLayer
persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(json.dumps(data_one).encode()).hexdigest())
persistence_layer = MockPersistenceLayer(
"test-func.dummy#" + hashlib.md5(json.dumps(data_one).encode()).hexdigest()
)

# GIVEN
@idempotent_function(data_keyword_argument="payload", persistence_store=persistence_layer)
Expand Down Expand Up @@ -1017,3 +1036,24 @@ def dummy_handler(event, context):
dummy_handler(mock_event, lambda_context)

assert len(persistence_store.table.method_calls) == 0


@pytest.mark.parametrize("idempotency_config", [{"use_local_cache": True}], indirect=True)
def test_idempotent_function_duplicates(
idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer
):
# Scenario to validate the both methods are called
mock_event = {"data": "value"}
persistence_store.table = MagicMock()

@idempotent_function(data_keyword_argument="data", persistence_store=persistence_store, config=idempotency_config)
def one(data):
return "one"

@idempotent_function(data_keyword_argument="data", persistence_store=persistence_store, config=idempotency_config)
def two(data):
return "two"

assert one(data=mock_event) == "one"
assert two(data=mock_event) == "two"
assert len(persistence_store.table.method_calls) == 4