Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.util.Map;
import java.util.Properties;
import java.util.ServiceLoader;
import java.util.Set;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -105,7 +104,8 @@ void initWithBuilders(Iterable<PublishPipelineServiceBuilder> builders, Pipeline
* Get pipeline services matching the given resource type and node configuration list.
*
* <p>Filters services that support the specified resource type and whose pipelineId is present
* in the nodes list. Results are sorted by {@link PublishPipelineService#getPreferOrder()} ascending.</p>
* in the nodes list. Results are sorted by configured node order first, then
* {@link PublishPipelineService#getPreferOrder()} ascending.</p>
*
* @param resourceType the resource type to filter by
* @param nodes the configured pipeline nodes to match against
Expand All @@ -114,14 +114,17 @@ void initWithBuilders(Iterable<PublishPipelineServiceBuilder> builders, Pipeline
public List<PublishPipelineService> getPipelineServices(
PublishPipelineResourceType resourceType,
List<PipelineNodeConfig> nodes) {
Set<String> pipelineIds = nodes.stream()
.map(PipelineNodeConfig::getPipelineId)
.collect(Collectors.toSet());
Map<String, PipelineNodeConfig> nodeConfigMap = new HashMap<>();
for (PipelineNodeConfig node : nodes) {
if (node.getPipelineId() != null) {
nodeConfigMap.put(node.getPipelineId(), node);
}
}

return serviceMap.values().stream()
.filter(service -> pipelineIds.contains(service.pipelineId()))
.filter(service -> nodeConfigMap.containsKey(service.pipelineId()))
.filter(service -> supportsResourceType(service, resourceType))
.sorted(Comparator.comparingInt(PublishPipelineService::getPreferOrder))
.sorted(Comparator.comparingInt(service -> getEffectiveOrder(service, nodeConfigMap)))
.collect(Collectors.toList());
}

Expand All @@ -142,4 +145,13 @@ private boolean supportsResourceType(PublishPipelineService service,
}
return Arrays.asList(types).contains(resourceType);
}

private int getEffectiveOrder(PublishPipelineService service,
Map<String, PipelineNodeConfig> nodeConfigMap) {
PipelineNodeConfig nodeConfig = nodeConfigMap.get(service.pipelineId());
if (nodeConfig != null && nodeConfig.getOrder() != null) {
return nodeConfig.getOrder();
}
return service.getPreferOrder();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,12 @@
import com.alibaba.nacos.sys.env.EnvUtil;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.TreeMap;

/**
* File-based (application.properties) implementation of {@link PipelineConfigProvider}.
Expand All @@ -39,11 +37,12 @@
* <ul>
* <li>{@code nacos.plugin.ai-pipeline.enabled} - optional global switch for ai-pipeline plugin</li>
* <li>{@code nacos.plugin.ai-pipeline.type} - enabled implementation type(s), for example
* {@code skill-scanner}</li>
* {@code skill-scanner}; default is {@code skill-spector,skill-scanner}</li>
* <li>{@code nacos.plugin.ai-pipeline.{type}.order} - optional execution order override;
* lower values execute first</li>
* <li>{@code nacos.plugin.ai-pipeline.{type}.{key}} - implementation properties passed to builder</li>
* </ul>
* <p>For example: {@code nacos.plugin.ai-pipeline.type=skill-scanner} and
* {@code nacos.plugin.ai-pipeline.skill-scanner.enabled=true}.
* <p>For example: {@code nacos.plugin.ai-pipeline.type=skill-scanner}.
*
* <p>Follows the singleton pattern like PushConfig.
*
Expand All @@ -60,6 +59,11 @@ public class FilePipelineConfigProvider extends AbstractDynamicConfig
private static final String KEY_ENABLED = KEY_PLUGIN_PREFIX + ".enabled";

private static final String KEY_TYPE = KEY_PLUGIN_PREFIX + ".type";

private static final String KEY_ORDER = "order";

private static final List<String> DEFAULT_TYPES = Collections.unmodifiableList(
Arrays.asList("skill-spector", "skill-scanner"));

private static final FilePipelineConfigProvider INSTANCE = new FilePipelineConfigProvider();

Expand Down Expand Up @@ -116,19 +120,15 @@ private List<PipelineNodeConfig> readEnabledPluginNodes() {
return Collections.emptyList();
}

Map<String, Properties> pluginPropertyMap = new TreeMap<>();
List<PipelineNodeConfig> nodes = new ArrayList<>(configuredTypes.size());
for (String typeName : configuredTypes) {
pluginPropertyMap.put(typeName, readNodeProperties(typeName, allProperties));
Properties nodeProperties = readNodeProperties(typeName, allProperties);
PipelineNodeConfig nodeConfig = new PipelineNodeConfig();
nodeConfig.setPipelineId(typeName);
nodeConfig.setProperties(nodeProperties);
nodeConfig.setOrder(parseOrder(nodeProperties.getProperty(KEY_ORDER)));
nodes.add(nodeConfig);
}
List<PipelineNodeConfig> nodes = new ArrayList<>(pluginPropertyMap.size());
pluginPropertyMap.entrySet().stream()
.sorted(Map.Entry.comparingByKey(Comparator.naturalOrder()))
.forEach(entry -> {
PipelineNodeConfig nodeConfig = new PipelineNodeConfig();
nodeConfig.setPipelineId(entry.getKey());
nodeConfig.setProperties(entry.getValue());
nodes.add(nodeConfig);
});
return nodes;
}

Expand All @@ -143,7 +143,7 @@ private boolean isPluginEnabled(Properties allProperties) {
private List<String> readConfiguredTypes(Properties allProperties) {
String typeValue = allProperties.getProperty(KEY_TYPE);
if (StringUtils.isBlank(typeValue)) {
return Collections.emptyList();
return DEFAULT_TYPES;
}
Set<String> types = new LinkedHashSet<>();
for (String each : typeValue.split(",")) {
Expand All @@ -169,6 +169,17 @@ private Properties readNodeProperties(String typeName, Properties allProperties)
}
return properties;
}

private Integer parseOrder(String value) {
if (StringUtils.isBlank(value)) {
return null;
}
try {
return Integer.parseInt(value.trim());
} catch (NumberFormatException e) {
return null;
}
}

@Override
protected String printConfig() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public class PipelineNodeConfig {
* Custom configuration properties for this node (e.g. endpoint, timeout).
*/
private Properties properties;

/**
* Optional configured execution order. Lower values execute first.
*/
private Integer order;

public PipelineNodeConfig() {
}
Expand All @@ -54,4 +59,12 @@ public Properties getProperties() {
public void setProperties(Properties properties) {
this.properties = properties;
}

public Integer getOrder() {
return order;
}

public void setOrder(Integer order) {
this.order = order;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.Set;
import java.util.stream.Collectors;

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.assertTrue;
Expand Down Expand Up @@ -205,6 +206,32 @@ void getPipelineServicesFilteringAndSorting() {
}
}
}

@Test
void configuredNodeOrderShouldOverrideServicePreferOrder() {
PublishPipelineManager manager = new PublishPipelineManager();
List<PublishPipelineServiceBuilder> builders = new ArrayList<>();
builders.add(createServiceBuilder(new ServiceDescriptor("first-by-default", 10,
PublishPipelineResourceType.values())));
builders.add(createServiceBuilder(new ServiceDescriptor("first-by-config", 20,
PublishPipelineResourceType.values())));

PipelineConfig config = new PipelineConfig();
config.setEnabled(true);
config.setNodes(new ArrayList<>());
manager.initWithBuilders(builders, config);

List<PipelineNodeConfig> nodes = new ArrayList<>();
nodes.add(createNode("first-by-default", 30));
nodes.add(createNode("first-by-config", 5));

List<PublishPipelineService> result =
manager.getPipelineServices(PublishPipelineResourceType.SKILL, nodes);

assertEquals(2, result.size());
assertEquals("first-by-config", result.get(0).pipelineId());
assertEquals("first-by-default", result.get(1).pipelineId());
}

private PublishPipelineServiceBuilder createMockBuilder(BuilderDescriptor desc) {
return new PublishPipelineServiceBuilder() {
Expand Down Expand Up @@ -245,6 +272,14 @@ public PublishPipelineResourceType[] pipelineResourceTypes() {
}
};
}

private PipelineNodeConfig createNode(String pipelineId, Integer order) {
PipelineNodeConfig nodeConfig = new PipelineNodeConfig();
nodeConfig.setPipelineId(pipelineId);
nodeConfig.setProperties(new Properties());
nodeConfig.setOrder(order);
return nodeConfig;
}

static class BuilderDescriptor {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Properties;
import java.util.stream.Collectors;

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.assertTrue;

/**
Expand All @@ -50,7 +50,6 @@ class FilePipelineConfigProviderTest {

private static List<List<NodeTestData>> sampleValidNodeConfigs() {
List<List<NodeTestData>> lists = new ArrayList<>();
lists.add(Collections.emptyList());
Properties p1 = new Properties();
p1.setProperty("k1", "v1");
lists.add(Collections.singletonList(new NodeTestData("scanner", "t", p1)));
Expand All @@ -62,6 +61,42 @@ private static List<List<NodeTestData>> sampleValidNodeConfigs() {
new NodeTestData("beta", "y", new Properties())));
return lists;
}

@Test
void defaultTypesShouldBeEnabledWhenTypeUnset() {
Properties envProperties = new Properties();
envProperties.setProperty("nacos.plugin.ai-pipeline.enabled", "true");

try (MockedStatic<EnvUtil> envMock = Mockito.mockStatic(EnvUtil.class);
MockedStatic<NotifyCenter> notifyMock = Mockito.mockStatic(NotifyCenter.class)) {
envMock.when(EnvUtil::getProperties).thenReturn(envProperties);

FilePipelineConfigProvider provider = createFreshInstance();
PipelineConfig config = provider.getConfig();

assertTrue(config.isEnabled());
assertEquals(2, config.getNodes().size());
assertNotNull(findNode(config, "skill-spector"));
assertNotNull(findNode(config, "skill-scanner"));
}
}

@Test
void explicitDisableShouldTurnOffDefaultTypes() {
Properties envProperties = new Properties();
envProperties.setProperty("nacos.plugin.ai-pipeline.enabled", "false");

try (MockedStatic<EnvUtil> envMock = Mockito.mockStatic(EnvUtil.class);
MockedStatic<NotifyCenter> notifyMock = Mockito.mockStatic(NotifyCenter.class)) {
envMock.when(EnvUtil::getProperties).thenReturn(envProperties);

FilePipelineConfigProvider provider = createFreshInstance();
PipelineConfig config = provider.getConfig();

assertFalse(config.isEnabled());
assertTrue(config.getNodes().isEmpty());
}
}

private static List<ErrorScenario> sampleErrorScenarios() {
return Arrays.asList(
Expand Down Expand Up @@ -101,8 +136,6 @@ void configParsingCorrectness() {

FilePipelineConfigProvider provider = createFreshInstance();
PipelineConfig config = provider.getConfig();
List<NodeTestData> expectedNodes = new ArrayList<>(nodeConfigs);
expectedNodes.sort(Comparator.comparing(node -> node.pipelineId));

assertNotNull(config, "Parsed config should not be null");
assertEquals(enabled && !nodeConfigs.isEmpty(), config.isEnabled(),
Expand All @@ -112,7 +145,7 @@ void configParsingCorrectness() {
"Number of parsed nodes should match input");

for (int i = 0; i < config.getNodes().size(); i++) {
NodeTestData expected = expectedNodes.get(i);
NodeTestData expected = nodeConfigs.get(i);
PipelineNodeConfig actual = config.getNodes().get(i);
assertEquals(expected.pipelineId, actual.getPipelineId(),
"Node pipelineId at index " + i + " should match");
Expand Down Expand Up @@ -163,11 +196,10 @@ void configErrorTolerance() {
}

@Test
void subPropertyEnabledShouldNotBeTreatedAsNodeSwitch() {
void subPropertyCommandShouldBePreserved() {
Properties envProperties = new Properties();
envProperties.setProperty("nacos.plugin.ai-pipeline.enabled", "true");
envProperties.setProperty("nacos.plugin.ai-pipeline.type", "skill-scanner");
envProperties.setProperty("nacos.plugin.ai-pipeline.skill-scanner.enabled", "true");
envProperties.setProperty("nacos.plugin.ai-pipeline.skill-scanner.command",
"/tmp/skill-scanner");
try (MockedStatic<EnvUtil> envMock = Mockito.mockStatic(EnvUtil.class);
Expand All @@ -183,12 +215,34 @@ void subPropertyEnabledShouldNotBeTreatedAsNodeSwitch() {
PipelineNodeConfig node = config.getNodes().get(0);
assertEquals("skill-scanner", node.getPipelineId(),
"Node id should match configured type");
assertEquals("true", node.getProperties().getProperty("enabled"),
"Type sub-property enabled should be preserved as node property");
assertEquals("/tmp/skill-scanner", node.getProperties().getProperty("command"),
"Type sub-property command should be preserved");
}
}

@Test
void nodeOrderShouldBeParsedFromSubProperty() {
Properties envProperties = new Properties();
envProperties.setProperty("nacos.plugin.ai-pipeline.enabled", "true");
envProperties.setProperty("nacos.plugin.ai-pipeline.type",
"skill-spector,skill-scanner,broken");
envProperties.setProperty("nacos.plugin.ai-pipeline.skill-spector.order", "20");
envProperties.setProperty("nacos.plugin.ai-pipeline.skill-scanner.order", "-10");
envProperties.setProperty("nacos.plugin.ai-pipeline.broken.order", "invalid");
try (MockedStatic<EnvUtil> envMock = Mockito.mockStatic(EnvUtil.class);
MockedStatic<NotifyCenter> notifyMock = Mockito.mockStatic(NotifyCenter.class)) {
envMock.when(EnvUtil::getProperties).thenReturn(envProperties);

FilePipelineConfigProvider provider = createFreshInstance();
PipelineConfig config = provider.getConfig();

assertEquals(20, findNode(config, "skill-spector").getOrder());
assertEquals(-10, findNode(config, "skill-scanner").getOrder());
assertNull(findNode(config, "broken").getOrder());
assertEquals("20", findNode(config, "skill-spector").getProperties()
.getProperty("order"));
}
}

private FilePipelineConfigProvider createFreshInstance() {
try {
Expand All @@ -201,6 +255,13 @@ private FilePipelineConfigProvider createFreshInstance() {
"Failed to create FilePipelineConfigProvider instance via reflection", e);
}
}

private PipelineNodeConfig findNode(PipelineConfig config, String pipelineId) {
return config.getNodes().stream()
.filter(node -> pipelineId.equals(node.getPipelineId()))
.findFirst()
.orElseThrow(() -> new AssertionError("Missing node: " + pipelineId));
}

static class ErrorScenario {

Expand Down
Loading
Loading