add toStrictEntity and extractStrictEntity directive

[Hawstein]

Ref #20881

[akka-ci]

Can one of the repo owners verify this patch?

[johanandren]

OK TO TEST

[johanandren]

Please add a warning that this will read the entire request entity into memory regardless of size and also a doc entry about the timeout parameter.

[johanandren]

Needs a Java DSL counterpart as well and should also have an entries in the respective basic-directives docs, thanks!

[ktoso]

I'd prefer this directive to trigger toStrict however not extract it - we have directives that extract the entity, please see my comment here: #20881 (comment)

[akka-ci]

Test FAILed.

[Hawstein]

@ktoso Your proposal make sense and I have tried to implement what you prefer, however, just like @Joe-Edwards have replied you in #20881(comment) , it will lost the strict info and I have to do some other stuffs to get the strict entity. Do you have some good idea to deal with that?

[ktoso]

Like I said, we can have both – but the more composable toStrictEntity one is a must, the extractStrictEntity is a nice to have. Both should be documented with a big fat warning that they effectively disable streaming

[Hawstein]

Hmm, since the strict info will be lost in the RequestContext even after the toStrictEntity is applied, it seems that it makes no big differences and may confuse the users. However, I have no strong options on that. If you still consider it a must, I will work on to implement both:)

[ktoso]

Hmm, since the strict info will be lost in the RequestContext even after the toStrictEntity is applied,

No it will not be "lost", only the type will, which is the smallest difference the toStrict call makes. It changes semantics and behaviour of the server / routes. One might not want to access the entity at all, just change the semantics.

[Hawstein]

It sounds making sense. working on it now.

[Hawstein]

@ktoso @johanandren PR is updated.

[akka-ci]

Test FAILed.

[akka-ci]

Test PASSed.

[ktoso]

A bit more precise maybe:

Converts the HttpEntity from the [[akka.http.javadsl.server.RequestContext]] into an 
[[akka.http.javadsl.model.HttpEntity.Strict]], or fails the route if unable to drain the 
entire request body within the timeout.

[ktoso]

Looks good, only needs to get java part of docs now.

[Hawstein]

@ktoso PR is updated.

[akka-ci]

Test PASSed.

[Hawstein]

Ping

[johanandren]

Not regardless of size, there is the default size limit of 8M that may be overriden by wrapping with the size limit directives, right?

[Hawstein]

Yes, limiting the size should be another directive's responsibility.

[johanandren]

No, the http server puts a stage in the entity stream that will limit it by default. Unless you explicitly opt out of it with the withoutSizeLimit directive.

[johanandren]

So I'm pretty sure this is doc statement not true (and the other places where it is stated)

[Hawstein]

I see. I misunderstood your first comment. so how about changing the doc to:

The directive will read the request entity into memory within the size limit(8M by default) and effectively disable streaming

[johanandren]

Sounds good, maybe mention it can be configured globally with akka.http.parsing.max-content-length and add links to the withSizeLimit and withoutSizeLimit directives for route local overriding also?

[johanandren]

LGTM after some minor fixes

[Hawstein]

@johanandren PR is updated according to your comments.

[akka-ci]

Test PASSed.

[ktoso]

Awesome work @Hawstein !
LGTM :-)