[
https://issues.apache.org/jira/browse/ARTEMIS-3808?focusedWorklogId=765547&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-765547
]
ASF GitHub Bot logged work on ARTEMIS-3808:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 03/May/22 17:01
Start Date: 03/May/22 17:01
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4061:
URL: https://github.com/apache/activemq-artemis/pull/4061#discussion_r863953256
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQComponent.java:
##########
@@ -29,4 +31,8 @@ default void asyncStop(Runnable callback) throws Exception {
}
boolean isStarted();
+
+ default SimpleString getName() {
+ return SimpleString.toSimpleString("");
+ }
Review Comment:
Using String would seem nicer than expanding the use of SimpleString for
something it doesnt seem needed for. It seemed like you had to change many
existing instances of using String already.
Also the description says "This was necessary in order to actually find the
WebServerComponent in the broker's list of components". Would a simple
alternative be a marker interface, supplied from the broker and implemented by
the base WebServerComponent class, allowing operating on it without needing any
of the widespread String vs SimpleString changes or random-constant name based
coordination ?
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ComponentConstants.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * 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.activemq.artemis.utils;
+
+import org.apache.activemq.artemis.api.core.SimpleString;
+
+public final class ComponentConstants {
+
+ public static final SimpleString WEB_SERVER =
RandomUtil.randomSimpleString();
Review Comment:
A random constant being used for coordination essentially inside the broker
seems a bit icky.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java:
##########
@@ -4479,5 +4482,39 @@ public void replay(String startScan, String endScan,
String address, String targ
server.replay(startScanDate, endScanDate, address, target, filter);
}
+
+ @Override
+ public void stopEmbeddedWebServer() throws Exception {
+ for (ActiveMQComponent component : server.getExternalComponents()) {
Review Comment:
The stop/start/restart seem like maybe they should be coordinated to avoid
falling over each other.
##########
artemis-web/src/test/java/org/apache/activemq/cli/test/WebServerComponentTest.java:
##########
@@ -175,31 +166,31 @@ public void testComponentStopBehavior() throws Exception {
Assert.assertFalse(webServerComponent.isStarted());
webServerComponent.configure(webServerDTO, "./src/test/resources/",
"./src/test/resources/");
webServerComponent.start();
- final int port = webServerComponent.getPort();
// Make the connection attempt.
- CountDownLatch latch = new CountDownLatch(1);
- final ClientHandler clientHandler = new ClientHandler(latch);
- bootstrap.group(group).channel(NioSocketChannel.class).handler(new
ChannelInitializer() {
- @Override
- protected void initChannel(Channel ch) throws Exception {
- ch.pipeline().addLast(new HttpClientCodec());
- ch.pipeline().addLast(clientHandler);
- }
- });
- Channel ch = bootstrap.connect("localhost", port).sync().channel();
+ verifyConnection(webServerComponent.getPort());
+ Assert.assertTrue(webServerComponent.isStarted());
- URI uri = new URI(URL);
- // Prepare the HTTP request.
- HttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1,
HttpMethod.GET, uri.getRawPath());
- request.headers().set(HttpHeaderNames.HOST, "localhost");
+ //usual stop won't actually stop it
+ webServerComponent.stop();
+ assertTrue(webServerComponent.isStarted());
- // Send the HTTP request.
- ch.writeAndFlush(request);
- assertTrue(latch.await(5, TimeUnit.SECONDS));
- assertEquals("12345", clientHandler.body.toString());
- // Wait for the server to close the connection.
- ch.close();
- ch.eventLoop().shutdownNow();
+ webServerComponent.stop(true);
+ Assert.assertFalse(webServerComponent.isStarted());
+ }
+
+ @Test
+ public void testComponentStopStartBehavior() throws Exception {
Review Comment:
Perhaps worth some access control testing, given its likely uses of this
will be restricted to certain people?. If these new operations could be covered
implicitly in some way by existing controls it may also be good to spell out
their addition in docs for upgraders so noone is caught out by them popping up.
Perhaps even specific default config changes?
Issue Time Tracking
-------------------
Worklog Id: (was: 765547)
Time Spent: 2h (was: 1h 50m)
> Support starting/stopping the embedded web server via mangement
> ---------------------------------------------------------------
>
> Key: ARTEMIS-3808
> URL: https://issues.apache.org/jira/browse/ARTEMIS-3808
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Time Spent: 2h
> Remaining Estimate: 0h
>
> It would be useful to be able to cycle the embedded web server if, for
> example, one needed to renew the SSL certificates.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
