Copilot commented on code in PR #5110:
URL: https://github.com/apache/solr/pull/5110#discussion_r4251183192
##########
solr/webapp/web/js/angular/controllers/logging.js:
##########
@@ -161,23 +143,15 @@ solrAdminApp.controller('LoggingController',
}
var setString = logger.name + ":" + newLevel;
logger.showOptions = false;
- // Intentionally still v1 (Logging, not LoggingV2): this relies on the
"nodes=all" param to
- // broadcast the level change to every live node. The v2 NodeLoggingApis
endpoint is
- // single-node only until SOLR-16738 wires it up to the new
V2SolrRequestBasedProxy (see the
- // TODO in NodeLogging.java). Move this to LoggingV2.modifyLocalLogLevel
once that lands.
var doSetLevel = function() {
var params = {set: setString};
- // "nodes=all" only makes sense in SolrCloud mode; in standalone
(user-managed) mode the
- // request is handled locally, so the param is omitted entirely.
- if ($scope.isCloudEnabled) {
+ if ($scope.isCloudEnabled && $scope.liveNodes &&
$scope.liveNodes.length > 1) {
Review Comment:
`liveNodes` is populated by a separate asynchronous request, so a level
change made after cloud mode is known but before that request finishes skips
`nodes=all` and changes only the receiving node. Broadcasting to `all` is valid
even for a one-node SolrCloud cluster; key this decision only on cloud mode so
correctness does not depend on node-list timing.
##########
solr/webapp/web/js/angular/services.js:
##########
@@ -88,6 +88,105 @@ solrAdminServices.factory('Metrics',
}
return {handle: handle};
}])
+.factory('NodeTarget',
+ ['$rootScope', function($rootScope) {
+ var STORAGE_KEY = 'adminui.selectedNode';
+ var state = {
+ localNode: null,
+ selectedNode: sessionStorage.getItem(STORAGE_KEY) || null,
+ liveNodes: []
+ };
Review Comment:
The new picker and target persistence have no browser-level coverage,
although this UI has dedicated Selenium suites for Dashboard, node screens,
Logging, and Core Admin. Add a two-node UI test that selects the other node,
verifies node-specific content, reloads the page to exercise session
restoration, and confirms standalone mode does not show the picker; the backend
proxy tests do not exercise this Angular state/route behavior.
##########
solr/webapp/web/index.html:
##########
@@ -167,98 +174,118 @@ <h2>Connection recovered...</h2>
<li id="login" class="global" ng-class="{active:page=='login'}"
ng-show="http401 || currentUser"><p><a href="#/login">{{http401 ? "Login" :
"Logout " + currentUser}}</a></p></li>
- <li id="index" class="global"
ng-class="{active:page=='index'}"><p><a href="#/">Dashboard</a></p></li>
-
<div ng-show="!http401">
- <li id="logging" class="global"
ng-class="{active:page=='logging'}"><p><a href="#/~logging">Logging</a></p>
- <ul ng-show="showingLogging">
- <li class="level"
ng-class="{active:page=='logging-levels'}"><a
href="#/~logging/level">Level</a></li>
+
+ <li class="menu-section" id="menu-section-cluster"
ng-show="isCloudEnabled">Cluster</li>
+
+ <li ng-show="isCloudEnabled" id="cloud-nodes" class="global"
ng-class="{active:page=='cloud-nodes'}"><p><a
href="#/~cloud?view=nodes">Nodes</a></p></li>
+ <li ng-show="isCloudEnabled" id="cloud-tree" class="global"
ng-class="{active:page=='cloud-tree'}"><p><a
href="#/~cloud?view=tree">Tree</a></p></li>
+ <li ng-show="isCloudEnabled" id="cloud-zkstatus" class="global"
ng-class="{active:page=='cloud-zkstatus'}"><p><a
href="#/~cloud?view=zkstatus">ZK Status</a></p></li>
+ <li ng-show="isCloudEnabled" id="cloud-graph" class="global"
ng-class="{active:page=='cloud-graph'}"><p><a
href="#/~cloud?view=graph">Graph</a></p></li>
+
+ <li ng-show="isCloudEnabled" id="security" class="global"
ng-class="{active:page=='security'}"><p><a
href="#/~security">Security</a></p></li>
+
+ <li ng-show="isCloudEnabled && isSchemaDesignerEnabled"
id="schema-designer" class="global"
ng-class="{active:page=='schema-designer'}"><p><a
href="#/~schema-designer">Schema Designer</a></p></li>
+
+ <li ng-show="isCloudEnabled" id="collections" class="global"
ng-class="{active:page=='collections'}"><p><a
href="#/~collections">Collections</a></p></li>
+
+ <div id="collection-selector" class="menu-control"
ng-show="isCloudEnabled">
+ <div id="has-collections" ng-show="collections.length!=0">
+ <select placeholder-text-single="'Collection Selector'"
+ ng-model="currentCollection"
+ chosen
+ search-contains="true"
+ ng-change="showCollection(currentCollection)"
+ ng-options="collection.name for collection in
aliases_and_collections"></select>
+ </div>
+ <p id="has-no-collections" ng-show="collections.length==0"><a
href="#/~collections">
+ No collections available
+ <span>Go and create one</span>
+ </a></p>
+ </div>
+ <div id="collection-menu" class="sub-menu"
ng-show="currentCollection">
+ <ul>
+ <li class="overview" ng-show="currentCollection.type ===
'collection'" ng-class="{active:page=='collection-overview'}"><a
href="#/{{currentCollection.name}}/collection-overview"><span>Overview</span></a></li>
+ <li class="overview" ng-show="currentCollection.type ===
'alias'" ng-class="{active:page=='alias-overview'}"><a
href="#/{{currentCollection.name}}/alias-overview"><span>Overview</span></a></li>
+ <li class="analysis"
ng-show="!isMultiDestAlias(currentCollection)"
ng-class="{active:page=='analysis'}"><a
href="#/{{currentCollection.name}}/analysis"><span>Analysis</span></a></li>
+ <li class="documents"
ng-show="!isMultiDestAlias(currentCollection)"
ng-class="{active:page=='documents'}"><a
href="#/{{currentCollection.name}}/documents"><span>Documents</span></a></li>
+ <li class="paramsets"
ng-show="!isMultiDestAlias(currentCollection)"
ng-class="{active:page=='paramsets'}"><a
href="#/{{currentCollection.name}}/paramsets"><span>Paramsets</span></a></li>
+ <li class="files"
ng-show="!isMultiDestAlias(currentCollection)"
ng-class="{active:page=='files'}"><a
href="#/{{currentCollection.name}}/files"><span>Files</span></a></li>
+ <li class="query" ng-class="{active:page=='query'}"><a
href="#/{{currentCollection.name}}/query"><span>Query</span></a></li>
+ <li class="stream" ng-class="{active:page=='stream'}"><a
href="#/{{currentCollection.name}}/stream"><span>Stream</span></a></li>
+ <li class="sqlquery" ng-class="{active:page=='sqlquery'}"><a
href="#/{{currentCollection.name}}/sqlquery"><span>SQL</span></a></li>
+ <li class="schema"
ng-show="!isMultiDestAlias(currentCollection)"
ng-class="{active:page=='schema'}"><a
href="#/{{currentCollection.name}}/schema"><span>Schema</span></a></li>
</ul>
+ </div>
+
+ <li class="menu-section" id="menu-section-node">Node</li>
+
+ <li id="node-selector-item" class="menu-control"
ng-show="isCloudEnabled && liveNodes.length > 1">
+ <div id="node-selector">
+ <div id="has-nodes">
+ <select placeholder-text-single="'Select node'"
Review Comment:
The new node selector has no `<label>` or ARIA name;
`placeholder-text-single` only supplies visible placeholder text to Chosen and
is not a semantic label. Give the control an accessible name (and ensure it is
applied to Chosen's generated interactive element) so screen-reader users can
identify the selector.
##########
solr/webapp/web/js/angular/app.js:
##########
@@ -525,35 +525,45 @@ solrAdminApp.controller('MainController',
function($scope, $route, $rootScope, $
$scope.cores = [];
$scope.collections = [];
$scope.aliases = [];
+ $scope.liveNodes = [];
+ $scope.isCloudEnabled = false;
Review Comment:
Initializing this to `false` removes the “unknown” state used by
`LoggingLevelController` to wait for asynchronous mode detection. On initial
load, a level change can therefore run the standalone/local path before the
system-info response identifies SolrCloud. Preserve `undefined` until detection
completes.
##########
solr/webapp/web/js/angular/services.js:
##########
@@ -88,6 +88,105 @@ solrAdminServices.factory('Metrics',
}
return {handle: handle};
}])
+.factory('NodeTarget',
+ ['$rootScope', function($rootScope) {
+ var STORAGE_KEY = 'adminui.selectedNode';
+ var state = {
+ localNode: null,
+ selectedNode: sessionStorage.getItem(STORAGE_KEY) || null,
+ liveNodes: []
+ };
+
+ function isRemote() {
+ return !!(state.selectedNode && state.localNode && state.selectedNode
!== state.localNode);
+ }
+
+ function nodesParam() {
+ return isRemote() ? state.selectedNode : undefined;
Review Comment:
A node restored from `sessionStorage` is treated as local until the
asynchronous system-info request supplies `localNode`. Each node-scoped
controller calls `withNodes()` immediately after `resetMenu()`, so after a full
page reload Dashboard, Java Properties, Threads, and Cores issue their only
request to the current node while the badge later reports the persisted remote
node (Logging is wrong until its next poll). Initialize/validate the target
before child controllers fetch, or explicitly reload their route once
local/live-node discovery completes.
##########
solr/webapp/web/js/angular/controllers/cores.js:
##########
@@ -16,15 +16,16 @@
*/
solrAdminApp.controller('CoreAdminController',
- function($scope, $routeParams, $location, $timeout, $route, CoresV2,
Update, Constants, ApiErrorHandler){
+ function($scope, $routeParams, $location, $timeout, $route, CoresV2,
Update, Constants, ApiErrorHandler, NodeTarget){
$scope.resetMenu("cores", Constants.IS_ROOT_PAGE);
$scope.selectedCore = $routeParams.corename; // use 'corename' not
'core' to distinguish from /solr/:core/
$scope.refresh = function() {
- CoresV2.getAllCoreStatus({}, function(error, data, response) {
+ CoresV2.getAllCoreStatus(NodeTarget.withNodes({}), function(error,
data, response) {
$timeout(function() {
if (error) { ApiErrorHandler.handle(response); return; }
+ var payload = NodeTarget.unwrap(data, response) || {};
Review Comment:
This makes Core Admin display status from the selected remote node, but
every action below (`createCore`, `unloadCore`, `renameCore`, `swapCores`, and
`reloadCore`) still omits `nodes` and therefore executes on the local node. The
route remains reachable directly and through the “No cores available” link, so
an operator can inspect a remote core and then mutate an identically named
local core. Either prevent the remote Core Admin view or add safe remote
targeting for all mutations before proxying this read.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
