Fork me on GitHub
#pedestal
<
2017-10-10
>
zallin14:10:53

hey @mtnygard , regarding advice you gave me about http method specific interceptors

zallin14:10:41

in terse syntax I get assertion error

Caused by: java.lang.AssertionError: Assert failed: This vector for the verb map has extra elements. The leftover elements are (my-ped.service/home-page) from the original data [my-ped.service/identity-ic my-ped.service/home-page]

zallin14:10:52

while it works fine in table syntax

mtnygard14:10:19

Terse syntax usually trips me up somewhere or other.

mtnygard14:10:38

I think it will work if you do this:

mtnygard14:10:10

{:get ^interceptors [other-interceptor] [:unique-route-name handler]}

mtnygard14:10:25

But it might have to be like this:

mtnygard14:10:41

{:get ^{:interceptors [other-interceptor]} [:unique-route-name handler]}

zallin14:10:27

and as for using multimethods as handlers Paul has closed my ticket saying that this behavior is not supported by design :robot_face:

zallin14:10:33

as far as i remember i tried first version and it was syntax error

mtnygard14:10:13

I don’t agree with Paul on that one. I’ve added a comment to the issue explaining my reasoning.

zallin14:10:39

second works thx !

mtnygard14:10:54

Glad it worked.

zallin14:10:54

but I do not really understand the difference ..

mtnygard14:10:33

I probably should have use ^:interceptors in the first one instead of ^interceptors.

mtnygard14:10:55

But that’s the old, somewhat out-of-style way of writing metadata anyway

zallin14:10:30

it seems that

^:key
just sets key to true in meta

zallin14:10:18

I think this case we ve worked out should be reflected in doc , because now it says "The value of each key is either a handler function or a list of interceptors."

mtnygard14:10:06

What happens if you try a vector like [:unique-route-name interceptor interceptor handler]?

mtnygard14:10:20

Maybe the problem was just missing the route name at the beginning…

zallin14:10:59

let me try

zallin15:10:49

no it is the same assertion error

mtnygard15:10:51

Thanks for trying those variations out. I added a docs issue: https://github.com/pedestal/pedestal-docs/issues/91

zallin15:10:11

ok thank you

zallin15:10:48

@mtnygard you know what? even using

(def routes
  `[[["/" {:get ^{:interceptors [identity-ic]} [:my-route home-page]}
      ^:interceptors [(body-params/body-params) http/html-body]
    ["/about" {:get about-page}]]]])
identity-ic is not added into interceptors vector, checked it via expand-routes.

zallin15:10:12

:face_with_rolling_eyes:

mtnygard15:10:34

OK. It’s time for me to consult the source instead of working from memory.

mtnygard15:10:53

Here’s the syntax straight from the tests:

mtnygard15:10:21

{:get [:terminal-intercepted ^:interceptors [interceptor-1 interceptor-2] request-inspection]}

mtnygard15:10:06

So it’s a boolean metadata flag for :interceptors attached to an ordinary vector, nested inside the verb’s action vector

mtnygard15:10:18

See, this is why I made the table syntax!

mtnygard15:10:51

I can never remember how to put the terse syntax together.

zallin15:10:16

I will try to use table syntax for a while, it is clearly more obvious but also quite verbose

mtnygard15:10:46

This sounds sarcastic when written in text, but I mean it sincerely. We’re always interested in better ways to use data to describe configuration like routes. If you are able to find a representation that reduces duplication but doesn’t create the confusing overloaded syntax of the terse format, please let me know. We can plug new routing parsers in pretty easily.

mpenet15:10:32

still working on the use of the router standalone: I am stuck on a weird thing that seems somehow intentional, it seems that a route defined for /foo/bar/ will only match /foo/bar (notice the missing trailing slash)?

mpenet15:10:02

it uses this regex internally apparently #"/\Qfoo\E/\Qbar\E"

mtnygard15:10:19

I wouldn’t call it intentional, necessarily, but it is for real.

mtnygard15:10:40

/foo/bar is a different resource than /foo/bar/baz

mpenet15:10:51

it's odd to say the least

mtnygard15:10:57

Oddly, specs differ in their opinion about /foo/bar/

mpenet15:10:23

the route is super clear/simple

mtnygard15:10:24

Web servers historically treat that as an alias for some subordinate like /foo/bar/index.html

mpenet15:10:18

well our proxy rewrite reqs when they are missing the trailing /, that makes it tricky to use

mtnygard15:10:22

But for an API, it’s not clear whether “/foo/bar/” should mean “/foo/bar” or “/foo/bar/some-default”

mpenet15:10:47

in a pure routing context, if the route is defined for /foo/bar/ it should match /foo/bar/

mpenet15:10:53

http or else

mtnygard15:10:50

Hmm… I found this issue that claims trailing slashes should be supported now. https://github.com/pedestal/pedestal/issues/477

mpenet15:10:46

maybe there's some sanitizing done at a lower level in the http bits, but the router does not support it

mtnygard15:10:10

@mpenet Which router are you using?

mpenet15:10:16

linear for now

mpenet15:10:31

but they all compile to the same regex I believe

mtnygard15:10:15

I’m looking at the source now to figure out why Paul’s comment doesn’t match the behavior you’re seeing.

mtnygard15:10:56

OK, it looks like that issue doesn’t really apply. It talks about routes defined with a trailing slash, but requests that don’t have it.

mpenet15:10:31

the trailing / or not imho should be a user level concern, you design your routes in one way and let either a proxy or middleware deal with it

mtnygard15:10:55

I understand what you’re saying.