diff --git a/changelog/unreleased/solr-18347-admin-ui-defects.yml b/changelog/unreleased/solr-18347-admin-ui-defects.yml new file mode 100644 index 000000000000..80f18ea21e8a --- /dev/null +++ b/changelog/unreleased/solr-18347-admin-ui-defects.yml @@ -0,0 +1,9 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Fix five Admin UI defects found by Selenium testing +type: fixed # added, changed, fixed, deprecated, removed, dependency_update, security, other +authors: + - name: Shrey Narayan + nick: NextbrickInc +links: + - name: SOLR-18347 + url: https://issues.apache.org/jira/browse/SOLR-18347 diff --git a/dev-docs/admin-ui-tests.md b/dev-docs/admin-ui-tests.md index 73ad2a9f65b7..3a5d070ea5d0 100644 --- a/dev-docs/admin-ui-tests.md +++ b/dev-docs/admin-ui-tests.md @@ -55,17 +55,6 @@ deliberately does not do. - Every test cluster in the JVM registers a log-watcher appender under the same name in the shared log4j config, so a later cluster's watcher can be blind; the events-viewer test detects this via the API and skips itself. -- The shared menu code logs a benign - `TypeError: Cannot read properties of null (reading 'name')` from - `$scope.showCore` while the per-collection menu resolves (filtered in the - console-error assertion; tracked in - [SOLR-18347](https://issues.apache.org/jira/browse/SOLR-18347)). -- The core overview ping widget answers 503 when the configset has no - healthcheck file (allowed in the affected tests; tracked in - [SOLR-18347](https://issues.apache.org/jira/browse/SOLR-18347)). -- The Schema Designer's backend transiently fails its own prep/analyze calls - with "version mismatch, retry" and recovers via its retry dialog; its API - errors are excluded from the console-error assertion. - ASF Jenkins jobs do not pass `-Ptests.selenium=true`, so these tests do not run there (a nightly job could opt in if its build nodes have a browser). In CI they run via the GitHub Actions workflow diff --git a/solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java index f7eaa97db78b..ba7ebd2d6889 100644 --- a/solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java @@ -213,10 +213,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw break; case STATUS: if (healthcheck == null) { - SolrException e = - new SolrException( - SolrException.ErrorCode.SERVICE_UNAVAILABLE, "healthcheck not configured"); - rsp.setException(e); + rsp.add("status", "not_configured"); } else { rsp.add("status", isPingDisabled() ? "disabled" : "enabled"); } diff --git a/solr/core/src/test/org/apache/solr/handler/PingRequestHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/PingRequestHandlerTest.java index 3f151e51efca..4180be913579 100644 --- a/solr/core/src/test/org/apache/solr/handler/PingRequestHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/PingRequestHandlerTest.java @@ -85,6 +85,10 @@ public void testPingWithNoHealthCheck() throws Exception { rsp = makeRequest(handler, req("action", "ping")); assertEquals("OK", rsp.getValues().get("status")); + + rsp = makeRequest(handler, req("action", "status")); + assertEquals("not_configured", rsp.getValues().get("status")); + assertNull(rsp.getException()); } public void testEnablingServer() throws Exception { diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionScreensTest.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionScreensTest.java index f758d46e11b4..b4e82fb54578 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionScreensTest.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionScreensTest.java @@ -101,7 +101,7 @@ public void testCoreOverviewShowsStats() { openPage(coreName + "/core-overview", By.id("dashboard")); waitForPageContains("Num Docs"); waitForPageContains(Integer.toString(NUM_DOCS)); - // the ping widget answers 503 when the configset has no healthcheck file - assertNoSevereConsoleErrors("/admin/ping"); + waitForPageContains("Ping request handler is not configured with a healthcheck file."); + assertNoSevereConsoleErrors(); } } diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionsScreenTest.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionsScreenTest.java index 8dad9b0be8a1..e8e44ad6ebd7 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionsScreenTest.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCollectionsScreenTest.java @@ -142,8 +142,8 @@ public void testAddAndDeleteReplicaViaUi() throws Exception { @Test public void testReloadCollectionViaUi() throws Exception { - // reloading resets the core's start time; that proves the action end-to-end, - // unlike the UI success indicator which only flashes for a second + // Reloading resets the core's start time, and the success state remains visible long enough + // for a user to notice instead of disappearing after one second. String coreName = coreNameOnNode0(COLLECTION); Object startTimeBefore = coreStartTime(coreName); @@ -152,6 +152,12 @@ public void testReloadCollectionViaUi() throws Exception { waitUntil( "core start time should change after reload", () -> !startTimeBefore.equals(coreStartTime(coreName))); + WebElement reload = waitFor(By.cssSelector("#reload.success")); + assertEquals("Reloaded", reload.getText()); + Thread.sleep(1500); + assertTrue( + "reload success should remain visible", reload.getAttribute("class").contains("success")); + assertEquals("Reloaded", reload.getText()); assertNoSevereConsoleErrors(); } diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCoreAdminStandaloneTest.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCoreAdminStandaloneTest.java index 0508fd636e79..ea801c7d8243 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCoreAdminStandaloneTest.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiCoreAdminStandaloneTest.java @@ -63,8 +63,8 @@ public void testStandaloneMenus() { openPage("swapa/core-overview", By.id("dashboard")); waitFor(By.cssSelector("#core-menu .query")); waitFor(By.cssSelector("#core-menu .replication")); - // the ping widget answers 503 when the configset has no healthcheck file - assertNoSevereConsoleErrors("/admin/ping"); + waitForPageContains("Ping request handler is not configured with a healthcheck file."); + assertNoSevereConsoleErrors(); } @Test diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSchemaDesignerTest.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSchemaDesignerTest.java index 85a76aec800c..670a9df12b37 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSchemaDesignerTest.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSchemaDesignerTest.java @@ -16,7 +16,6 @@ */ package org.apache.solr.webapp; -import org.apache.lucene.tests.util.LuceneTestCase; import org.junit.Test; import org.openqa.selenium.By; import org.openqa.selenium.WebElement; @@ -25,11 +24,9 @@ * Happy-path test of the Schema Designer screen: create a new schema, paste a sample document and * let the designer analyze it. * - *

AwaitsFix: the designer backend transiently fails its own prep/analyze calls ("version - * mismatch, retry", "Error loading solr config") when driven at automation speed, making this test - * flaky even with retries. + *

The Analyze action remains disabled until creation of the mutable schema has completed, so a + * fast user cannot race the prep and analyze requests. */ -@LuceneTestCase.AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-18347") public class AdminUiSchemaDesignerTest extends AdminUiTestBase { @Test @@ -47,34 +44,9 @@ public void testDesignSchemaFromSampleDocument() throws Exception { WebElement sampleDocs = waitFor(By.cssSelector("#sample-docs textarea#document")); sampleDocs.clear(); sampleDocs.sendKeys("[{\"id\":\"1\",\"designer_title\":\"Hello Designer\"}]"); - click(By.id("analyze")); + click(By.cssSelector("#analyze:not([disabled])")); - // the analyzed schema lists the field derived from the sample doc. The designer - // backend transiently fails its own calls ("version mismatch, retry", "Error - // loading solr config") and surfaces an error dialog - dismiss it and analyze - // again, with a generous budget since each round trips several requests - long deadlineNanos = System.nanoTime() + WAIT_TIMEOUT.multipliedBy(3).toNanos(); - boolean analyzed = false; - while (!analyzed && System.nanoTime() < deadlineNanos) { - analyzed = driver.getPageSource().contains("designer_title"); - if (!analyzed) { - for (String dismissButton : new String[] {"Reload Schema", "OK"}) { - driver.findElements(By.xpath("//button[contains(., '" + dismissButton + "')]")).stream() - .filter(WebElement::isDisplayed) - .findFirst() - .ifPresent(WebElement::click); - } - driver.findElements(By.id("analyze")).stream() - .filter(WebElement::isDisplayed) - .findFirst() - .ifPresent(WebElement::click); - Thread.sleep(500); - } - } - assertTrue("Analyzed schema should list the sample doc field", analyzed); - // the designer's own API calls (prep/analyze/luke against its temp core) error - // transiently while it persists and reloads the schema - it recovers via its retry - // dialog, so only unrelated console errors fail the test - assertNoSevereConsoleErrors("schema-designer/", "._designer_"); + waitForPageContains("designer_title"); + assertNoSevereConsoleErrors(); } } diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSmokeTest.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSmokeTest.java index 5dfc5b2a4517..a20059160181 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSmokeTest.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiSmokeTest.java @@ -91,9 +91,7 @@ public void testCoreScreens() { coreName + "/plugins", By.id("plugins"), coreName + "/segments", By.id("segments")); screens.forEach(this::smoke); - // the ping widget on the overview answers 503 when no healthcheck file is configured, - // as is the case for the _default configset - smoke(coreName + "/core-overview", By.id("dashboard"), "/admin/ping"); + smoke(coreName + "/core-overview", By.id("dashboard")); } private void smoke(String route, By anchor) { diff --git a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java index 5d0ea91fe9f4..ac471487420c 100644 --- a/solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java +++ b/solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java @@ -537,12 +537,6 @@ protected static void assertNoSevereConsoleErrors(String... allowedSubstrings) { // the ui-grid icon font referenced from ui-grid.min.css is not shipped with // the webapp at all, so it 404s in production too .filter(entry -> !entry.getMessage().contains("fonts/ui-grid")) - // benign race in the shared menu code: showCore() fires with a null core - // while the per-collection menu resolves after navigation - .filter( - entry -> - !(entry.getMessage().contains("reading 'name'") - && entry.getMessage().contains("showCore"))) .toList(); assertTrue("Severe browser console errors: " + severe, severe.isEmpty()); } diff --git a/solr/webapp/web/js/angular/app.js b/solr/webapp/web/js/angular/app.js index 27a2f458682f..1bbaa4b9163e 100644 --- a/solr/webapp/web/js/angular/app.js +++ b/solr/webapp/web/js/angular/app.js @@ -653,7 +653,9 @@ solrAdminApp.controller('MainController', function($scope, $route, $rootScope, $ } $scope.showCore = function(core) { - $location.url("/" + core.name + "/core-overview"); + if (core) { + $location.url("/" + core.name + "/core-overview"); + } } $scope.showCollection = function(collection) { diff --git a/solr/webapp/web/js/angular/controllers/collections.js b/solr/webapp/web/js/angular/controllers/collections.js index 93fd5fb1b6d1..a70089d37e37 100644 --- a/solr/webapp/web/js/angular/controllers/collections.js +++ b/solr/webapp/web/js/angular/controllers/collections.js @@ -232,15 +232,15 @@ solrAdminApp.controller('CollectionsController', alert("No collection selected."); return; } + $scope.reloadSuccess = false; + $scope.reloadFailure = false; CollectionsV2.reloadCollection($scope.collection.name, {}, function(error, data,response) { $timeout(function() { if (error) { $scope.reloadFailure = true; - $timeout(function() {$scope.reloadFailure=false}, 1000); - $location.path("/~collections"); + ApiErrorHandler.handle(response); } else { $scope.reloadSuccess = true; - $timeout(function() {$scope.reloadSuccess=false}, 1000); } }); }); diff --git a/solr/webapp/web/js/angular/controllers/core-overview.js b/solr/webapp/web/js/angular/controllers/core-overview.js index 4c97e6d12b07..7ea076b432a4 100644 --- a/solr/webapp/web/js/angular/controllers/core-overview.js +++ b/solr/webapp/web/js/angular/controllers/core-overview.js @@ -56,14 +56,23 @@ function($scope, $rootScope, $routeParams, Luke, CoreInfo, Update, Replication, $scope.refreshPing = function() { Ping.status({core: $routeParams.core}, function(data) { - if (data.error) { - $scope.healthcheckStatus = false; - if (data.error.code == 503) { - $scope.healthcheckMessage = 'Ping request handler is not configured with a healthcheck file.'; - } - } else { + // Three states, and they are not interchangeable. "enabled" and "disabled" both mean a + // healthcheck file is configured, so toggleHealthcheck() works and the widget shows the + // lit / unlit control. "not_configured" means there is no healthcheck file at all, and + // enable/disable would answer 503 - so set a message, which hides the toggle rather than + // offering a control that cannot work. + delete $scope.healthcheckMessage; + if (data.status == "enabled" || data.status == "disabled") { $scope.healthcheckStatus = data.status == "enabled"; + } else { + $scope.healthcheckStatus = false; + $scope.healthcheckMessage = data.status == "not_configured" + ? 'Ping request handler is not configured with a healthcheck file.' + : 'Unexpected ping status: ' + data.status; } + }, function(error) { + $scope.healthcheckStatus = false; + $scope.healthcheckMessage = error.data && error.data.error ? error.data.error.msg : 'Unable to read ping status.'; }); }; diff --git a/solr/webapp/web/js/angular/controllers/schema-designer.js b/solr/webapp/web/js/angular/controllers/schema-designer.js index 7ec4282bb053..a2d08aa79d73 100644 --- a/solr/webapp/web/js/angular/controllers/schema-designer.js +++ b/solr/webapp/web/js/angular/controllers/schema-designer.js @@ -24,6 +24,7 @@ solrAdminApp.controller('SchemaDesignerController', function ($scope, $timeout, $scope.sortableFields = []; $scope.hlFields = []; $scope.types = []; + $scope.preparingSchema = false; $scope.onWarning = function (warnMsg, warnDetails) { $scope.updateWorking = false; @@ -34,6 +35,7 @@ solrAdminApp.controller('SchemaDesignerController', function ($scope, $timeout, $scope.onError = function (errorMsg, errorCode, errorDetails) { $scope.updateWorking = false; + $scope.preparingSchema = false; delete $scope.updateStatusMessage; $scope.designerAPIError = errorMsg; if (errorDetails) { @@ -303,7 +305,9 @@ solrAdminApp.controller('SchemaDesignerController', function ($scope, $timeout, $scope.currentSchema = $scope.newSchema; $scope.sampleMessage = "Please upload or paste some sample documents to analyze for building the '" + $scope.currentSchema + "' schema."; + $scope.preparingSchema = true; SchemaDesigner.post({path: "prep", configSet: $scope.newSchema, copyFrom: $scope.copyFrom}, null, function (data) { + $scope.preparingSchema = false; $scope.initDesignerSettingsFromResponse(data); }, $scope.errorHandler); }; @@ -451,9 +455,11 @@ solrAdminApp.controller('SchemaDesignerController', function ($scope, $timeout, // re-apply the filters on the updated schema $scope.onTreeFilterOptionChanged(); - // Load the Luke schema - Luke.schema({core: data.core}, function (schema) { - Luke.raw({core: data.core}, function (index) { + // Route Luke through the temporary collection so the request reaches its + // active replica even when the Admin UI is connected to a different node. + var lukeTarget = data.tempCollection || data.core; + Luke.schema({core: lukeTarget}, function (schema) { + Luke.raw({core: lukeTarget}, function (index) { $scope.luke = mergeIndexAndSchemaData(index, schema.schema); $scope.types = Object.keys(schema.schema.types); $scope.showSchemaActions = true; diff --git a/solr/webapp/web/partials/collections.html b/solr/webapp/web/partials/collections.html index a05ebe7198f3..8973d31562c6 100644 --- a/solr/webapp/web/partials/collections.html +++ b/solr/webapp/web/partials/collections.html @@ -128,7 +128,8 @@ + ng-class="{success: reloadSuccess, warn: reloadFailure, disabled:!isPermitted(permissions.COLL_EDIT_PERM)}" + aria-live="polite">{{reloadSuccess ? 'Reloaded' : (reloadFailure ? 'Reload failed' : 'Reload')}}

diff --git a/solr/webapp/web/partials/schema-designer.html b/solr/webapp/web/partials/schema-designer.html index 4d7fbd5b4b38..c5c0e1b6e7e0 100644 --- a/solr/webapp/web/partials/schema-designer.html +++ b/solr/webapp/web/partials/schema-designer.html @@ -487,7 +487,9 @@

Sample Documents

{{sampleMessage}}

- +