Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions changelog/unreleased/solr-18347-admin-ui-defects.yml
Original file line number Diff line number Diff line change
@@ -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
11 changes: 0 additions & 11 deletions dev-docs/admin-ui-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
*
* <p>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.
* <p>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")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, if there is a lot of flakiness in this test, I am getting closer to migrating the Admin UI to using the generated V2 Javascript client, which I think is more reliable. And honestly, I think i might have made some commits that may even have broke the schema-designer.js now that I think of it (not sure), so it may be okay to not worry about this page... It's on my list to get to "done done'!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @epugh — two things that might change the calculus.

It isn't flaky any more. The @AwaitsFix blamed "transient failures at automation speed"; the
real cause was deterministic. The designer addressed Luke by core name, and a core name only
resolves on the node hosting it — with configureCluster(2), that's the wrong node about half the
time. Addressing the collection fixes it: 53 tests, 18 suites, 0 failures.

Your hunch is right. SchemaDesignerResponse.core came in with SOLR-18152 (#4203), commented
"Active replica core name ... used for Luke API calls" — reasonable-looking, just not routable
from an arbitrary node. Nothing else in that migration looks affected; every Schema Designer
assertion passes on top of it.

So I'd keep the test rather than drop the page — it's what would have caught this. And the V2
client would issue the same request against the same core name, so this class of bug survives the
migration either way.


Status — I think this is ready to go in.

  • @janhoy's three comments are answered; the one that needed a change is pushed as 9b5545ddb4.
  • Full Admin UI Selenium suite: 53 tests, 18 suites, 0 failures, 0 skipped. tidy clean,
    PingRequestHandlerTest 6/6.
  • No open review threads and nothing outstanding on my side.

@janhoy — the only reservation you raised was the five-in-one packaging. If you're willing to take
it as-is, I'd appreciate an approval. If not, say so and I'll split it along
PingRequestHandler + Core Overview vs. the four UI-only fixes today.

@epugh — if the Schema Designer question above is settled for you, a second approval would let this
land.

public class AdminUiSchemaDesignerTest extends AdminUiTestBase {

@Test
Expand All @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
4 changes: 3 additions & 1 deletion solr/webapp/web/js/angular/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Comment thread
janhoy marked this conversation as resolved.
}
}

$scope.showCollection = function(collection) {
Expand Down
6 changes: 3 additions & 3 deletions solr/webapp/web/js/angular/controllers/collections.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
});
Expand Down
21 changes: 15 additions & 6 deletions solr/webapp/web/js/angular/controllers/core-overview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Comment thread
janhoy marked this conversation as resolved.
} 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.';
});
};

Expand Down
12 changes: 9 additions & 3 deletions solr/webapp/web/js/angular/controllers/schema-designer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
};
Expand Down Expand Up @@ -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;
Comment thread
janhoy marked this conversation as resolved.
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;
Expand Down
3 changes: 2 additions & 1 deletion solr/webapp/web/partials/collections.html
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@
<button id="delete-alias" ng-class="{action,disabled:!isPermitted(permissions.COLL_EDIT_PERM)}" ng-disabled="!isPermitted(permissions.COLL_EDIT_PERM)" ng-click="toggleDeleteAlias()" ng-show="collection && collection.type === 'alias'"><span>Delete alias</span></button>
<button id="reload" ng-click="reloadCollection()" ng-show="collection && collection.type === 'collection'"
ng-disabled="!isPermitted(permissions.COLL_EDIT_PERM)"
ng-class="{success: reloadSuccess, warn: reloadFailure, disabled:!isPermitted(permissions.COLL_EDIT_PERM)}"><span>Reload</span></button>
ng-class="{success: reloadSuccess, warn: reloadFailure, disabled:!isPermitted(permissions.COLL_EDIT_PERM)}"
aria-live="polite"><span>{{reloadSuccess ? 'Reloaded' : (reloadFailure ? 'Reload failed' : 'Reload')}}</span></button>

<div class="action delete" ng-show="showDelete">

Expand Down
4 changes: 3 additions & 1 deletion solr/webapp/web/partials/schema-designer.html
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,9 @@ <h2><span>Sample Documents</span></h2>
</div>
<div id="analyze-buttons">
<p class="clearfix note" ng-show="sampleMessage && !fileUpload && !sampleDocuments"><span>{{sampleMessage}}</span></p>
<button type="submit" ng-click="doAnalyze()" id="analyze" ng-show="currentSchema && (hasDocsOnServer || sampleDocuments || fileUpload)"><span>Analyze Documents</span></button>
<button type="submit" ng-click="doAnalyze()" id="analyze"
ng-show="currentSchema && (hasDocsOnServer || sampleDocuments || fileUpload)"
ng-disabled="preparingSchema || updateWorking"><span>Analyze Documents</span></button>
</div>
</form>
</div>
Expand Down
Loading