Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue research #1

Open
kevindeyne opened this issue Aug 24, 2023 · 3 comments
Open

Issue research #1

kevindeyne opened this issue Aug 24, 2023 · 3 comments

Comments

@kevindeyne
Copy link
Collaborator

kevindeyne commented Aug 24, 2023

When clicking the button, you see empty attributes in the JS. This is because, when adding fragments to the page, it's comparing two empty strings to each other as "lastRenderedHTML". There's no diffs, so its generating empty lists.

This does not occur when no fragments are present or when you're loading fragments locally, because hydraconnectioncontroller#askHydraForFragment has a shortcut that provides an instant response. In the scenario of an actual fragment being loaded from a different instance, there's no instance response but a slightly delayed one due to the wait for a hydra REST call to complete. The rest call itself is not the problem, but the additional delay explains the difference in behavior.

It also means that the cause of the error happens at render already, even though the render itself visually looks ok. Behind the scenes, the session is loaded with an empty 'lastRenderedHTML' meaning that any future changes are compared to an empty string, causing wrong attributes to be loaded.

So, specifically, the problem is here:
https://github.com/medusa-ui/medusa/blob/eeb1c2e9cb171dc7c2901bdd1ea1f87239d2bf5d/medusa-ui/src/main/java/io/getmedusa/medusa/core/router/action/SocketHandler.java#L81-L84

Essentially, the final Flux<DataBuffer> dataBufferFlux = renderer.render(route.getTemplateHTML(), s); only works with immediate responses. When the Flux can actually have a delay -- because for example, it's a REST call with network latency -- then the s.setLastRenderedHTML(newHtml); resolves before the Flux completes, causing the empty string.

One way to fix it would be something like:

final Flux<DataBuffer> dataBufferFlux = renderer.render(route.getTemplateHTML(), s);
dataBufferFlux.subscribe(buffer -> {
    final String oldHTML = s.getLastRenderedHTML();
    final String newHtml = FluxUtils.dataBufferToString(buffer);
    s.setLastRenderedHTML(newHtml);
    sessionMemoryRepository.store(s);

    //run diff engine old HTML vs new
    s.getSink().push(mergeDiffs(diffEngine.calculate(oldHTML, newHtml), passThroughAttributes));
    s.setDepth(0);
});

But it doesn't render right on action yet

@kevindeyne
Copy link
Collaborator Author

One way to get rid of the double rendering is:

    default Mono<ServerResponse> defaultRender(ServerRequest request,
                                               Route route,
                                               SecurityContext securityContext,
                                               Renderer renderer,
                                               SessionMemoryRepository sessionMemoryRepository) {
        Route.URIs.add(request.uri().getScheme() + "://" + request.uri().getAuthority());

        final Session session = new Session(route, request);
        return route.getSetupAttributes(request, session).flatMap(sAttributes -> {
            session.setLastParameters(sAttributes);
            Mono<DataBuffer> bufferMono = DataBufferUtils.join(renderer.render(route.getTemplateHTML(), session));
            return bufferMono.flatMap(r -> {
                session.setLastRenderedHTML(FluxUtils.dataBufferToString(r));
                sessionMemoryRepository.store(session);
                return ok().contentType(MediaType.TEXT_HTML).body(Flux.just(FluxUtils.stringToDataBuffer(session.getLastRenderedHTML())), DataBuffer.class);
            });
        });
    }

@kevindeyne
Copy link
Collaborator Author

And a better way, to remove the back and forth between buffer -> string -> buffer:

    default Mono<ServerResponse> defaultRender(ServerRequest request,
                                               Route route,
                                               SecurityContext securityContext,
                                               Renderer renderer,
                                               SessionMemoryRepository sessionMemoryRepository) {
        Route.URIs.add(request.uri().getScheme() + "://" + request.uri().getAuthority());

        final Session session = new Session(route, request);
        return route.getSetupAttributes(request, session).flatMap(sAttributes -> {
            session.setLastParameters(sAttributes);
            Mono<DataBuffer> bufferMono = DataBufferUtils.join(renderer.render(route.getTemplateHTML(), session));
            return bufferMono.flatMap(r -> {
                String renderedHTML = FluxUtils.dataBufferToString(r);
                session.setLastRenderedHTML(renderedHTML);
                sessionMemoryRepository.store(session);
                return ok().contentType(MediaType.TEXT_HTML).body(Mono.just(renderedHTML), String.class);
            });
        });
    }

@kevindeyne
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant