diff --git a/build.gradle.kts b/build.gradle.kts index 6a846c98..14da2b74 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -148,6 +148,7 @@ dependencies { implementation(libs.spring.boot.starter.web) implementation(libs.spring.boot.starter.actuator) implementation(libs.spring.boot.starter.aop) + implementation(libs.spring.boot.starter.validation) implementation(libs.spring.ai.starter.mcp.server.webmvc) implementation(libs.solr.solrj) implementation(libs.commons.csv) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 473ff9ff..99c8725a 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -47,6 +47,7 @@ spring-boot-starter-web = { module = "org.springframework.boot:spring-boot-start spring-boot-starter-actuator = { module = "org.springframework.boot:spring-boot-starter-actuator" } spring-boot-starter-aop = { module = "org.springframework.boot:spring-boot-starter-aop" } spring-boot-starter-security = { module = "org.springframework.boot:spring-boot-starter-security" } +spring-boot-starter-validation = { module = "org.springframework.boot:spring-boot-starter-validation" } spring-boot-starter-oauth2-resource-server = { module = "org.springframework.boot:spring-boot-starter-oauth2-resource-server" } spring-boot-docker-compose = { module = "org.springframework.boot:spring-boot-docker-compose" } spring-boot-starter-test = { module = "org.springframework.boot:spring-boot-starter-test" } diff --git a/src/main/java/org/apache/solr/mcp/server/config/SolrConfigurationProperties.java b/src/main/java/org/apache/solr/mcp/server/config/SolrConfigurationProperties.java index a0e15357..f7879553 100644 --- a/src/main/java/org/apache/solr/mcp/server/config/SolrConfigurationProperties.java +++ b/src/main/java/org/apache/solr/mcp/server/config/SolrConfigurationProperties.java @@ -16,8 +16,10 @@ */ package org.apache.solr.mcp.server.config; +import jakarta.validation.constraints.NotBlank; import org.jspecify.annotations.Nullable; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.validation.annotation.Validated; /** * Spring Boot Configuration Properties record for Apache Solr connection @@ -131,6 +133,8 @@ * @see org.springframework.boot.context.properties.ConfigurationProperties * @see org.springframework.boot.context.properties.EnableConfigurationProperties */ +@Validated @ConfigurationProperties(prefix = "solr") -public record SolrConfigurationProperties(String url, @Nullable String username, @Nullable String password) { +public record SolrConfigurationProperties(@NotBlank @SolrUrl String url, @Nullable String username, + @Nullable String password) { } diff --git a/src/main/java/org/apache/solr/mcp/server/config/SolrUrl.java b/src/main/java/org/apache/solr/mcp/server/config/SolrUrl.java new file mode 100644 index 00000000..660e2f21 --- /dev/null +++ b/src/main/java/org/apache/solr/mcp/server/config/SolrUrl.java @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.config; + +import jakarta.validation.Constraint; +import jakarta.validation.Payload; +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Bean Validation constraint asserting that a value is a URL that SolrJ can + * actually connect to. + * + *

+ * A value satisfies this constraint when it is an absolute URL whose + * scheme is {@code http} or {@code https} and which carries a non-empty host. + * That is precisely the set of URLs + * {@link org.apache.solr.client.solrj.impl.HttpJdkSolrClient} can talk to, so + * anything rejected here would have failed later anyway — at first request + * rather than at startup, and with a far less actionable message. + * + *

+ * Why this is needed alongside {@code @NotBlank}: + * + *

+ * {@code @NotBlank} rejects only {@code null}, {@code ""} and all-whitespace + * values. It happily accepts {@code solr.url=localhost:8983} — a common + * misconfiguration, since omitting the scheme looks harmless. + * {@link java.net.URI} parses that string as scheme {@code localhost} + * with a {@code null} host, and {@link SolrConfig} normalizes it by pure string + * concatenation into {@code localhost:8983/solr/} without ever noticing. + * + *

+ * Accepted: + * + *

+ * + *

+ * Rejected: + * + *

+ * + * @see SolrUrlValidator + * @see SolrConfigurationProperties + */ +@Documented +@Constraint(validatedBy = SolrUrlValidator.class) +@Target({ElementType.METHOD, ElementType.FIELD, ElementType.ANNOTATION_TYPE, ElementType.CONSTRUCTOR, + ElementType.PARAMETER, ElementType.TYPE_USE}) +@Retention(RetentionPolicy.RUNTIME) +public @interface SolrUrl { + + /** + * {@return the validation message rendered when the value is not a usable Solr + * URL} + */ + String message() default "must be an absolute http or https URL including a host, for example http://localhost:8983/solr/"; + + /** {@return the validation groups this constraint belongs to} */ + Class[] groups() default {}; + + /** {@return the payload associated with this constraint} */ + Class[] payload() default {}; +} diff --git a/src/main/java/org/apache/solr/mcp/server/config/SolrUrlValidator.java b/src/main/java/org/apache/solr/mcp/server/config/SolrUrlValidator.java new file mode 100644 index 00000000..fae8c88f --- /dev/null +++ b/src/main/java/org/apache/solr/mcp/server/config/SolrUrlValidator.java @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.config; + +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; +import org.jspecify.annotations.Nullable; +import org.springframework.util.StringUtils; +import org.springframework.web.util.UriComponents; +import org.springframework.web.util.UriComponentsBuilder; + +/** + * Validates {@link SolrUrl} by parsing the candidate value with Spring's + * {@link UriComponentsBuilder} and inspecting the resulting scheme and host. + * + *

+ * {@code UriComponentsBuilder} is used rather than {@link java.net.URL} because + * it parses without performing any network or protocol-handler lookup, and it + * exposes {@link UriComponents#getScheme()} and {@link UriComponents#getHost()} + * as independent components — which is exactly the distinction that separates + * {@code http://localhost:8983} from {@code localhost:8983}. + * + * @see SolrUrl + */ +public class SolrUrlValidator implements ConstraintValidator { + + private static final String HTTP_SCHEME = "http"; + private static final String HTTPS_SCHEME = "https"; + + /** Default constructor used by the Bean Validation provider. */ + public SolrUrlValidator() { + } + + /** + * Determines whether {@code value} is a URL SolrJ can connect to. + * + *

+ * Null and blank values are reported as valid so that emptiness stays the + * concern of {@code @NotBlank}. Without this, an unset {@code solr.url} would + * surface two overlapping messages instead of one. + * + * @param value + * the configured URL, or {@code null} when the property is unset + * @param context + * the constraint validator context + * @return {@code true} if {@code value} satisfies the constraint + */ + @Override + public boolean isValid(@Nullable String value, ConstraintValidatorContext context) { + // Emptiness is @NotBlank's responsibility; reporting it here too would + // yield two messages for a single mistake. + if (!StringUtils.hasText(value)) { + return true; + } + + UriComponents uri; + try { + uri = UriComponentsBuilder.fromUriString(value).build(); + } catch (IllegalArgumentException ex) { + // Not parseable as a URI at all (for example "not a url"). + return false; + } + + // A missing scheme means the value is relative ("/solr"); a scheme that + // is neither http nor https is one SolrJ's HTTP client cannot speak. + String scheme = uri.getScheme(); + if (!HTTP_SCHEME.equals(scheme) && !HTTPS_SCHEME.equals(scheme)) { + return false; + } + + // "localhost:8983" parses as scheme "localhost" with no host and is + // already excluded above, but "http://" reaches here with a null host. + return StringUtils.hasText(uri.getHost()); + } +} diff --git a/src/main/resources/logback.xml b/src/main/resources/logback.xml deleted file mode 100644 index 9463f026..00000000 --- a/src/main/resources/logback.xml +++ /dev/null @@ -1,32 +0,0 @@ - - - - - - diff --git a/src/test/java/org/apache/solr/mcp/server/config/SolrUrlValidatorTest.java b/src/test/java/org/apache/solr/mcp/server/config/SolrUrlValidatorTest.java new file mode 100644 index 00000000..28fa3c07 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/config/SolrUrlValidatorTest.java @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.config; + +import static org.assertj.core.api.Assertions.assertThat; + +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; +import java.util.Set; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledInNativeImage; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; + +/** + * Verifies the {@link SolrUrl} constraint both in isolation and through Spring + * Boot's configuration-property binding, which is the path that actually + * decides whether a misconfigured deployment fails at startup. + */ +class SolrUrlValidatorTest { + + private static ValidatorFactory validatorFactory; + private static Validator validator; + + @BeforeAll + static void openValidatorFactory() { + validatorFactory = Validation.buildDefaultValidatorFactory(); + validator = validatorFactory.getValidator(); + } + + @AfterAll + static void closeValidatorFactory() { + validatorFactory.close(); + } + + @ParameterizedTest + @ValueSource( + strings = {"http://localhost:8983", "http://localhost:8983/", "http://localhost:8983/solr", + "http://localhost:8983/solr/", "https://solr.internal:8983/custom/solr/", + "https://solr.example.com"}) + void acceptsAbsoluteHttpUrlsWithAHost(String url) { + assertThat(violations(url)).isEmpty(); + } + + @ParameterizedTest + @ValueSource( + strings = {"localhost:8983", "solr.example.com", "/solr", "ftp://solr.example.com/solr", "file:///var/solr", + "not a url", "http://"}) + void rejectsUrlsSolrJCannotConnectTo(String url) { + assertThat(violations(url)).extracting(ConstraintViolation::getMessage).containsExactly( + "must be an absolute http or https URL including a host, " + "for example http://localhost:8983/solr/"); + } + + /** + * A blank URL must report only {@code @NotBlank}'s message. {@link SolrUrl} + * deliberately passes blank values through so a single mistake does not produce + * two overlapping messages. + */ + @ParameterizedTest + @ValueSource(strings = {"", " "}) + void reportsBlankUrlOnlyThroughNotBlank(String url) { + assertThat(violations(url)).extracting(ConstraintViolation::getMessage).containsExactly("must not be blank"); + } + + /** + * Disabled in native image because {@link ApplicationContextRunner} builds its + * {@code AssertableApplicationContext} with {@link java.lang.reflect.Proxy}, + * and GraalVM cannot materialize a JDK dynamic proxy that was not registered at + * build time. This is a limitation of the test harness, not of the constraint — + * the JVM build covers this path, and the constraint itself is exercised + * natively by the parameterized tests above. + */ + @Test + @DisabledInNativeImage + void applicationContextFailsToStartWhenSolrUrlOmitsTheScheme() { + contextRunner().withPropertyValues("solr.url=localhost:8983").run(context -> assertThat(context).hasFailed()); + } + + /** @see #applicationContextFailsToStartWhenSolrUrlOmitsTheScheme() */ + @Test + @DisabledInNativeImage + void applicationContextStartsWhenSolrUrlIsAbsolute() { + contextRunner().withPropertyValues("solr.url=http://localhost:8983/solr/") + .run(context -> assertThat(context).hasNotFailed()); + } + + private static Set> violations(String url) { + return validator.validate(new SolrConfigurationProperties(url, null, null)); + } + + private static ApplicationContextRunner contextRunner() { + return new ApplicationContextRunner().withUserConfiguration(SolrPropertiesConfiguration.class); + } + + @EnableConfigurationProperties(SolrConfigurationProperties.class) + static class SolrPropertiesConfiguration { + } +}