elct9620 / hanami-lambda

Hanami Lambda is a gem that provides a way to run hanami application on AWS Lambda.
MIT License
4 stars 3 forks source link

#4, refactor: add Rack env builder #6

Open requiemformemories opened 3 months ago

requiemformemories commented 3 months ago

Description

The Rack env build method is too large that hard to extend new behaviors. Add Hanami::Lambda::Env to improve it.

Related Issues

elct9620 commented 3 months ago

@requiemformemories please rebase main branch and sign your commit

requiemformemories commented 3 months ago

Thanks for the review. I made up for the RBS types and I also used another way to transform the headers.

requiemformemories commented 1 month ago

Currently I am facing some difficulties with type checking.

sig/hanami/lambda/env.rbs:14:55: [error] Type `::Hash` is generic but used as a non generic type
│ Diagnostic ID: RBS::InvalidTypeApplication
│
└       def initialize: (event: Hash[::String, String? | Hash], headers: Hash[::String, ::String], context: Object) -> void
                                                         ~~~~

(1) @event is quite complicated. For example, we might have event['requestContext']['http']['method'] and if we want to list all types, we have to list all ::Hash inside the ::Hash

lib/hanami/lambda/env.rb:25:32: [error] Cannot pass a value of type `(::String | nil)` as an argument of type `(::String | ::Hash | ::StringIO | ::Hash[::String, (::String | ::Hash)] | ::Object)`
│   (::String | nil) <: (::String | ::Hash | ::StringIO | ::Hash[::String, (::String | ::Hash)] | ::Object)
│     nil <: (::String | ::Hash | ::StringIO | ::Hash[::String, (::String | ::Hash)] | ::Object)
│       nil <: ::String
│
│ Diagnostic ID: Ruby::ArgumentTypeMismatch
│
└           env["CONTENT_TYPE"] = @content_type if @content_type

(2) In the source code we have env["CONTENT_TYPE"] = @content_type if @content_type and the if clause could make sure that only string could be store in env["CONTENT_TYPE"]. I have to define the value of @event could be nil or I could not pass the validation.

lib/hanami/lambda/env.rb:21:45: [error] Cannot pass a value of type `(::String | ::Hash)` as an argument of type `::String`
│   (::String | ::Hash) <: ::String
│     ::Hash <: ::String
│       ::Object <: ::String
│         ::BasicObject <: ::String
│
│ Diagnostic ID: Ruby::ArgumentTypeMismatch
│
└           ::Rack::RACK_INPUT => StringIO.new(@event["body"] || ""),
                                               ~~~~~~~~~~~~~~~~~~~~

(3) StringIO.new(@event["body"] || "") would also violate the type checking because Steep do not know the detail of @event.

elct9620 commented 1 month ago

(1) @event is quite complicated.

RBS support alias means we can write something like

# Nested "event" key-value
type event = Hash[String, event]

def initialize: (event: event, ...) -> ...

(2) env["CONTENT_TYPE"] = @content_type if @content_type

I think you need to think about why @content_type will be nil?

The nil usually is not the correct state

(3) StringIO.new(@event["body"] || "")

If we give initialize(event: ...) and then assign it to @event = event the RBS should know it's type. If not, it may be a bug.