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: + * + *
+ * {@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
+ * 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/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