wimdeblauwe / htmx-spring-boot

Spring Boot and Thymeleaf helpers for working with htmx
Apache License 2.0
521 stars 49 forks source link

Issue #42: Moving HxTrigger and HxRefresh handling to preHandle to be… #43

Closed credmond closed 1 year ago

credmond commented 1 year ago

… compatible with @ResponseBody

Due to the nature of how ResponseBody is processed, a request is committed (therefore silently immutable) before an interceptor's postHandle is ever called. Therefore, these headers were being lost in this ResponseBody situation (and maybe in other unknown situations). The fix is to set the headers instead in preHandle(), as it makes no difference to existing functionality but fixes this issue.

checketts commented 1 year ago

@credmond Were you able to create a test the reproduced the bug?

credmond commented 1 year ago

@credmond Were you able to create a test the reproduced the bug?

Yes! However... it requires a @SpringBootApplication: i.e., it requires a real web server (Tomcat Coyote code, etc., to reproduce).

The below test passes with the suggested change, but fails without (proving bug and proving fix).

I wanted to add a permanent test as part of the PR, but, it would stick out as "different" to all the others. So, I think the whole testing approach should change for other functionality too if we add something like the below. Given the nature of the library, I feel like it could be an idea to move from basic unit testing to more integration-style tests such as the below to prove the functionality.

I'm happy to attempt an overhaul if you want to go that direction; or you guys can assess the pros/cons and do whatever you find best.

package io.github.wimdeblauwe.hsbt.mvc;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.client.TestRestTemplate;
import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.http.ResponseEntity;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@EnableAutoConfiguration(exclude = {SecurityAutoConfiguration.class}) // Security is on by default
class HtmxHandlerInterceptorIT {

    @Autowired
    protected TestRestTemplate restTemplate;

    @LocalServerPort
    protected int port;

    @Test
    public void testHeaderIsSetOnResponseIfHxTriggerIsPresent() {
        final ResponseEntity<String> response
                = restTemplate.getForEntity("http://localhost:" + port + "/with-trigger", String.class);

        // Passes if preHandle is used, fails if postHandle is used (HtmxHandlerInterceptor)
        assertThat(response.getHeaders().get("HX-Trigger")).isNotNull();
        assertThat(response.getHeaders().get("HX-Trigger")).asList().containsOnly("eventTriggered");
    }
}