2011/3/3 Gregory Collins <[email protected]>
> Hi Román,
>
> A couple of suggestions about this interface:
>
> 1) there is currently no nice way of setting the request body using a
> ByteString. Users who want to test a handler which accepts PUT
> requests will want this.
>
Ok.
>
> 2) I think for test functions we can make all of the map types
> [(ByteString, ByteString)] instead of [(ByteString, [ByteString])] --
> repeated headers are an obscure feature of the HTTP specification
>
I did it like that initially and changed just to keep it up with the current
Snap API.
I agree with you, so cool.
>
> 3) buildBoundary wasn't exactly what I had in mind, the length of the
> boundary is not really supposed to vary with the size of the input;
> when I brought up base16 encoding it was in the context of generating
> a fixed-size randomly-generated string. You'd be just as well off
> setting the boundary to a fixed string like
> "___snap_test_boundary___".
>
I just wanted to do a random boundary, to make it more real. But yeah, that
should be enough, should I do the same with sub-boundaries in file entries
in the multipart?
something like "____snap_test_subboundary___"?
>
> 4) looking at the functions exported (get, put, post, get', put',
> post', etc) I see a lot of duplication, or rather a lot of functions
> which do multiple unrelated things together; this is a sign we have
> the model slightly wrong. (For instance, "get" both builds the request
> and runs the test handler.)
>
> I like the idea of building up the Request to test using a monad; I
> think we might be better off if we changed to:
>
> newtype RequestBuilder a = RequestBuilder (State Request a)
> deriving (Monad, MonadState Request)
>
> We could kill a lot of functions if we did this. For instance, instead
> of get/put/post/get'/etc returning the Response, you could have a
> single function:
>
> runHandler :: Snap () -> RequestBuilder () -> IO Response
>
> and you'd have to expand the set of operations in the RequestBuilder monad:
>
> setURI :: ByteString -> RequestBuilder ()
> setMethod :: Method -> RequestBuilder ()
> addHeader :: ByteString -> ByteString -> RequestBuilder ()
> postUrlEncoded :: Params -> RequestBuilder ()
> postMultipart :: Params -> FileParams -> RequestBuilder ()
> setRequestBody :: ByteString -> RequestBuilder ()
>
> You could still expose convenience functions for common cases, but
> they would work in RequestBuilder:
>
> get :: ByteString -> [(ByteString, ByteString)] -> RequestBuilder ()
>
> so that the use case becomes:
>
> resp <- runHandler foo $ do
> get "/foo" [("bar", "baz")]
> addHeader "X-Forwarded-For" "127.0.0.1"
>
> Let me know what you think.
>
4.a) I like the idea of having just one function to get (IO Response), and
makes more sense to makes everything inside the monad, that way you can have
really specialized request buildings done by developers depending on their
needs.
4.b) RequestBuilder was initially a State monad of the
Snap.Http.Internal.Request type, however I changed it to make it easier to
build the body of the request from the parameters.
Having the Request directly makes it really difficult to create the body of
the request because you are receiving parameters one by one instead of the
whole thing.
Probably the encoding of the body request and adding it to the request can
be done in:
- postUrlEncoded (should be default to post I guess)
- postMultipart
And the QueryString encoded in the URI can be added in the modifier:
- get
I will be working on that, I can't promise to have it soonish given that I
had a lot of things to do regarding the Haskell Meetup (next Tues), once
that's done I'll be starting with this. You are not in a hurry right?
Thanks for the feedback.
Roman.-
>
> G
>
> 2011/3/3 Román González <[email protected]>:
> > Guys,
> > I think I'm more or less finished on the first version of the test Snap
> API,
> > we are supporting requests for each HTTP method, requests with file
> uploads,
> > some assertion functions. More assert functions and modifiers should be
> > added when needed. There are also some basic request modifiers, again,
> add
> > more when needed as well (the lazy way) :-)
> > Here is the Snap.Internal.Test file that has both public and private
> > functions:
> >
> https://github.com/roman/snap-core/blob/testing/src/Snap/Internal/Test.hs
> > The Snap.Test that will have the functions we want to export:
> > https://github.com/roman/snap-core/blob/testing/src/Snap/Test.hs
> > And the test done to this API on the Snap test folder:
> >
> https://github.com/roman/snap-core/blob/testing/test/suite/Snap/Test/Tests.hs
> > If you (all the Snap team) have any suggestions/questions regarding
> > documentation, code style or pending functions that you think are really
> > necessary, please let me know.
> > Regards.
> > Roman.-
> >
> > 2011/2/27 Gregory Collins <[email protected]>
> >>
> >> 2011/2/26 Román González <[email protected]>:
> >> > Hey Greg,
> >> > Thanks for the feedback, just one more question (bellow) about reading
> >> > the
> >> > files the tester is providing...
> >>
> >> >> > - Reading the contents of the files given by the developer on the
> >> >> > Test
> >> >> > API
> >> >>
> >> >> I'm not sure I follow you here: what specifically is the problem?
> >> >
> >> > I think I'm wondering where are we going to read the files, would we
> be
> >> > requiring the tester to provide a full URI for the File location?
> >> > get' "/my-upload" [] $ do
> >> > addFile "photo" "/tmp/file.jpg"
> >> > Or are we going to have like a SNAP_ROOT where all the paths will go
> >> > get' "/my-upload" [] $ do
> >> > addFile "photo" "tmp/file.jpg" -- this go to
> SNAP_ROOT/tmp/file.jpg
> >>
> >> I would say, for the testing interface, just supply them as bytestrings.
> >>
> >> G
> >> --
> >> Gregory Collins <[email protected]>
> >
> >
>
>
>
> --
> Gregory Collins <[email protected]>
>
_______________________________________________
Snap mailing list
[email protected]
http://mailman-mail5.webfaction.com/listinfo/snap
