feat(appsync): Support AppSyncResolverEvent subclassing

[whardier]

…vent

No outsstanding issue

Description of changes:

Allows for custom subclass of AppSyncResolverEvent data_class to be offered to AppSyncResolver.resolve

Checklist

• Meet tenets criteria
• Update tests
• Update docs
• PR title follows conventional commit semantics

Breaking change checklist

Not breaking

RFC issue #:

• Migration process documented
• Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[boring-cyborg]

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.

[whardier]

Alternatively the annotation 'current_event: AppSyncResolverEvent' could simply be overridden and then utilized via self.annotations'current_event' .. but I'm not sure if annotations is optimized out or how recognized that pattern is.

[heitorlessa]

Hey @whardier, thanks for the contribution!!

Could you share a sample subclass that you created for AppSync event?

This will help me understand the use case, and what's potentially missing from the AppSyncEvent class before we add tests for this PR.

Thank you

[whardier]

Sure.. the following is a non-patched solution to provide the custom properties we are accessing from the underlaying _data:

from aws_lambda_powertools.utilities.data_classes.appsync_resolver_event import (
    AppSyncIdentityCognito,
    AppSyncResolverEvent,
)

class CustomAppSyncResolverEvent(AppSyncResolverEvent):
    @cached_property
    def auth_client_id(self) -> Optional[str]:
        auth_client_id: Optional[str] = None
        if isinstance(self.identity, AppSyncIdentityCognito):
            auth_client_id = self.identity.claims.get("custom:client_id")
        return auth_client_id

    @cached_property
    def auth_user_profile_id(self) -> Optional[str]:
        auth_user_profile_id: Optional[str] = None
        if isinstance(self.identity, AppSyncIdentityCognito):
            auth_user_profile_id = self.identity.claims.get("custom:user_profile_id") or self.identity.sub
        return auth_user_profile_id

# ... below to assist with hints

class CustomAppSyncResolver(AppSyncResolver):
    current_event: CustomAppSyncResolverEvent
    
# ... specific enough sample appsync lambda event handler

@appsync.resolver(type_name="Mutation", field_name="logThingUserEvent")
@validate_arguments
@typechecked
def log_thing_user_event(
    event_type: ThingUserEventTypeEnum,
    measure_reference: Annotated[str, Field(max_length=256)],
    measure_source: Annotated[str, Field(max_length=32)] = DEFAULT_THING_USER_EVENT_APPSYNC_METRIC_SOURCE,
):

    # ... sample code
    
    auth_client_id: Optional[str] = appsync.current_event.auth_client_id
    auth_user_profile_id: Optional[str] = appsync.current_event.auth_user_profile_id
    
    # ... other junk

@core_tracer.capture_lambda_handler
@core_logger.inject_lambda_context(correlation_id_path=correlation_paths.APPSYNC_RESOLVER)
@event_source(data_class=CustomAppSyncResolverEvent)
@typechecked
def appsync_handler(event: CustomAppSyncResolverEvent, context: AnyLambdaContext) -> dict:

    # NOTE: Until aws lambda powertools allows for hooks/class references to be part of the resolve method to set up a
    # proper current_event with a subclassed dataclass and repeat the innards of the resolve method here (SRS).

    appsync.current_event = event
    appsync.lambda_context = context
    resolver = appsync._get_resolver(event.type_name, event.field_name)
    return resolver(**event.arguments)
    

The proposed patch mimics a bit of the model_class approach seen elsewhere in this project. However a sneaky pattern to do this is as follows and seems to be gaining in popularity:

class LibraryDataClass:
    pass


class CustomDataClass(LibraryDataClass):
    @property
    def custom_property(self):
        return "custom property data"


class LibraryExecutionClass:
    data: LibraryDataClass

    def execute(self):
        data_class = self.__annotations__["data"]
        self.data = data_class()


class CustomExecutionClass(LibraryExecutionClass):
    data: CustomDataClass


# >>> custom_executor = CustomExecutionClass()
# >>> custom_executor.execute()
# >>> custom_executor.data.custom_property
# 'custom property data'

[whardier]

I concerned that using model as a kwarg here would lead to thoughts that this should be a parser.model rather than a dictwrap data_class. I know I would have been confused seeing model.

[heitorlessa]

I concerned that using model as a kwarg here would lead to thoughts that this should be a parser.model rather than a dictwrap data_class. I know I would have been confused seeing model.

That's a great point. I find naming hard because even data_class could lead to discussions like #481.

How does data_model sound? I've made all changes necessary - docs, tests, mypy, agreeing on that and we can merge and release ;)

[codecov-commenter]

Codecov Report

Merging #526 (c884cfa) into develop (749a372) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop     #526   +/-   ##
========================================
  Coverage    99.86%   99.86%           
========================================
  Files          113      113           
  Lines         4482     4483    +1     
  Branches       243      243           
========================================
+ Hits          4476     4477    +1     
  Misses           3        3           
  Partials         3        3           

Impacted Files	Coverage Δ
aws_lambda_powertools/event_handler/appsync.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 749a372...c884cfa. Read the comment docs.

[nmoutschen]

I concerned that using model as a kwarg here would lead to thoughts that this should be a parser.model rather than a dictwrap data_class. I know I would have been confused seeing model.

That's a great point. I find naming hard because even data_class could lead to discussions like #481.

How does data_model sound? I've made all changes necessary - docs, tests, mypy, agreeing on that and we can merge and release ;)

Sharing my 2 cents here, I'd personally prefer data_model as model is quite overloaded in different Python frameworks and libraries.

[heitorlessa]

Waiting for @whardier before we merge it

[whardier]

I'm still down with something as simple as data_class (as seen here: @event_source(data_class=CustomAppSyncResolverEvent) ) but I'm not as behind the scenes on this project ;) - I can't find any other examples on where passing the data_class class name is done any other way. I will have to catch up on #481. I really dig the dict wrapper and feel it is appropriately named and unfortunately overlapping other useful naming. For continuity sake I would prefer to see data_class used until a breaking change can be made to ultimately address any concerns surrounding it.

My preferences aside. I approve this and thank you for catching the call case and handling the typing the proper way. It was nice being able to submit some thoughts this way and see changes come back as code.

[heitorlessa]

We'll keep it as data_model as the amount of energy spent on #481 was abnormal on naming things - I'm merging and release it this week ;)

Thanks a lot again @whardier

[boring-cyborg]

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!