Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
4 changes: 3 additions & 1 deletion changes/en-us/2.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ Add changes here for all PR submitted to the 2.x branch.
- [[#7282](https://github.com/apache/incubator-seata/pull/7282)] optimize unexpected NullPointerException in lookup method in FileRegistryServiceImpl class
- [[#7310](https://github.com/seata/seata/pull/7310)] Optimize minor issues in the naming-server
- [[#7329](https://github.com/apache/incubator-seata/pull/7329)] upgrade tomcat to 9.0.100
- [[#7344](https://github.com/apache/incubator-seata/pull/7344)] raft mode performs transaction size check in advance
- [[#7337](https://github.com/apache/incubator-seata/pull/7337)] Add ChannelEventListener support to prevent memory leaks
- [[#7344](https://github.com/apache/incubator-seata/pull/7344)] raft mode performs transaction size check in advance
- [[#7345](https://github.com/apache/incubator-seata/pull/7345)] add empty check and duplicate type check to RegistryFactory


### security:
Expand Down Expand Up @@ -64,6 +65,7 @@ Add changes here for all PR submitted to the 2.x branch.
- [[#7294](https://github.com/apache/incubator-seata/pull/7294)] improved test testGetInsertParamsValue in SqlServerInsertRecognizerTest by splitting and parameterizing
- [[#7295](https://github.com/apache/incubator-seata/pull/7295)] updated 3 tests in StringUtilsTest to use parameterized unit testing
- [[#7205](https://github.com/apache/incubator-seata/issues/7205)] add UT for namingserver module
-
### refactor:

- [[#7315](https://github.com/apache/incubator-seata/pull/7315)] Refactor log testing to use ListAppender for more accurate and efficient log capture
Expand Down
3 changes: 2 additions & 1 deletion changes/zh-cn/2.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@
- [[#7282](https://github.com/apache/incubator-seata/pull/7282)] 优化FileRegistryServiceImpl类lookup的NullPointerException问题
- [[#7310](https://github.com/seata/seata/pull/7310)] 优化naming-server中的一些小问题
- [[#7329](https://github.com/apache/incubator-seata/pull/7329)] 将 tomcat 升级到 9.0.100
- [[#7344](https://github.com/apache/incubator-seata/pull/7344)] raft模式提前检查事务大小
- [[#7337](https://github.com/apache/incubator-seata/pull/7337)] 添加 ChannelEventListener 支持以防止内存泄漏
- [[#7344](https://github.com/apache/incubator-seata/pull/7344)] raft模式提前检查事务大小
- [[#7345](https://github.com/apache/incubator-seata/pull/7345)] 为 RegistryFactory 增加空校验与重复类型检查


### security:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
package org.apache.seata.discovery.registry;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;

import org.apache.seata.common.ConfigurationKeys;
import org.apache.seata.common.Constants;
Expand All @@ -31,7 +33,6 @@

/**
* The type multiple Registry factory.
*
*/
public class MultiRegistryFactory {

Expand All @@ -48,26 +49,36 @@ public static List<RegistryService> getInstances() {

private static List<RegistryService> buildRegistryServices() {
List<RegistryService> registryServices = new ArrayList<>();
String registryTypeNamesStr =
ConfigurationFactory.CURRENT_FILE_INSTANCE.getConfig(ConfigurationKeys.FILE_ROOT_REGISTRY
+ ConfigurationKeys.FILE_CONFIG_SPLIT_CHAR + ConfigurationKeys.FILE_ROOT_TYPE);
Set<String> processedRegistryTypes = new HashSet<>();
String registryTypeNamesStr = ConfigurationFactory.CURRENT_FILE_INSTANCE.getConfig(
ConfigurationKeys.FILE_ROOT_REGISTRY + ConfigurationKeys.FILE_CONFIG_SPLIT_CHAR + ConfigurationKeys.FILE_ROOT_TYPE);

if (StringUtils.isBlank(registryTypeNamesStr)) {
registryTypeNamesStr = RegistryType.File.name();
}
String[] registryTypeNames = registryTypeNamesStr.split(Constants.REGISTRY_TYPE_SPLIT_CHAR);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not convert this directly to a TreeSet? This would allow for case-insensitivity and also handle deduplication

Copy link
Member

Choose a reason for hiding this comment

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

+1
Also, it would be great to add a test case that covers this scenario.


if (registryTypeNames.length > 1) {
LOGGER.info("use multi registry center type: {}", registryTypeNamesStr);
}

for (String registryTypeName : registryTypeNames) {
RegistryType registryType;
try {
registryType = RegistryType.getType(registryTypeName);
} catch (Exception exx) {
throw new NotSupportYetException("not support registry type: " + registryTypeName);
}

if (processedRegistryTypes.contains(registryType.name())) {
LOGGER.warn("The duplicate registration center type '{}' was found in the configuration and has been skipped.", registryType.name());
continue;
Copy link
Member

Choose a reason for hiding this comment

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

I saw a test that verifies a method was called once by checking if instances.size() equals 1.
How about also adding a test to verify that the corresponding log message is properly output?

What do you think?

Copy link
Contributor Author

@YoWuwuuuw YoWuwuuuw May 20, 2025

Choose a reason for hiding this comment

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

Ok, learned and used the method you wrote before in #7315 (it's awesome)

}

RegistryService registryService = EnhancedServiceLoader
.load(RegistryProvider.class, Objects.requireNonNull(registryType).name()).provide();
.load(RegistryProvider.class, Objects.requireNonNull(registryType).name()).provide();
registryServices.add(registryService);
processedRegistryTypes.add(registryType.name());
}
return registryServices;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@

import org.apache.seata.common.exception.NotSupportYetException;
import org.apache.seata.common.loader.EnhancedServiceLoader;
import org.apache.seata.common.util.StringUtils;
import org.apache.seata.config.ConfigurationFactory;
import org.apache.seata.config.ConfigurationKeys;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* The type Registry factory.
*
*/
public class RegistryFactory {

Expand All @@ -45,8 +45,12 @@ public static RegistryService getInstance() {
private static RegistryService buildRegistryService() {
RegistryType registryType;
String registryTypeName = ConfigurationFactory.CURRENT_FILE_INSTANCE.getConfig(
ConfigurationKeys.FILE_ROOT_REGISTRY + ConfigurationKeys.FILE_CONFIG_SPLIT_CHAR
+ ConfigurationKeys.FILE_ROOT_TYPE);
ConfigurationKeys.FILE_ROOT_REGISTRY + ConfigurationKeys.FILE_CONFIG_SPLIT_CHAR + ConfigurationKeys.FILE_ROOT_TYPE);

if (StringUtils.isBlank(registryTypeName)) {
registryTypeName = RegistryType.File.name();
}

LOGGER.info("use registry center type: {}", registryTypeName);
try {
Copy link
Member

Choose a reason for hiding this comment

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

This try-catch block could be removed, as it simply catches the exception and rethrows it without adding any additional handling.

registryType = RegistryType.getType(registryTypeName);
Expand Down
Copy link
Member

@YongGoose YongGoose May 23, 2025

Choose a reason for hiding this comment

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

Since it’s been changed to a TreeSet, it might be good to add a few more test cases

  1. When the same RegistryType is provided with different casing (e.g., File, FILE), does it recognize them as the same?

    • This could be verified by checking that no log is printed.
  2. When more than one RegistryType is provided, does the log output as expected?

  3. When a value not defined in RegistryType is passed, is the appropriate exception thrown? (After removing the try-catch block, this test should be added.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* 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.seata.discovery.registry;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.List;

import org.apache.seata.common.ConfigurationKeys;
import org.apache.seata.common.Constants;
import org.apache.seata.common.exception.NotSupportYetException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

/**
* The type Multi registry factory test.
*/
public class MultiRegistryFactoryTest {

private static final String REGISTRY_TYPE_KEY =
ConfigurationKeys.FILE_ROOT_REGISTRY + ConfigurationKeys.FILE_CONFIG_SPLIT_CHAR + ConfigurationKeys.FILE_ROOT_TYPE;

@AfterEach
public void tearDown() {
System.clearProperty(REGISTRY_TYPE_KEY);
}

/**
* Test getInstances with default config.
*/
@Test
public void testGetInstancesWithDefaultConfig() {
// Set "registry.type = file" as default config
System.setProperty(REGISTRY_TYPE_KEY, RegistryType.File.name());

List<RegistryService> instances = MultiRegistryFactory.getInstances();
Assertions.assertNotNull(instances);

for (RegistryService service : instances) {
Assertions.assertNotNull(service);
}
}

/**
* Test buildRegistryServices with multiple tests of the same registry type.
*/
@Test
public void testGetInstancesWithMultiRegistryTypes() throws Throwable {
// Set up multiple registration center configurations
System.setProperty(REGISTRY_TYPE_KEY, RegistryType.File.name() + Constants.REGISTRY_TYPE_SPLIT_CHAR + RegistryType.File.name());

List<RegistryService> instances = invokeBuildRegistryServices();
Assertions.assertNotNull(instances);
Assertions.assertEquals(1, instances.size());
}

/**
* Test buildRegistryServices with blank registry type.
*/
@Test
public void testGetInstancesWithBlankRegistryType() throws Throwable {
System.setProperty(REGISTRY_TYPE_KEY, "");

List<RegistryService> instances = invokeBuildRegistryServices();
Assertions.assertNotNull(instances);
}

/**
* Test buildRegistryServices with invalid registry type.
*/
@Test
public void testGetInstancesWithInvalidRegistryType() {
System.setProperty(REGISTRY_TYPE_KEY, "InvalidRegistryType");

Assertions.assertThrows(NotSupportYetException.class, MultiRegistryFactoryTest::invokeBuildRegistryServices);
}

/**
* Use reflection to call the buildRegistryServices method
*/
private static List<RegistryService> invokeBuildRegistryServices() throws Throwable {
Method buildMethod = MultiRegistryFactory.class.getDeclaredMethod("buildRegistryServices");
buildMethod.setAccessible(true);

try {
return (List<RegistryService>) buildMethod.invoke(null);
} catch (InvocationTargetException e) {
throw e.getTargetException();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* 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.seata.discovery.registry;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;

import org.apache.seata.common.ConfigurationKeys;
import org.apache.seata.common.exception.NotSupportYetException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

/**
* The type Registry factory test.
*/
public class RegistryFactoryTest {

private static final String REGISTRY_TYPE_KEY =
ConfigurationKeys.FILE_ROOT_REGISTRY + ConfigurationKeys.FILE_CONFIG_SPLIT_CHAR + ConfigurationKeys.FILE_ROOT_TYPE;

@AfterEach
public void tearDown() {
System.clearProperty(REGISTRY_TYPE_KEY);
}

/**
* Test getInstance with default config.
*/
@Test
public void testGetInstanceWithDefaultConfig() {
System.setProperty(REGISTRY_TYPE_KEY, RegistryType.File.name());

RegistryService instance = RegistryFactory.getInstance();
Assertions.assertNotNull(instance);
}

/**
* Test buildRegistryService with invalid registry type.
*/
@Test
public void testGetInstanceOfInvalidRegistryType() {
System.setProperty(REGISTRY_TYPE_KEY, "InvalidRegistryType");

Assertions.assertThrows(NotSupportYetException.class, RegistryFactoryTest::invokeBuildRegistryService);
}

/**
* Test buildRegistryService with blank registry type.
*/
@Test
public void testGetInstancesWithBlankRegistryType() throws Throwable {
System.setProperty(REGISTRY_TYPE_KEY, "");

RegistryService instance = invokeBuildRegistryService();
Assertions.assertEquals(instance.getClass(), FileRegistryServiceImpl.class);
}

/**
* Use reflection to call the buildRegistryServices method
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the comment should refer to buildRegistryService instead of buildRegistryServices.

*/
private static RegistryService invokeBuildRegistryService() throws Throwable {
Method buildMethod = RegistryFactory.class.getDeclaredMethod("buildRegistryService");

buildMethod.setAccessible(true);
try {
return (RegistryService) buildMethod.invoke(null);
} catch (InvocationTargetException e) {
throw e.getTargetException();
}
}
}
Loading