This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new 76122a4557 fix(tcp): fix ignored proxy selector port and property
updates (#7497)
76122a4557 is described below
commit 76122a4557336bf0fb20af97606d33a9985f0d3c
Author: lymerin <[email protected]>
AuthorDate: Sat Oct 10 09:54:41 2026 +0800
fix(tcp): fix ignored proxy selector port and property updates (#7497)
---
.../service/impl/ProxySelectorServiceImpl.java | 24 ++-
.../admin/service/ProxySelectorServiceTest.java | 45 ++++-
.../plugin/tcp/handler/TcpBootstrapFactory.java | 184 +++++++++++++++++----
.../tcp/handler/TcpProxySelectorDataHandler.java | 12 +-
.../tcp/handler/TcpBootstrapFactoryTest.java | 146 +++++++++++++++-
.../handler/TcpProxySelectorDataHandlerTest.java | 68 +++++++-
.../tcp/handler/TcpUpstreamDataHandlerTest.java | 19 ++-
7 files changed, 441 insertions(+), 57 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ProxySelectorServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ProxySelectorServiceImpl.java
index eb609f0fd4..03462919f6 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ProxySelectorServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/ProxySelectorServiceImpl.java
@@ -22,6 +22,7 @@ import org.apache.shenyu.admin.aspect.annotation.Pageable;
import org.apache.shenyu.admin.discovery.DiscoveryLevel;
import org.apache.shenyu.admin.discovery.DiscoveryProcessor;
import org.apache.shenyu.admin.discovery.DiscoveryProcessorHolder;
+import org.apache.shenyu.admin.listener.DataChangedEvent;
import org.apache.shenyu.admin.mapper.DiscoveryHandlerMapper;
import org.apache.shenyu.admin.mapper.DiscoveryMapper;
import org.apache.shenyu.admin.mapper.DiscoveryRelMapper;
@@ -51,10 +52,13 @@ import org.apache.shenyu.admin.transfer.DiscoveryTransfer;
import org.apache.shenyu.admin.utils.ShenyuResultMessage;
import org.apache.shenyu.admin.utils.Assert;
import org.apache.shenyu.common.dto.ProxySelectorData;
+import org.apache.shenyu.common.enums.ConfigGroupEnum;
+import org.apache.shenyu.common.enums.DataEventTypeEnum;
import org.apache.shenyu.common.utils.UUIDUtils;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.springframework.context.ApplicationEventPublisher;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionSynchronization;
@@ -98,11 +102,13 @@ public class ProxySelectorServiceImpl implements
ProxySelectorService {
private final DiscoveryProcessorHolder discoveryProcessorHolder;
+ private final ApplicationEventPublisher eventPublisher;
+
public ProxySelectorServiceImpl(final ProxySelectorMapper
proxySelectorMapper, final DiscoveryMapper discoveryMapper,
final DiscoveryUpstreamMapper
discoveryUpstreamMapper, final DiscoveryHandlerMapper discoveryHandlerMapper,
final DiscoveryRelMapper
discoveryRelMapper,
final SelectorMapper selectorMapper,
- final DiscoveryProcessorHolder
discoveryProcessorHolder) {
+ final DiscoveryProcessorHolder
discoveryProcessorHolder, final ApplicationEventPublisher eventPublisher) {
this.proxySelectorMapper = proxySelectorMapper;
this.discoveryMapper = discoveryMapper;
@@ -111,6 +117,7 @@ public class ProxySelectorServiceImpl implements
ProxySelectorService {
this.discoveryHandlerMapper = discoveryHandlerMapper;
this.selectorMapper = selectorMapper;
this.discoveryProcessorHolder = discoveryProcessorHolder;
+ this.eventPublisher = eventPublisher;
}
/**
@@ -427,6 +434,21 @@ public class ProxySelectorServiceImpl implements
ProxySelectorService {
.map(DiscoveryTransfer.INSTANCE::mapToDTO).collect(Collectors.toList());
DiscoveryProcessor discoveryProcessor =
discoveryProcessorHolder.chooseProcessor(discoveryDO.getDiscoveryType());
discoveryProcessor.changeUpstream(DiscoveryTransfer.INSTANCE.mapToDTO(proxySelectorDO),
fetchAll);
+ DataChangedEvent event = new
DataChangedEvent(ConfigGroupEnum.PROXY_SELECTOR, DataEventTypeEnum.UPDATE,
+
Collections.singletonList(DiscoveryTransfer.INSTANCE.mapToData(DiscoveryTransfer.INSTANCE.mapToDTO(proxySelectorDO))));
+ if (TransactionSynchronizationManager.isSynchronizationActive()) {
+ TransactionSynchronizationManager.registerSynchronization(new
TransactionSynchronization() {
+ @Override
+ public void afterCompletion(final int status) {
+ // Completion runs after synchronization is cleared, so
the cluster dispatcher does not defer the event again.
+ if (status == STATUS_COMMITTED) {
+ eventPublisher.publishEvent(event);
+ }
+ }
+ });
+ } else {
+ eventPublisher.publishEvent(event);
+ }
return ShenyuResultMessage.UPDATE_SUCCESS;
}
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ProxySelectorServiceTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ProxySelectorServiceTest.java
index bf38597066..84c14a2c5a 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ProxySelectorServiceTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/ProxySelectorServiceTest.java
@@ -19,6 +19,7 @@ package org.apache.shenyu.admin.service;
import org.apache.shenyu.admin.discovery.DiscoveryProcessor;
import org.apache.shenyu.admin.discovery.DiscoveryProcessorHolder;
+import org.apache.shenyu.admin.listener.DataChangedEvent;
import org.apache.shenyu.admin.exception.ValidFailException;
import org.apache.shenyu.admin.mapper.DiscoveryHandlerMapper;
import org.apache.shenyu.admin.mapper.DiscoveryMapper;
@@ -39,6 +40,7 @@ import org.apache.shenyu.admin.model.vo.ProxySelectorVO;
import org.apache.shenyu.admin.service.impl.ProxySelectorServiceImpl;
import org.apache.shenyu.admin.utils.ShenyuResultMessage;
import org.apache.shenyu.common.dto.ProxySelectorData;
+import org.apache.shenyu.common.enums.ConfigGroupEnum;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -49,6 +51,9 @@ import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;
+import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.transaction.support.TransactionSynchronization;
+import
org.springframework.transaction.support.TransactionSynchronizationManager;
import java.sql.Timestamp;
import java.util.ArrayList;
@@ -102,11 +107,14 @@ class ProxySelectorServiceTest {
@Mock
private DiscoveryProcessorHolder discoveryProcessorHolder;
+ @Mock
+ private ApplicationEventPublisher eventPublisher;
+
@BeforeEach
void testSetUp() {
proxySelectorService = new
ProxySelectorServiceImpl(proxySelectorMapper, discoveryMapper,
discoveryUpstreamMapper,
- discoveryHandlerMapper, discoveryRelMapper, selectorMapper,
discoveryProcessorHolder);
+ discoveryHandlerMapper, discoveryRelMapper, selectorMapper,
discoveryProcessorHolder, eventPublisher);
}
@Test
@@ -246,14 +254,17 @@ class ProxySelectorServiceTest {
assertEquals(proxySelectorService.createOrUpdate(proxySelectorDTO),
ShenyuResultMessage.CREATE_SUCCESS);
}
- @Test
- void testUpdateWithNullDiscoveryUpstreams() {
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testUpdatePublishesListenerConfigurationAfterCommit(final boolean
commit) {
ProxySelectorAddDTO proxySelectorDTO = new ProxySelectorAddDTO();
proxySelectorDTO.setId("proxy-1");
proxySelectorDTO.setName("test");
- proxySelectorDTO.setPluginName("test");
+ proxySelectorDTO.setPluginName("tcp");
proxySelectorDTO.setForwardPort(8080);
+ proxySelectorDTO.setNamespaceId("namespace-1");
+ proxySelectorDTO.setProps("{\"loadBalance\":\"roundRobin\"}");
proxySelectorDTO.setDiscovery(new ProxySelectorAddDTO.Discovery());
DiscoveryRelDO discoveryRelDO = new DiscoveryRelDO();
@@ -273,8 +284,30 @@ class ProxySelectorServiceTest {
given(discoveryProcessorHolder.chooseProcessor("local")).willReturn(discoveryProcessor);
given(discoveryUpstreamMapper.selectByDiscoveryHandlerId("handler-1")).willReturn(Collections.emptyList());
- assertEquals(proxySelectorService.update(proxySelectorDTO),
ShenyuResultMessage.UPDATE_SUCCESS);
- verify(discoveryUpstreamMapper,
never()).deleteByDiscoveryHandlerId(any());
+ TransactionSynchronizationManager.initSynchronization();
+ try {
+ assertEquals(proxySelectorService.update(proxySelectorDTO),
ShenyuResultMessage.UPDATE_SUCCESS);
+ verify(discoveryUpstreamMapper,
never()).deleteByDiscoveryHandlerId(any());
+ verifyNoInteractions(eventPublisher);
+ int status = commit ? TransactionSynchronization.STATUS_COMMITTED
: TransactionSynchronization.STATUS_ROLLED_BACK;
+
TransactionSynchronizationManager.getSynchronizations().forEach(synchronization
-> synchronization.afterCompletion(status));
+ if (!commit) {
+ verifyNoInteractions(eventPublisher);
+ return;
+ }
+ ArgumentCaptor<DataChangedEvent> captor =
ArgumentCaptor.forClass(DataChangedEvent.class);
+ verify(eventPublisher).publishEvent(captor.capture());
+ DataChangedEvent event = captor.getValue();
+ assertEquals(ConfigGroupEnum.PROXY_SELECTOR, event.getGroupKey());
+ List<?> payload = (List<?>) event.getSource();
+ assertEquals(1, payload.size());
+ ProxySelectorData data = (ProxySelectorData) payload.get(0);
+ assertEquals("test", data.getName());
+ assertEquals(8080, data.getForwardPort());
+ assertEquals("roundRobin",
data.getProps().getProperty("loadBalance"));
+ } finally {
+ TransactionSynchronizationManager.clearSynchronization();
+ }
}
@ParameterizedTest
diff --git
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/main/java/org/apache/shenyu/plugin/tcp/handler/TcpBootstrapFactory.java
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/main/java/org/apache/shenyu/plugin/tcp/handler/TcpBootstrapFactory.java
index cf0fe0ee8d..9f65356572 100644
---
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/main/java/org/apache/shenyu/plugin/tcp/handler/TcpBootstrapFactory.java
+++
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/main/java/org/apache/shenyu/plugin/tcp/handler/TcpBootstrapFactory.java
@@ -26,11 +26,15 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Collections;
+import java.util.HashSet;
import java.util.Objects;
+import java.util.Properties;
+import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.locks.ReentrantLock;
/**
* TcpBootstrapFactory.
@@ -41,10 +45,14 @@ public final class TcpBootstrapFactory {
private static final TcpBootstrapFactory SINGLETON = new
TcpBootstrapFactory();
- private final ConcurrentMap<String, BootstrapServer> cache = new
ConcurrentHashMap<>();
+ private final ConcurrentMap<String, CachedServer> cache = new
ConcurrentHashMap<>();
private final ConcurrentMap<String, CompletableFuture<BootstrapServer>>
creations = new ConcurrentHashMap<>();
+ // Initial creation uses single-flight; only replacement and removal share
these locks.
+ // Retain locks so waiting callers always synchronize on the same instance.
+ private final ConcurrentMap<String, ReentrantLock> selectorLocks = new
ConcurrentHashMap<>();
+
private TcpBootstrapFactory() {
}
@@ -66,12 +74,70 @@ public final class TcpBootstrapFactory {
public BootstrapServer createBootstrapServer(final TcpServerConfiguration
configuration) {
EventBus eventBus = new EventBus();
BootstrapServer bootstrapServer = new TcpBootstrapServer(eventBus);
- bootstrapServer.start(configuration);
+ try {
+ bootstrapServer.start(configuration);
+ } catch (RuntimeException ex) {
+ // Initialization can fail before start() enters its bind failure
cleanup.
+ try {
+ bootstrapServer.shutdown();
+ } catch (RuntimeException cleanupFailure) {
+ ex.addSuppressed(cleanupFailure);
+ }
+ throw ex;
+ }
return bootstrapServer;
}
/**
- * Create and cache a bootstrap server if absent.
+ * Create a bootstrap server or replace it when its listener configuration
changes.
+ *
+ * @param configuration configuration
+ * @return true if a bootstrap server was created or replaced
+ */
+ public boolean createOrUpdateBootstrapServer(final TcpServerConfiguration
configuration) {
+ String selectorName = configuration.getPluginSelectorName();
+ TcpServerConfiguration snapshot = snapshot(configuration);
+ while (true) {
+ CachedServer cachedServer = cache.get(selectorName);
+ if (Objects.isNull(cachedServer)) {
+ if (createBootstrapServerIfAbsent(snapshot)) {
+ return true;
+ }
+ // Another creation may have applied a different configuration.
+ continue;
+ }
+ ReentrantLock lock = selectorLocks.computeIfAbsent(selectorName,
key -> new ReentrantLock());
+ lock.lock();
+ try {
+ // A replacement or removal may have changed the cached
listener while this caller waited.
+ cachedServer = cache.get(selectorName);
+ if (Objects.isNull(cachedServer)) {
+ continue;
+ }
+ if (cachedServer.matches(snapshot)) {
+ return false;
+ }
+ if (cachedServer.configuration.getPort() ==
snapshot.getPort()) {
+ replaceOnSamePort(selectorName, cachedServer, snapshot);
+ } else {
+ // Different port: keep the current listener available
until the replacement starts.
+ BootstrapServer replacement =
createBootstrapServer(snapshot);
+ cache.put(selectorName, new CachedServer(replacement,
snapshot));
+ try {
+ cachedServer.server.shutdown();
+ } catch (RuntimeException ex) {
+ LOG.error("Failed to shutdown replaced
TcpBootstrapServer for selector {}", selectorName, ex);
+ }
+ }
+ return true;
+ } finally {
+ lock.unlock();
+ }
+ }
+ }
+
+ /**
+ * Create and cache a bootstrap server if absent, sharing the in-flight
creation result.
*
* @param configuration configuration
* @return true if a bootstrap server was created
@@ -87,24 +153,33 @@ public final class TcpBootstrapFactory {
awaitCreation(existingCreation);
return false;
}
+ UpstreamProvider upstreamProvider = UpstreamProvider.getSingleton();
+ boolean initializeUpstreams = false;
try {
- BootstrapServer cachedServer = cache.get(selectorName);
+ CachedServer cachedServer = cache.get(selectorName);
if (Objects.nonNull(cachedServer)) {
- creation.complete(cachedServer);
+ creation.complete(cachedServer.server);
return false;
}
- UpstreamProvider.getSingleton().createUpstreams(selectorName,
Collections.emptyList());
- BootstrapServer bootstrapServer =
createBootstrapServer(configuration);
- BootstrapServer existingServer = cache.putIfAbsent(selectorName,
bootstrapServer);
+ // Discovery upstreams can arrive before the listener; preserve
that existing entry.
+ initializeUpstreams = !upstreamProvider.inCache(selectorName);
+ if (initializeUpstreams) {
+ upstreamProvider.createUpstreams(selectorName,
Collections.emptyList());
+ }
+ TcpServerConfiguration snapshot = snapshot(configuration);
+ BootstrapServer bootstrapServer = createBootstrapServer(snapshot);
+ CachedServer existingServer = cache.putIfAbsent(selectorName, new
CachedServer(bootstrapServer, snapshot));
if (Objects.nonNull(existingServer)) {
bootstrapServer.shutdown();
- creation.complete(existingServer);
+ creation.complete(existingServer.server);
return false;
}
creation.complete(bootstrapServer);
return true;
} catch (RuntimeException ex) {
- UpstreamProvider.getSingleton().removeUpstreams(selectorName);
+ if (initializeUpstreams) {
+ upstreamProvider.removeUpstreams(selectorName);
+ }
creation.completeExceptionally(ex);
throw ex;
} finally {
@@ -112,6 +187,22 @@ public final class TcpBootstrapFactory {
}
}
+ private void replaceOnSamePort(final String selectorName, final
CachedServer previous, final TcpServerConfiguration configuration) {
+ try {
+ // Same port: release the current listener before the replacement
can bind.
+ previous.server.shutdown();
+ cache.put(selectorName, new
CachedServer(createBootstrapServer(configuration), configuration));
+ } catch (RuntimeException ex) {
+ try {
+ cache.put(selectorName, new
CachedServer(createBootstrapServer(previous.configuration),
previous.configuration));
+ } catch (RuntimeException recoveryFailure) {
+ cache.remove(selectorName);
+ ex.addSuppressed(recoveryFailure);
+ }
+ throw ex;
+ }
+ }
+
private static void awaitCreation(final CompletableFuture<BootstrapServer>
creation) {
try {
creation.join();
@@ -127,14 +218,26 @@ public final class TcpBootstrapFactory {
}
}
+ private static TcpServerConfiguration snapshot(final
TcpServerConfiguration configuration) {
+ TcpServerConfiguration snapshot = new TcpServerConfiguration();
+ snapshot.setPluginSelectorName(configuration.getPluginSelectorName());
+ snapshot.setPort(configuration.getPort());
+ Properties props = new Properties();
+ if (Objects.nonNull(configuration.getProps())) {
+ props.putAll(configuration.getProps());
+ }
+ snapshot.setProps(props);
+ return snapshot;
+ }
+
/**
- * cache bootstrapServer by selectorName.
+ * Cache a bootstrap server with its listener configuration.
*
- * @param selectorName selectorName
+ * @param configuration configuration
* @param bootstrapServer bootstrapServer
*/
- public void cache(final String selectorName, final BootstrapServer
bootstrapServer) {
- cache.put(selectorName, bootstrapServer);
+ public void cache(final TcpServerConfiguration configuration, final
BootstrapServer bootstrapServer) {
+ cache.put(configuration.getPluginSelectorName(), new
CachedServer(bootstrapServer, snapshot(configuration)));
}
/**
@@ -154,7 +257,8 @@ public final class TcpBootstrapFactory {
* @return BootstrapServer
*/
public BootstrapServer removeCache(final String selectorName) {
- return cache.remove(selectorName);
+ CachedServer cachedServer = cache.remove(selectorName);
+ return Objects.isNull(cachedServer) ? null : cachedServer.server;
}
/**
@@ -164,26 +268,31 @@ public final class TcpBootstrapFactory {
* @return true if a bootstrap server was removed
*/
public boolean removeAndShutdown(final String selectorName) {
- BootstrapServer bootstrapServer = cache.remove(selectorName);
- UpstreamProvider.getSingleton().removeUpstreams(selectorName);
- if (Objects.isNull(bootstrapServer)) {
- return false;
+ ReentrantLock lock = selectorLocks.computeIfAbsent(selectorName, key
-> new ReentrantLock());
+ lock.lock();
+ try {
+ BootstrapServer bootstrapServer = removeCache(selectorName);
+ UpstreamProvider.getSingleton().removeUpstreams(selectorName);
+ if (Objects.isNull(bootstrapServer)) {
+ return false;
+ }
+ bootstrapServer.shutdown();
+ return true;
+ } finally {
+ lock.unlock();
}
- bootstrapServer.shutdown();
- return true;
}
/**
* Clear cache.
*/
public void clearCache() {
- cache.forEach((selectorName, bootstrapServer) -> {
- if (cache.remove(selectorName, bootstrapServer)) {
- try {
- bootstrapServer.shutdown();
- } catch (RuntimeException ex) {
- LOG.error("Failed to shutdown TcpBootstrapServer for
selector {}", selectorName, ex);
- }
+ Set<String> selectorNames = new HashSet<>(cache.keySet());
+ selectorNames.forEach(selectorName -> {
+ try {
+ removeAndShutdown(selectorName);
+ } catch (RuntimeException ex) {
+ LOG.error("Failed to shutdown TcpBootstrapServer for selector
{}", selectorName, ex);
}
});
UpstreamProvider.getSingleton().clear();
@@ -196,7 +305,24 @@ public final class TcpBootstrapFactory {
* @return BootstrapServer
*/
public BootstrapServer getCache(final String selectorName) {
- return cache.get(selectorName);
+ CachedServer cachedServer = cache.get(selectorName);
+ return Objects.isNull(cachedServer) ? null : cachedServer.server;
+ }
+
+ private static final class CachedServer {
+
+ private final BootstrapServer server;
+
+ private final TcpServerConfiguration configuration;
+
+ private CachedServer(final BootstrapServer server, final
TcpServerConfiguration configuration) {
+ this.server = server;
+ this.configuration = configuration;
+ }
+
+ private boolean matches(final TcpServerConfiguration incoming) {
+ return configuration.getPort() == incoming.getPort() &&
configuration.getProps().equals(incoming.getProps());
+ }
}
}
diff --git
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/main/java/org/apache/shenyu/plugin/tcp/handler/TcpProxySelectorDataHandler.java
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/main/java/org/apache/shenyu/plugin/tcp/handler/TcpProxySelectorDataHandler.java
index b39bdce1d1..3a198be3ec 100644
---
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/main/java/org/apache/shenyu/plugin/tcp/handler/TcpProxySelectorDataHandler.java
+++
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/main/java/org/apache/shenyu/plugin/tcp/handler/TcpProxySelectorDataHandler.java
@@ -31,20 +31,16 @@ public class TcpProxySelectorDataHandler implements
ProxySelectorDataHandler {
@Override
public void handlerProxySelector(final ProxySelectorData
proxySelectorData) {
String name = proxySelectorData.getName();
- TcpBootstrapFactory factory = TcpBootstrapFactory.getSingleton();
- if (factory.inCache(name)) {
- LOG.info("shenyu already created TcpBootstrapServer name is {}
port is {}", name, proxySelectorData.getForwardPort());
- return;
- }
Integer forwardPort = proxySelectorData.getForwardPort();
TcpServerConfiguration tcpServerConfiguration = new
TcpServerConfiguration();
tcpServerConfiguration.setPort(forwardPort);
tcpServerConfiguration.setProps(proxySelectorData.getProps());
tcpServerConfiguration.setPluginSelectorName(name);
- if (factory.createBootstrapServerIfAbsent(tcpServerConfiguration)) {
- LOG.info("shenyu create TcpBootstrapServer success name is {} port
is {}", name, forwardPort);
+ TcpBootstrapFactory factory = TcpBootstrapFactory.getSingleton();
+ if (factory.createOrUpdateBootstrapServer(tcpServerConfiguration)) {
+ LOG.info("shenyu apply TcpBootstrapServer configuration success
name is {} port is {}", name, forwardPort);
} else {
- LOG.info("shenyu already created TcpBootstrapServer name is {}
port is {}", name, forwardPort);
+ LOG.debug("shenyu TcpBootstrapServer configuration unchanged name
is {} port is {}", name, forwardPort);
}
}
diff --git
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpBootstrapFactoryTest.java
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpBootstrapFactoryTest.java
index 95b6d7e05c..07f3c5f985 100644
---
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpBootstrapFactoryTest.java
+++
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpBootstrapFactoryTest.java
@@ -18,17 +18,23 @@
package org.apache.shenyu.plugin.tcp.handler;
import org.apache.shenyu.protocol.tcp.BootstrapServer;
+import org.apache.shenyu.protocol.tcp.TcpBootstrapServer;
import org.apache.shenyu.protocol.tcp.TcpServerConfiguration;
import org.apache.shenyu.protocol.tcp.UpstreamProvider;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.ArgumentCaptor;
+import org.mockito.MockedConstruction;
import java.io.IOException;
import java.lang.reflect.Field;
import java.net.ServerSocket;
import java.util.ArrayList;
import java.util.List;
+import java.util.Properties;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.CountDownLatch;
@@ -36,16 +42,23 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockConstruction;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
public final class TcpBootstrapFactoryTest {
@@ -129,6 +142,134 @@ public final class TcpBootstrapFactoryTest {
}
}
+ @Test
+ public void shouldApplyDifferentConfigurationAfterWaitingForCreation()
throws Exception {
+ TcpBootstrapFactory factorySpy = spy(factory);
+ BootstrapServer original = mock(BootstrapServer.class);
+ BootstrapServer replacement = mock(BootstrapServer.class);
+
doReturn(replacement).when(factorySpy).createBootstrapServer(any(TcpServerConfiguration.class));
+ CountDownLatch creationAwaited = new CountDownLatch(1);
+ CompletableFuture<BootstrapServer> creation = spy(new
CompletableFuture<BootstrapServer>());
+ doAnswer(invocation -> {
+ creationAwaited.countDown();
+ return invocation.callRealMethod();
+ }).when(creation).join();
+ ConcurrentMap<String, CompletableFuture<BootstrapServer>> creations =
getCreations();
+ creations.put(FIRST_SELECTOR, creation);
+ ExecutorService executor = Executors.newSingleThreadExecutor();
+ try {
+ final Future<Boolean> result = executor.submit(() ->
factorySpy.createOrUpdateBootstrapServer(configuration(FIRST_SELECTOR, 9001)));
+ assertTrue(creationAwaited.await(5, TimeUnit.SECONDS));
+ factory.cache(configuration(FIRST_SELECTOR, 9000), original);
+ creation.complete(original);
+ assertTrue(result.get(5, TimeUnit.SECONDS));
+ assertSame(replacement, factory.getCache(FIRST_SELECTOR));
+ ArgumentCaptor<TcpServerConfiguration> captor =
ArgumentCaptor.forClass(TcpServerConfiguration.class);
+ verify(factorySpy).createBootstrapServer(captor.capture());
+ assertEquals(9001, captor.getValue().getPort());
+ verify(original).shutdown();
+ } finally {
+ creation.complete(original);
+ creations.remove(FIRST_SELECTOR, creation);
+ executor.shutdownNow();
+ assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS));
+ }
+ }
+
+ @Test
+ public void shouldSerializeReplacementAndRemovalForSameSelector() throws
Exception {
+ TcpBootstrapFactory factorySpy = spy(factory);
+ BootstrapServer original = mock(BootstrapServer.class);
+ BootstrapServer replacement = mock(BootstrapServer.class);
+ factory.cache(configuration(FIRST_SELECTOR, 0), original);
+ TcpServerConfiguration update = configuration(FIRST_SELECTOR, 0);
+ update.getProps().setProperty("clientMaxConnections", "30");
+
doReturn(replacement).when(factorySpy).createBootstrapServer(any(TcpServerConfiguration.class));
+ CountDownLatch shutdownStarted = new CountDownLatch(1);
+ CountDownLatch releaseShutdown = new CountDownLatch(1);
+ doAnswer(invocation -> {
+ shutdownStarted.countDown();
+ assertTrue(releaseShutdown.await(5, TimeUnit.SECONDS));
+ return null;
+ }).when(original).shutdown();
+ CountDownLatch removalStarted = new CountDownLatch(1);
+ ExecutorService executor = Executors.newFixedThreadPool(2);
+ try {
+ final Future<Boolean> first = executor.submit(() ->
factorySpy.createOrUpdateBootstrapServer(update));
+ assertTrue(shutdownStarted.await(5, TimeUnit.SECONDS));
+ final Future<Boolean> second = executor.submit(() -> {
+ removalStarted.countDown();
+ return factorySpy.removeAndShutdown(FIRST_SELECTOR);
+ });
+ assertTrue(removalStarted.await(5, TimeUnit.SECONDS));
+ assertThrows(TimeoutException.class, () -> second.get(100,
TimeUnit.MILLISECONDS));
+ assertSame(original, factory.getCache(FIRST_SELECTOR));
+ verify(original, times(1)).shutdown();
+ releaseShutdown.countDown();
+ assertTrue(first.get(5, TimeUnit.SECONDS));
+ assertTrue(second.get(5, TimeUnit.SECONDS));
+ assertNull(factory.getCache(FIRST_SELECTOR));
+ verify(replacement).shutdown();
+ verify(factorySpy,
times(1)).createBootstrapServer(any(TcpServerConfiguration.class));
+ } finally {
+ releaseShutdown.countDown();
+ executor.shutdownNow();
+ assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS));
+ }
+ }
+
+ @Test
+ public void shouldReplacePropertiesOnSamePort() throws IOException {
+ TcpServerConfiguration configuration = configuration(FIRST_SELECTOR,
getFreePort());
+ configuration.setProps(null);
+ factory.createOrUpdateBootstrapServer(configuration);
+ BootstrapServer previous = factory.getCache(FIRST_SELECTOR);
+ configuration.setProps(new Properties());
+ configuration.getProps().setProperty("clientMaxConnections", "30");
+ try (MockedConstruction<TcpBootstrapServer> servers =
mockConstruction(TcpBootstrapServer.class)) {
+ assertTrue(factory.createOrUpdateBootstrapServer(configuration));
+ BootstrapServer replacement = factory.getCache(FIRST_SELECTOR);
+ assertNotSame(previous, replacement);
+ ArgumentCaptor<TcpServerConfiguration> captor =
ArgumentCaptor.forClass(TcpServerConfiguration.class);
+ verify(replacement).start(captor.capture());
+ assertEquals("30",
captor.getValue().getProps().getProperty("clientMaxConnections"));
+ try (ServerSocket releasedPort = new
ServerSocket(configuration.getPort())) {
+ assertEquals(configuration.getPort(),
releasedPort.getLocalPort());
+ }
+ assertFalse(factory.createOrUpdateBootstrapServer(configuration));
+ assertEquals(1, servers.constructed().size());
+ factory.removeAndShutdown(FIRST_SELECTOR);
+ verify(replacement).shutdown();
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void shouldRetainPreviousConfigurationAfterUpdateFailure(final
boolean samePort) throws IOException {
+ TcpServerConfiguration configuration = configuration(FIRST_SELECTOR,
getFreePort());
+ factory.createOrUpdateBootstrapServer(configuration);
+ BootstrapServer previous = factory.getCache(FIRST_SELECTOR);
+ UpstreamProvider upstreamProvider = UpstreamProvider.getSingleton();
+ upstreamProvider.registerSelector("first-id", FIRST_SELECTOR);
+ try (ServerSocket occupiedPort = new ServerSocket(0)) {
+ TcpServerConfiguration update = configuration(FIRST_SELECTOR,
samePort ? configuration.getPort() : occupiedPort.getLocalPort());
+ if (samePort) {
+ update.getProps().setProperty("workerGroupThreadCount",
"invalid");
+ }
+ assertThrows(RuntimeException.class, () ->
factory.createOrUpdateBootstrapServer(update));
+ BootstrapServer retained = factory.getCache(FIRST_SELECTOR);
+ assertNotNull(retained);
+ if (samePort) {
+ assertNotSame(previous, retained);
+ } else {
+ assertSame(previous, retained);
+ }
+ assertEquals(FIRST_SELECTOR,
upstreamProvider.getSelectorName("first-id"));
+ assertTrue(upstreamProvider.inCache(FIRST_SELECTOR));
+ assertFalse(factory.createOrUpdateBootstrapServer(configuration));
+ }
+ }
+
@Test
public void shouldNotBlockDifferentSelectorRemovalDuringShutdown() throws
Exception {
BootstrapServer blockingServer = mock(BootstrapServer.class);
@@ -140,8 +281,8 @@ public final class TcpBootstrapFactoryTest {
assertTrue(releaseShutdown.await(5, TimeUnit.SECONDS));
return null;
}).when(blockingServer).shutdown();
- factory.cache(FIRST_SELECTOR, blockingServer);
- factory.cache(SECOND_SELECTOR, secondServer);
+ factory.cache(configuration(FIRST_SELECTOR, 0), blockingServer);
+ factory.cache(configuration(SECOND_SELECTOR, 0), secondServer);
ExecutorService executor = Executors.newFixedThreadPool(2);
try {
@@ -177,4 +318,5 @@ public final class TcpBootstrapFactoryTest {
field.setAccessible(true);
return (ConcurrentMap<String, CompletableFuture<BootstrapServer>>)
field.get(factory);
}
+
}
diff --git
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpProxySelectorDataHandlerTest.java
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpProxySelectorDataHandlerTest.java
index ad0d8359dc..62fc301689 100644
---
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpProxySelectorDataHandlerTest.java
+++
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpProxySelectorDataHandlerTest.java
@@ -17,18 +17,30 @@
package org.apache.shenyu.plugin.tcp.handler;
+import org.apache.shenyu.common.dto.DiscoveryUpstreamData;
+import org.apache.shenyu.common.dto.ProxySelectorData;
import org.apache.shenyu.plugin.base.cache.CommonProxySelectorDataSubscriber;
import org.apache.shenyu.protocol.tcp.BootstrapServer;
+import org.apache.shenyu.protocol.tcp.TcpServerConfiguration;
import org.apache.shenyu.protocol.tcp.UpstreamProvider;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import java.io.IOException;
+import java.net.BindException;
+import java.net.ServerSocket;
import java.util.Collections;
+import java.util.List;
+import java.util.Properties;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@@ -55,12 +67,46 @@ public final class TcpProxySelectorDataHandlerTest {
factory.clearCache();
}
+ @Test
+ public void shouldApplyChangedPortThroughSubscriberAndPreserveUpstreams()
throws IOException {
+ ProxySelectorData data = new ProxySelectorData();
+ data.setPluginName("tcp");
+ data.setName(FIRST_SELECTOR);
+ data.setForwardPort(freePort());
+ data.setProps(new Properties());
+ CommonProxySelectorDataSubscriber subscriber = new
CommonProxySelectorDataSubscriber(Collections.singletonList(new
TcpProxySelectorDataHandler()));
+ subscriber.onSubscribe(data);
+ BootstrapServer original = factory.getCache(FIRST_SELECTOR);
+ subscriber.onSubscribe(data);
+ assertSame(original, factory.getCache(FIRST_SELECTOR));
+ UpstreamProvider upstreamProvider = UpstreamProvider.getSingleton();
+ List<DiscoveryUpstreamData> upstreams =
Collections.singletonList(DiscoveryUpstreamData.builder().url("127.0.0.1:10001").build());
+ upstreamProvider.createUpstreams(FIRST_SELECTOR, upstreams);
+ upstreamProvider.registerSelector(FIRST_SELECTOR_ID, FIRST_SELECTOR);
+ int previousPort = data.getForwardPort();
+ data.setForwardPort(freePort());
+
+ subscriber.onSubscribe(data);
+
+ assertNotSame(original, factory.getCache(FIRST_SELECTOR));
+ assertSame(upstreams, upstreamProvider.provide(FIRST_SELECTOR));
+ assertEquals(FIRST_SELECTOR,
upstreamProvider.getSelectorName(FIRST_SELECTOR_ID));
+ try (ServerSocket releasedPort = new ServerSocket(previousPort)) {
+ assertEquals(previousPort, releasedPort.getLocalPort());
+ }
+ assertThrows(BindException.class, () -> {
+ try (ServerSocket occupiedPort = new
ServerSocket(data.getForwardPort())) {
+ assertEquals(data.getForwardPort().intValue(),
occupiedPort.getLocalPort());
+ }
+ });
+ }
+
@Test
public void testRefreshThroughSubscriber() {
BootstrapServer firstServer = mock(BootstrapServer.class);
BootstrapServer secondServer = mock(BootstrapServer.class);
- factory.cache(FIRST_SELECTOR, firstServer);
- factory.cache(SECOND_SELECTOR, secondServer);
+ factory.cache(configuration(FIRST_SELECTOR), firstServer);
+ factory.cache(configuration(SECOND_SELECTOR), secondServer);
UpstreamProvider.getSingleton().createUpstreams(FIRST_SELECTOR,
Collections.emptyList());
UpstreamProvider.getSingleton().createUpstreams(SECOND_SELECTOR,
Collections.emptyList());
UpstreamProvider.getSingleton().registerSelector(FIRST_SELECTOR_ID,
FIRST_SELECTOR);
@@ -83,8 +129,8 @@ public final class TcpProxySelectorDataHandlerTest {
BootstrapServer failingServer = mock(BootstrapServer.class);
BootstrapServer secondServer = mock(BootstrapServer.class);
doThrow(new IllegalStateException("shutdown
failed")).when(failingServer).shutdown();
- factory.cache(FIRST_SELECTOR, failingServer);
- factory.cache(SECOND_SELECTOR, secondServer);
+ factory.cache(configuration(FIRST_SELECTOR), failingServer);
+ factory.cache(configuration(SECOND_SELECTOR), secondServer);
UpstreamProvider.getSingleton().createUpstreams(FIRST_SELECTOR,
Collections.emptyList());
UpstreamProvider.getSingleton().registerSelector(FIRST_SELECTOR_ID,
FIRST_SELECTOR);
@@ -101,7 +147,7 @@ public final class TcpProxySelectorDataHandlerTest {
@Test
public void testRemoveProxySelector() {
BootstrapServer bootstrapServer = mock(BootstrapServer.class);
- factory.cache(FIRST_SELECTOR, bootstrapServer);
+ factory.cache(configuration(FIRST_SELECTOR), bootstrapServer);
UpstreamProvider.getSingleton().createUpstreams(FIRST_SELECTOR,
Collections.emptyList());
UpstreamProvider.getSingleton().registerSelector(FIRST_SELECTOR_ID,
FIRST_SELECTOR);
TcpProxySelectorDataHandler handler = new
TcpProxySelectorDataHandler();
@@ -114,4 +160,16 @@ public final class TcpProxySelectorDataHandlerTest {
assertNull(UpstreamProvider.getSingleton().getSelectorName(FIRST_SELECTOR_ID));
assertDoesNotThrow(() -> handler.removeProxySelector(FIRST_SELECTOR));
}
+
+ private static TcpServerConfiguration configuration(final String
selectorName) {
+ TcpServerConfiguration configuration = new TcpServerConfiguration();
+ configuration.setPluginSelectorName(selectorName);
+ return configuration;
+ }
+
+ private static int freePort() throws IOException {
+ try (ServerSocket socket = new ServerSocket(0)) {
+ return socket.getLocalPort();
+ }
+ }
}
diff --git
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpUpstreamDataHandlerTest.java
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpUpstreamDataHandlerTest.java
index f9c2594d59..88212d52fd 100644
---
a/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpUpstreamDataHandlerTest.java
+++
b/shenyu-plugin/shenyu-plugin-proxy/shenyu-plugin-tcp/src/test/java/org/apache/shenyu/plugin/tcp/handler/TcpUpstreamDataHandlerTest.java
@@ -21,6 +21,7 @@ import org.apache.shenyu.common.dto.DiscoverySyncData;
import org.apache.shenyu.sync.data.api.DiscoveryUpstreamKey;
import org.apache.shenyu.common.dto.DiscoveryUpstreamData;
import org.apache.shenyu.protocol.tcp.BootstrapServer;
+import org.apache.shenyu.protocol.tcp.TcpServerConfiguration;
import org.apache.shenyu.protocol.tcp.UpstreamProvider;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
@@ -79,7 +80,7 @@ public final class TcpUpstreamDataHandlerTest {
DiscoveryUpstreamData added = upstream("127.0.0.1:10003");
UpstreamProvider.getSingleton().createUpstreams(SELECTOR,
Arrays.asList(kept, removed));
BootstrapServer bootstrapServer = mock(BootstrapServer.class);
- factory.cache(SELECTOR, bootstrapServer);
+ factory.cache(configuration(SELECTOR), bootstrapServer);
dataHandler.handlerDiscoveryUpstreamData(
syncData(kept, added));
@@ -114,7 +115,7 @@ public final class TcpUpstreamDataHandlerTest {
public void removeShouldResolveSelectorNameByIdAndEvictUpstreams() {
DiscoveryUpstreamData upstream = upstream("127.0.0.1:10001");
BootstrapServer bootstrapServer = mock(BootstrapServer.class);
- factory.cache(SELECTOR, bootstrapServer);
+ factory.cache(configuration(SELECTOR), bootstrapServer);
dataHandler.handlerDiscoveryUpstreamData(syncData(upstream));
assertTrue(UpstreamProvider.getSingleton().inCache(SELECTOR));
assertEquals(SELECTOR,
UpstreamProvider.getSingleton().getSelectorName(SELECTOR_ID));
@@ -133,8 +134,8 @@ public final class TcpUpstreamDataHandlerTest {
DiscoveryUpstreamData upstream = upstream("127.0.0.1:10001");
BootstrapServer bootstrapServer = mock(BootstrapServer.class);
BootstrapServer otherServer = mock(BootstrapServer.class);
- factory.cache(SELECTOR, bootstrapServer);
- factory.cache("new-name", otherServer);
+ factory.cache(configuration(SELECTOR), bootstrapServer);
+ factory.cache(configuration("new-name"), otherServer);
UpstreamProvider.getSingleton().createUpstreams("new-name",
Collections.singletonList(upstream));
dataHandler.handlerDiscoveryUpstreamData(syncData(upstream));
@@ -150,7 +151,7 @@ public final class TcpUpstreamDataHandlerTest {
public void removeShouldFallBackToEventNameWithoutLocalMapping() {
DiscoveryUpstreamData upstream = upstream("127.0.0.1:10001");
BootstrapServer bootstrapServer = mock(BootstrapServer.class);
- factory.cache(SELECTOR, bootstrapServer);
+ factory.cache(configuration(SELECTOR), bootstrapServer);
UpstreamProvider.getSingleton().createUpstreams(SELECTOR,
Collections.singletonList(upstream));
dataHandler.removeDiscoveryUpstreamData(new
DiscoveryUpstreamKey("tcp", "unknown", SELECTOR));
@@ -163,7 +164,7 @@ public final class TcpUpstreamDataHandlerTest {
public void removeShouldIgnoreUnknownSelectorId() {
DiscoveryUpstreamData upstream = upstream("127.0.0.1:10001");
BootstrapServer bootstrapServer = mock(BootstrapServer.class);
- factory.cache(SELECTOR, bootstrapServer);
+ factory.cache(configuration(SELECTOR), bootstrapServer);
dataHandler.handlerDiscoveryUpstreamData(syncData(upstream));
clearInvocations(bootstrapServer);
DiscoveryUpstreamKey deleteData = new DiscoveryUpstreamKey("tcp",
"unknown", null);
@@ -180,6 +181,12 @@ public final class TcpUpstreamDataHandlerTest {
assertDoesNotThrow(() -> dataHandler.removeDiscoveryUpstreamData(new
DiscoveryUpstreamKey("tcp", null, null)));
}
+ private static TcpServerConfiguration configuration(final String
selectorName) {
+ TcpServerConfiguration configuration = new TcpServerConfiguration();
+ configuration.setPluginSelectorName(selectorName);
+ return configuration;
+ }
+
private DiscoverySyncData syncData(final DiscoveryUpstreamData...
upstreams) {
DiscoverySyncData discoverySyncData = new DiscoverySyncData();
discoverySyncData.setSelectorId(SELECTOR_ID);