mirror of
https://github.com/langgenius/dify.git
synced 2026-10-04 13:47:55 +08:00
fix(agent): auto-rename duplicate package imports (#42623)
This commit is contained in:
1 parent
99df0ca07f
commit
11038c5936
4 files changed
+59
-48
No files matched your search
@@ -241,7 +241,7 @@ class AgentDslService:
|
||||
tenant_id=app.tenant_id,
|
||||
account_id=account.id,
|
||||
app_id=app.id,
|
||||
name=self._unique_roster_name(tenant_id=app.tenant_id, requested=app.name or metadata.name),
|
||||
name=self.unique_roster_name(tenant_id=app.tenant_id, requested=app.name or metadata.name),
|
||||
description=app.description or metadata.description,
|
||||
role=metadata.role,
|
||||
icon_type=self._agent_icon_type(metadata.icon_type),
|
||||
@@ -765,7 +765,7 @@ class AgentDslService:
|
||||
self.session.flush()
|
||||
return snapshot
|
||||
|
||||
def _unique_roster_name(self, *, tenant_id: str, requested: str) -> str:
|
||||
def unique_roster_name(self, *, tenant_id: str, requested: str) -> str:
|
||||
candidates = [requested]
|
||||
for index in range(1, 100):
|
||||
suffix = " import" if index == 1 else f" import {index}"
|
||||
|
||||
@@ -16,14 +16,11 @@ from extensions.ext_storage import storage
|
||||
from libs.datetime_utils import naive_utc_now
|
||||
from models import Account
|
||||
from models.agent import (
|
||||
Agent,
|
||||
AgentConfigDraft,
|
||||
AgentConfigDraftType,
|
||||
AgentConfigRevisionOperation,
|
||||
AgentConfigSnapshot,
|
||||
AgentScope,
|
||||
AgentSource,
|
||||
AgentStatus,
|
||||
)
|
||||
from models.agent_config_entities import AgentSoulConfig
|
||||
from models.model import App, AppMode, AppModelConfig, UploadFile
|
||||
@@ -77,7 +74,6 @@ class RosterAgentPackageImporter:
|
||||
|
||||
check_package_dependencies(tenant_id=tenant_id, account=account, dependencies=app_dsl.dependencies)
|
||||
try:
|
||||
self._ensure_name_available(tenant_id=tenant_id, name=agent_package.metadata.name)
|
||||
materialized, skill_warnings = self._resources.materialize(
|
||||
archive=package,
|
||||
resources=package.manifest,
|
||||
@@ -124,22 +120,6 @@ class RosterAgentPackageImporter:
|
||||
)
|
||||
return RosterAgentPackageImportResult(app_id=app_id, agent_id=agent_id, warnings=warnings)
|
||||
|
||||
@staticmethod
|
||||
def _ensure_name_available(*, tenant_id: str, name: str) -> None:
|
||||
with session_factory.create_session() as session:
|
||||
exists = session.scalar(
|
||||
select(Agent.id)
|
||||
.where(
|
||||
Agent.tenant_id == tenant_id,
|
||||
Agent.scope == AgentScope.ROSTER,
|
||||
Agent.status == AgentStatus.ACTIVE,
|
||||
Agent.name == name,
|
||||
)
|
||||
.limit(1)
|
||||
)
|
||||
if exists is not None:
|
||||
raise AgentNameConflictError()
|
||||
|
||||
@staticmethod
|
||||
def _persist_import(
|
||||
*,
|
||||
@@ -149,10 +129,11 @@ class RosterAgentPackageImporter:
|
||||
soul: AgentSoulConfig,
|
||||
) -> tuple[str, str]:
|
||||
with session_factory.create_session() as session, session.begin():
|
||||
name = AgentDslService(session).unique_roster_name(tenant_id=tenant_id, requested=metadata.name)
|
||||
app = App(**default_app_templates[AppMode.AGENT]["app"])
|
||||
app.id = str(uuid4())
|
||||
app.tenant_id = tenant_id
|
||||
app.name = metadata.name
|
||||
app.name = name
|
||||
app.description = metadata.description
|
||||
app.icon_type = DEFAULT_ICON_TYPE
|
||||
app.icon = DEFAULT_ICON
|
||||
@@ -173,7 +154,7 @@ class RosterAgentPackageImporter:
|
||||
tenant_id=tenant_id,
|
||||
account_id=account.id,
|
||||
app_id=app.id,
|
||||
name=metadata.name,
|
||||
name=name,
|
||||
description=metadata.description,
|
||||
role=metadata.role,
|
||||
source=AgentSource.IMPORTED,
|
||||
|
||||
@@ -418,7 +418,7 @@ def test_import_agent_app_package_creates_config_and_unpublished_draft(
|
||||
soul = AgentSoulConfig(config_note="portable")
|
||||
warning = DslImportWarning(code="setup", path="agent.soul", message="setup required")
|
||||
service.resolve_package_soul = Mock(return_value=(soul, [warning]))
|
||||
service._unique_roster_name = Mock(return_value="Portable Agent import")
|
||||
service.unique_roster_name = Mock(return_value="Portable Agent import")
|
||||
agent = _agent()
|
||||
agent.active_config_snapshot_id = "snapshot-1"
|
||||
agent.active_config_is_published = True
|
||||
@@ -999,7 +999,7 @@ def test_unique_roster_name_uses_first_available_suffix(sqlite_session: Session)
|
||||
)
|
||||
sqlite_session.commit()
|
||||
|
||||
result = AgentDslService(sqlite_session)._unique_roster_name(tenant_id="tenant-1", requested="Agent")
|
||||
result = AgentDslService(sqlite_session).unique_roster_name(tenant_id="tenant-1", requested="Agent")
|
||||
|
||||
assert result == "Agent import 2"
|
||||
|
||||
|
||||
@@ -591,34 +591,64 @@ def test_import_validates_all_file_limits_before_staging(
|
||||
assert storage.files == {}
|
||||
|
||||
|
||||
def test_import_rejects_duplicate_agent_name_before_staging(
|
||||
@pytest.mark.parametrize(
|
||||
("requested", "existing_names", "expected"),
|
||||
[
|
||||
("Imported Agent", [], "Imported Agent"),
|
||||
("Imported Agent", ["Imported Agent"], "Imported Agent import"),
|
||||
(
|
||||
"Imported Agent",
|
||||
["Imported Agent", "Imported Agent import", "Imported Agent import 2"],
|
||||
"Imported Agent import 3",
|
||||
),
|
||||
(
|
||||
"Imported Agent",
|
||||
["Imported Agent", "Imported Agent import 2"],
|
||||
"Imported Agent import",
|
||||
),
|
||||
("A" * 255, ["A" * 255], "A" * 248 + " import"),
|
||||
],
|
||||
)
|
||||
def test_import_uses_available_name_for_app_and_agent(
|
||||
sqlite_session_factory: sessionmaker[Session],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
requested: str,
|
||||
existing_names: list[str],
|
||||
expected: str,
|
||||
) -> None:
|
||||
monkeypatch.setattr(AppService, "finalize_created_app", lambda *_args, **_kwargs: None)
|
||||
with sqlite_session_factory() as session, session.begin():
|
||||
session.add(
|
||||
Agent(
|
||||
tenant_id="tenant-1",
|
||||
name="Imported Agent",
|
||||
description="",
|
||||
role="",
|
||||
agent_kind=AgentKind.DIFY_AGENT,
|
||||
scope=AgentScope.ROSTER,
|
||||
source=AgentSource.AGENT_APP,
|
||||
status=AgentStatus.ACTIVE,
|
||||
created_by=_account().id,
|
||||
updated_by=_account().id,
|
||||
for name in existing_names:
|
||||
session.add(
|
||||
Agent(
|
||||
tenant_id="tenant-1",
|
||||
name=name,
|
||||
description="",
|
||||
role="",
|
||||
agent_kind=AgentKind.DIFY_AGENT,
|
||||
scope=AgentScope.ROSTER,
|
||||
source=AgentSource.AGENT_APP,
|
||||
status=AgentStatus.ACTIVE,
|
||||
created_by=_account().id,
|
||||
updated_by=_account().id,
|
||||
)
|
||||
)
|
||||
)
|
||||
storage = _MemoryStorage()
|
||||
|
||||
with pytest.raises(AgentNameConflictError):
|
||||
RosterAgentPackageImporter(storage_backend=storage).import_package(
|
||||
source=io.BytesIO(_package()),
|
||||
tenant_id="tenant-1",
|
||||
account=_account(),
|
||||
)
|
||||
result = RosterAgentPackageImporter(storage_backend=storage).import_package(
|
||||
source=io.BytesIO(_package(name=requested)),
|
||||
tenant_id="tenant-1",
|
||||
account=_account(),
|
||||
)
|
||||
|
||||
assert storage.files == {}
|
||||
with sqlite_session_factory() as session:
|
||||
app = session.get(App, result.app_id)
|
||||
agent = session.get(Agent, result.agent_id)
|
||||
assert app is not None
|
||||
assert agent is not None
|
||||
assert app.name == agent.name == expected
|
||||
assert _count(session, Agent) == len(existing_names) + 1
|
||||
assert len(storage.files) == 4
|
||||
|
||||
|
||||
@pytest.mark.parametrize("fail_after_write", [False, True])
|
||||
@@ -672,7 +702,7 @@ def test_import_preserves_error_mapping(
|
||||
def fail(**_kwargs) -> None:
|
||||
raise failure
|
||||
|
||||
monkeypatch.setattr(importer, "_ensure_name_available", fail)
|
||||
monkeypatch.setattr(importer._resources, "materialize", fail)
|
||||
with pytest.raises(expected) as caught:
|
||||
importer.import_package(source=io.BytesIO(_package()), tenant_id="tenant-1", account=_account())
|
||||
if isinstance(failure, expected):
|
||||
|
||||
Reference in new issue
Block a user