vlingo / xoom-schemata

The VLINGO XOOM Schema Registry.
https://vlingo.io
Mozilla Public License 2.0
17 stars 9 forks source link

[Docker] Make schemata app run within docker again #102

Closed wwerner closed 4 years ago

wwerner commented 4 years ago

Currently, the app hangs when serving resources, but only when run within docker. Running the jar works as expected.

Steps to reproduce

@kbastani wrote an alternative implementation for serving static resources (https://github.com/vlingo/vlingo-xoom/blob/master/vlingo-xoom-server/src/main/java/io/vlingo/xoom/resource/CachedStaticFilesResource.java) using caches instead of re-reading resources from the FS that seems not to suffer from this issue.

kbastani commented 4 years ago

This works on the most recent version of Docker for Mac for the latest commit hash on the master branch. There is however a hanging issue with the content length header that is a valid issue for http://localhost:8080/app or any 301 redirects being served for rewriting a URL. I think @wwerner fixed that, so we should push that fix to master.

One thing I did differently here was to make sure I installed the latest Vlingo platform core libraries using mvn clean install on vlingo-common vlingo-actors vlingo-wire and vlingo-http with the latest commit hash on those projects.

Let's make a mental note about static resource issues in the future with vlingo http. But for now let's close this issue. Thanks!

VaughnVernon commented 4 years ago

@kbastani I walked the code multiple times last week in search of how a Content-Length header is not being created. Literally when a Response is instantiated the body is checked for content and existing headers are checked for content length and added if missing. I don't see a code path where that doesn't happen. Can you pinpoint where this bypass occurs?

kbastani commented 4 years ago

Try schemata and remove the content length header from the UI resource code that performs a 301 redirect. This is definitely an open issue.

On Fri, Dec 6, 2019 at 5:40 PM Vaughn Vernon notifications@github.com wrote:

@kbastani https://github.com/kbastani I walked the code multiple times last week in search of how a Content-Length header is not being created. Literally when a Response is instantiated the body is checked for content and existing headers are checked for content length and added if missing. I don't see a code path where that doesn't happen. Can you pinpoint where this bypass occurs?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vlingo/vlingo-schemata/issues/102?email_source=notifications&email_token=AAP7VGVK7I4XOEMZQLDYY4TQXLIFRA5CNFSM4JW736D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGFSAKQ#issuecomment-562765866, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAP7VGQJVRNKBGRSIOVMG7DQXLIFRANCNFSM4JW736DQ .

VaughnVernon commented 4 years ago

@kbastani I see the Content-Length header here:

https://github.com/vlingo/vlingo-schemata/blob/c4f891a258628ffe7434c3e03e1f83a211e607e8/src/main/java/io/vlingo/schemata/resource/UiResource.java#L120

The 301 Moved Permanently does not contain content and currently doesn't set a Content-Length header:

https://github.com/vlingo/vlingo-schemata/blob/c4f891a258628ffe7434c3e03e1f83a211e607e8/src/main/java/io/vlingo/schemata/resource/UiResource.java#L104

Can you clarify which of these will be problematic when a Content-Length header is missing in the Response?

kbastani commented 4 years ago
private Completes<Response> redirectToApp(String path) {
    return Completes.withSuccess(
            Response.of(MovedPermanently, Header.Headers.of(
                    ResponseHeader.of("Location", path)), ""));
}

The above does not work.

private Completes<Response> redirectToApp(String path) {
    return Completes.withSuccess(
            Response.of(MovedPermanently, Header.Headers.of(
                    ResponseHeader.of("Location", path),
                    ResponseHeader.of(ContentLength, 0)), ""));
}

The above works.

VaughnVernon commented 4 years ago

@wwerner @kbastani I have written three tests. There are two in vlingo-http that test the general feature of a response sans Content-Length header, and one in vlingo-schemata that tests the specific case of MovedPermanently. All three tests pass.

UserResource

https://github.com/vlingo/vlingo-http/blob/bc1e39c63b4dc56485faf0bb435986d1ce5dbfc5/src/test/java/io/vlingo/http/sample/user/UserResource.java#L62

ServerTest#testThatServerRespondsPermanentRedirectWithNoContentLengthHeader ServerTest#testThatServerRespondsOkWithNoContentLengthHeader

https://github.com/vlingo/vlingo-http/blob/bc1e39c63b4dc56485faf0bb435986d1ce5dbfc5/src/test/java/io/vlingo/http/resource/ServerTest.java#L132

https://github.com/vlingo/vlingo-http/blob/bc1e39c63b4dc56485faf0bb435986d1ce5dbfc5/src/test/java/io/vlingo/http/resource/ServerTest.java#L150