mirror of https://github.com/halo-dev/halo
				
				
				
			Respond not found if no theme template found (#6511)
#### What type of PR is this? /kind improvement /area core /milestone 2.19.0 #### What this PR does / why we need it: This PR refactors "Template Not Found Exception" into "NotFoundException" to prevent too many exception stacktraces in logs file. #### Which issue(s) this PR fixes: Fixes #6501 #### Special notes for your reviewer: 1. Activate default theme 2. Request <http://localhost:8090/categories> 3. See the result #### Does this PR introduce a user-facing change? ```release-note 优化当主题模板找不到的异常提示 ```pull/6515/head
							parent
							
								
									3767a5a239
								
							
						
					
					
						commit
						50adc29e42
					
				| 
						 | 
				
			
			@ -103,6 +103,7 @@ tasks.named('build') {
 | 
			
		|||
 | 
			
		||||
tasks.named('test', Test) {
 | 
			
		||||
    useJUnitPlatform()
 | 
			
		||||
    maxHeapSize = '1G'
 | 
			
		||||
    finalizedBy jacocoTestReport
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -0,0 +1,62 @@
 | 
			
		|||
package run.halo.app.theme;
 | 
			
		||||
 | 
			
		||||
import static java.util.Comparator.comparing;
 | 
			
		||||
import static java.util.Comparator.naturalOrder;
 | 
			
		||||
import static java.util.Comparator.nullsLast;
 | 
			
		||||
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
import java.util.Objects;
 | 
			
		||||
import java.util.Optional;
 | 
			
		||||
import java.util.stream.Collectors;
 | 
			
		||||
import org.thymeleaf.IEngineConfiguration;
 | 
			
		||||
import org.thymeleaf.templateresolver.ITemplateResolver;
 | 
			
		||||
import org.thymeleaf.templateresolver.TemplateResolution;
 | 
			
		||||
import run.halo.app.infra.exception.NotFoundException;
 | 
			
		||||
 | 
			
		||||
/**
 | 
			
		||||
 * Composite template resolver to control execution flow of multiple template resolvers.
 | 
			
		||||
 *
 | 
			
		||||
 * @author johnniang
 | 
			
		||||
 */
 | 
			
		||||
class CompositeTemplateResolver implements ITemplateResolver {
 | 
			
		||||
 | 
			
		||||
    private final List<ITemplateResolver> resolvers;
 | 
			
		||||
 | 
			
		||||
    public CompositeTemplateResolver(Collection<ITemplateResolver> resolvers) {
 | 
			
		||||
        this.resolvers = Optional.ofNullable(resolvers).orElseGet(List::of)
 | 
			
		||||
            .stream()
 | 
			
		||||
            .distinct()
 | 
			
		||||
            // we keep the same order comparison as
 | 
			
		||||
            // org.thymeleaf.EngineConfiguration.TemplateResolverComparator
 | 
			
		||||
            .sorted(comparing(ITemplateResolver::getOrder, nullsLast(naturalOrder())))
 | 
			
		||||
            .toList();
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Override
 | 
			
		||||
    public String getName() {
 | 
			
		||||
        return resolvers.stream()
 | 
			
		||||
            .map(ITemplateResolver::getName)
 | 
			
		||||
            .collect(Collectors.joining(", "));
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Override
 | 
			
		||||
    public Integer getOrder() {
 | 
			
		||||
        // null order means to be the end of the resolvers.
 | 
			
		||||
        return null;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Override
 | 
			
		||||
    public TemplateResolution resolveTemplate(IEngineConfiguration configuration,
 | 
			
		||||
        String ownerTemplate, String template, Map<String, Object> templateResolutionAttributes) {
 | 
			
		||||
        return resolvers.stream()
 | 
			
		||||
            .map(resolver -> resolver.resolveTemplate(
 | 
			
		||||
                configuration, ownerTemplate, template, templateResolutionAttributes)
 | 
			
		||||
            )
 | 
			
		||||
            .filter(Objects::nonNull)
 | 
			
		||||
            .findFirst()
 | 
			
		||||
            .orElseThrow(() -> new NotFoundException("Template " + template + " was not found."));
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			@ -4,13 +4,18 @@ import java.util.HashMap;
 | 
			
		|||
import java.util.List;
 | 
			
		||||
import java.util.Locale;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
import java.util.Optional;
 | 
			
		||||
import org.attoparser.ParseException;
 | 
			
		||||
import org.springframework.beans.factory.annotation.Autowired;
 | 
			
		||||
import org.springframework.context.ApplicationContext;
 | 
			
		||||
import org.springframework.http.MediaType;
 | 
			
		||||
import org.springframework.lang.NonNull;
 | 
			
		||||
import org.springframework.stereotype.Component;
 | 
			
		||||
import org.springframework.web.ErrorResponse;
 | 
			
		||||
import org.springframework.web.reactive.result.view.View;
 | 
			
		||||
import org.springframework.web.server.ServerWebExchange;
 | 
			
		||||
import org.thymeleaf.exceptions.TemplateInputException;
 | 
			
		||||
import org.thymeleaf.exceptions.TemplateProcessingException;
 | 
			
		||||
import org.thymeleaf.spring6.view.reactive.ThymeleafReactiveView;
 | 
			
		||||
import org.thymeleaf.spring6.view.reactive.ThymeleafReactiveViewResolver;
 | 
			
		||||
import reactor.core.publisher.Flux;
 | 
			
		||||
| 
						 | 
				
			
			@ -54,7 +59,26 @@ public class HaloViewResolver extends ThymeleafReactiveViewResolver {
 | 
			
		|||
                // calculate the engine before rendering
 | 
			
		||||
                setTemplateEngine(engineManager.getTemplateEngine(theme));
 | 
			
		||||
                exchange.getAttributes().put(ModelConst.POWERED_BY_HALO_TEMPLATE_ENGINE, true);
 | 
			
		||||
                return super.render(model, contentType, exchange);
 | 
			
		||||
                return super.render(model, contentType, exchange)
 | 
			
		||||
                    .onErrorMap(TemplateProcessingException.class::isInstance, tee -> {
 | 
			
		||||
                        if (tee instanceof TemplateInputException) {
 | 
			
		||||
                            // map the error response exception while fragment not found
 | 
			
		||||
                            return Optional.of(tee)
 | 
			
		||||
                                .map(Throwable::getCause)
 | 
			
		||||
                                .filter(ParseException.class::isInstance)
 | 
			
		||||
                                .map(Throwable::getCause)
 | 
			
		||||
                                .filter(TemplateProcessingException.class::isInstance)
 | 
			
		||||
                                .map(Throwable::getCause)
 | 
			
		||||
                                .filter(ErrorResponse.class::isInstance)
 | 
			
		||||
                                .orElse(tee);
 | 
			
		||||
                        }
 | 
			
		||||
                        // map the error response exception while template not found
 | 
			
		||||
                        return Optional.of(tee)
 | 
			
		||||
                            .map(Throwable::getCause)
 | 
			
		||||
                            .filter(ErrorResponse.class::isInstance)
 | 
			
		||||
                            .orElse(tee);
 | 
			
		||||
                    });
 | 
			
		||||
 | 
			
		||||
            });
 | 
			
		||||
        }
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -137,6 +137,13 @@ public class TemplateEngineManager {
 | 
			
		|||
        engine.addDialect(new HaloProcessorDialect());
 | 
			
		||||
 | 
			
		||||
        templateResolvers.orderedStream().forEach(engine::addTemplateResolver);
 | 
			
		||||
 | 
			
		||||
        // we collect all template resolvers and add them into composite template resolver
 | 
			
		||||
        // to control the resolution flow
 | 
			
		||||
        var compositeTemplateResolver =
 | 
			
		||||
            new CompositeTemplateResolver(engine.getTemplateResolvers());
 | 
			
		||||
        engine.setTemplateResolver(compositeTemplateResolver);
 | 
			
		||||
 | 
			
		||||
        dialects.orderedStream().forEach(engine::addDialect);
 | 
			
		||||
 | 
			
		||||
        return engine;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -0,0 +1,73 @@
 | 
			
		|||
package run.halo.app.theme;
 | 
			
		||||
 | 
			
		||||
import static org.junit.jupiter.api.Assertions.assertEquals;
 | 
			
		||||
import static org.junit.jupiter.api.Assertions.assertNull;
 | 
			
		||||
import static org.junit.jupiter.api.Assertions.assertThrows;
 | 
			
		||||
import static org.mockito.Mockito.mock;
 | 
			
		||||
import static org.mockito.Mockito.when;
 | 
			
		||||
 | 
			
		||||
import java.util.List;
 | 
			
		||||
import org.junit.jupiter.api.Test;
 | 
			
		||||
import org.thymeleaf.templateresolver.ITemplateResolver;
 | 
			
		||||
import org.thymeleaf.templateresolver.TemplateResolution;
 | 
			
		||||
import run.halo.app.infra.exception.NotFoundException;
 | 
			
		||||
 | 
			
		||||
class CompositeTemplateResolverTest {
 | 
			
		||||
 | 
			
		||||
    @Test
 | 
			
		||||
    void shouldGetBlankNameIfNoResolvers() {
 | 
			
		||||
        var resolver = new CompositeTemplateResolver(null);
 | 
			
		||||
        assertEquals("", resolver.getName());
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Test
 | 
			
		||||
    void shouldGetNameIfResolversProvided() {
 | 
			
		||||
        var resolverA = mock(ITemplateResolver.class);
 | 
			
		||||
        when(resolverA.getName()).thenReturn("A");
 | 
			
		||||
        var resolverB = mock(ITemplateResolver.class);
 | 
			
		||||
        when(resolverB.getName()).thenReturn("B");
 | 
			
		||||
        var resolver = new CompositeTemplateResolver(List.of(resolverB, resolverA));
 | 
			
		||||
        assertEquals("B, A", resolver.getName());
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Test
 | 
			
		||||
    void shouldGetNullOrder() {
 | 
			
		||||
        var resolver = new CompositeTemplateResolver(null);
 | 
			
		||||
        assertNull(resolver.getOrder());
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Test
 | 
			
		||||
    void shouldThrowNotFoundExceptionIfNoResolvers() {
 | 
			
		||||
        var resolver = new CompositeTemplateResolver(null);
 | 
			
		||||
        assertThrows(
 | 
			
		||||
            NotFoundException.class,
 | 
			
		||||
            () -> resolver.resolveTemplate(null, null, null, null)
 | 
			
		||||
        );
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Test
 | 
			
		||||
    void shouldThrowNotFoundExceptionIfAllResolversReturnNull() {
 | 
			
		||||
        var resolverA = mock(ITemplateResolver.class);
 | 
			
		||||
        when(resolverA.resolveTemplate(null, null, null, null)).thenReturn(null);
 | 
			
		||||
        var resolverB = mock(ITemplateResolver.class);
 | 
			
		||||
        when(resolverB.resolveTemplate(null, null, null, null)).thenReturn(null);
 | 
			
		||||
        var resolver = new CompositeTemplateResolver(List.of(resolverA, resolverB));
 | 
			
		||||
        assertThrows(
 | 
			
		||||
            NotFoundException.class,
 | 
			
		||||
            () -> resolver.resolveTemplate(null, null, null, null)
 | 
			
		||||
        );
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Test
 | 
			
		||||
    void shouldResolveTemplateIfResolvedByOneOfResolvers() {
 | 
			
		||||
        var resolverA = mock(ITemplateResolver.class);
 | 
			
		||||
        var resolution = mock(TemplateResolution.class);
 | 
			
		||||
        when(resolverA.resolveTemplate(null, null, null, null))
 | 
			
		||||
            .thenReturn(resolution);
 | 
			
		||||
        var resolverB = mock(ITemplateResolver.class);
 | 
			
		||||
        when(resolverB.resolveTemplate(null, null, null, null))
 | 
			
		||||
            .thenReturn(null);
 | 
			
		||||
        var resolver = new CompositeTemplateResolver(List.of(resolverA, resolverB));
 | 
			
		||||
        assertEquals(resolution, resolver.resolveTemplate(null, null, null, null));
 | 
			
		||||
    }
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			@ -0,0 +1,93 @@
 | 
			
		|||
package run.halo.app.theme;
 | 
			
		||||
 | 
			
		||||
import static org.mockito.Mockito.when;
 | 
			
		||||
 | 
			
		||||
import java.util.LinkedHashSet;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
import org.hamcrest.Matchers;
 | 
			
		||||
import org.junit.jupiter.api.BeforeEach;
 | 
			
		||||
import org.junit.jupiter.api.Test;
 | 
			
		||||
import org.springframework.beans.factory.annotation.Autowired;
 | 
			
		||||
import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient;
 | 
			
		||||
import org.springframework.boot.test.context.SpringBootTest;
 | 
			
		||||
import org.springframework.boot.test.context.TestConfiguration;
 | 
			
		||||
import org.springframework.boot.test.mock.mockito.MockBean;
 | 
			
		||||
import org.springframework.context.annotation.Bean;
 | 
			
		||||
import org.springframework.context.annotation.Import;
 | 
			
		||||
import org.springframework.http.MediaType;
 | 
			
		||||
import org.springframework.test.annotation.DirtiesContext;
 | 
			
		||||
import org.springframework.test.web.reactive.server.WebTestClient;
 | 
			
		||||
import org.springframework.web.reactive.function.server.RouterFunction;
 | 
			
		||||
import org.springframework.web.reactive.function.server.RouterFunctions;
 | 
			
		||||
import org.springframework.web.reactive.function.server.ServerResponse;
 | 
			
		||||
import reactor.core.publisher.Mono;
 | 
			
		||||
import run.halo.app.core.extension.Menu;
 | 
			
		||||
import run.halo.app.core.extension.MenuItem;
 | 
			
		||||
import run.halo.app.extension.ExtensionClient;
 | 
			
		||||
import run.halo.app.extension.Metadata;
 | 
			
		||||
import run.halo.app.infra.InitializationStateGetter;
 | 
			
		||||
 | 
			
		||||
@SpringBootTest
 | 
			
		||||
@Import(ThemeIntegrationTest.TestConfig.class)
 | 
			
		||||
@AutoConfigureWebTestClient
 | 
			
		||||
@DirtiesContext
 | 
			
		||||
public class ThemeIntegrationTest {
 | 
			
		||||
 | 
			
		||||
    @Autowired
 | 
			
		||||
    WebTestClient webClient;
 | 
			
		||||
 | 
			
		||||
    @MockBean
 | 
			
		||||
    InitializationStateGetter initializationStateGetter;
 | 
			
		||||
 | 
			
		||||
    @Autowired
 | 
			
		||||
    ExtensionClient client;
 | 
			
		||||
 | 
			
		||||
    @BeforeEach
 | 
			
		||||
    void setUp() {
 | 
			
		||||
        when(initializationStateGetter.userInitialized()).thenReturn(Mono.just(true));
 | 
			
		||||
 | 
			
		||||
        // create a menu item
 | 
			
		||||
        var menuItem = new MenuItem();
 | 
			
		||||
        menuItem.setMetadata(new Metadata());
 | 
			
		||||
        menuItem.getMetadata().setName("main-menu-home");
 | 
			
		||||
        menuItem.setSpec(new MenuItem.MenuItemSpec());
 | 
			
		||||
        menuItem.getSpec().setDisplayName("Home");
 | 
			
		||||
        menuItem.getSpec().setHref("/");
 | 
			
		||||
        client.create(menuItem);
 | 
			
		||||
 | 
			
		||||
        // create a primary menu
 | 
			
		||||
        var menu = new Menu();
 | 
			
		||||
        menu.setMetadata(new Metadata());
 | 
			
		||||
        menu.getMetadata().setName("main-menu");
 | 
			
		||||
        menu.setSpec(new Menu.Spec());
 | 
			
		||||
        menu.getSpec().setDisplayName("Mail Menu");
 | 
			
		||||
        menu.getSpec().setMenuItems(new LinkedHashSet<>(List.of("main-menu-home")));
 | 
			
		||||
        client.create(menu);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @TestConfiguration
 | 
			
		||||
    static class TestConfig {
 | 
			
		||||
 | 
			
		||||
        @Bean
 | 
			
		||||
        RouterFunction<ServerResponse> noTemplateExistsRoute() {
 | 
			
		||||
            return RouterFunctions.route()
 | 
			
		||||
                .GET(
 | 
			
		||||
                    "/no-template-exists",
 | 
			
		||||
                    request -> ServerResponse.ok().render("no-template-exists")
 | 
			
		||||
                )
 | 
			
		||||
                .build();
 | 
			
		||||
        }
 | 
			
		||||
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    @Test
 | 
			
		||||
    void shouldRespondNotFoundIfNoTemplateFound() {
 | 
			
		||||
        webClient.get().uri("/no-template-exists")
 | 
			
		||||
            .accept(MediaType.TEXT_HTML)
 | 
			
		||||
            .exchange()
 | 
			
		||||
            .expectStatus().isNotFound()
 | 
			
		||||
            .expectBody(String.class)
 | 
			
		||||
            .value(Matchers.containsString("Template no-template-exists was not found"));
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
		Loading…
	
		Reference in New Issue