Compare commits

...

4 Commits

Author SHA1 Message Date
twwu
337ad75114 refactor: remove document download functionality and related translations 2025-08-22 21:31:11 +08:00
-LAN-
9704319e10 fix: use automatic transaction commit in provider update handler
The provider update handler was missing proper transaction handling,
causing quota deductions to be lost. This fix uses session.begin()
context manager for automatic commit/rollback, ensuring provider
quota updates are properly persisted.

Fixes #24356
2025-08-22 21:24:55 +08:00
Harry
7f7156b325 refactor: improve session management in ToolManager 2025-08-15 13:09:54 +08:00
Harry
715a7fc19f fix: update session management in BuiltinToolManageService to use no_autoflush 2025-08-15 11:24:58 +08:00
6 changed files with 49 additions and 94 deletions

View File

@@ -9,6 +9,7 @@ from typing import TYPE_CHECKING, Any, Literal, Optional, Union, cast
import sqlalchemy as sa import sqlalchemy as sa
from pydantic import TypeAdapter from pydantic import TypeAdapter
from sqlalchemy.orm import Session
from yarl import URL from yarl import URL
import contexts import contexts
@@ -617,8 +618,9 @@ class ToolManager:
WHERE tenant_id = :tenant_id WHERE tenant_id = :tenant_id
ORDER BY tenant_id, provider, is_default DESC, created_at DESC ORDER BY tenant_id, provider, is_default DESC, created_at DESC
""" """
ids = [row.id for row in db.session.execute(sa.text(sql), {"tenant_id": tenant_id}).all()] with Session(db.engine).no_autoflush as session:
return db.session.query(BuiltinToolProvider).where(BuiltinToolProvider.id.in_(ids)).all() ids = [row.id for row in session.execute(sa.text(sql), {"tenant_id": tenant_id}).all()]
return session.query(BuiltinToolProvider).where(BuiltinToolProvider.id.in_(ids)).all()
@classmethod @classmethod
def list_providers_from_api( def list_providers_from_api(

View File

@@ -188,7 +188,7 @@ def _execute_provider_updates(updates_to_perform: list[_ProviderUpdateOperation]
# Use SQLAlchemy's context manager for transaction management # Use SQLAlchemy's context manager for transaction management
# This automatically handles commit/rollback # This automatically handles commit/rollback
with Session(db.engine) as session: with Session(db.engine) as session, session.begin():
# Use a single transaction for all updates # Use a single transaction for all updates
for update_operation in updates_to_perform: for update_operation in updates_to_perform:
filters = update_operation.filters filters = update_operation.filters

View File

@@ -546,54 +546,53 @@ class BuiltinToolManageService:
# get all builtin providers # get all builtin providers
provider_controllers = ToolManager.list_builtin_providers(tenant_id) provider_controllers = ToolManager.list_builtin_providers(tenant_id)
with db.session.no_autoflush: # get all user added providers
# get all user added providers db_providers: list[BuiltinToolProvider] = ToolManager.list_default_builtin_providers(tenant_id)
db_providers: list[BuiltinToolProvider] = ToolManager.list_default_builtin_providers(tenant_id)
# rewrite db_providers # rewrite db_providers
for db_provider in db_providers: for db_provider in db_providers:
db_provider.provider = str(ToolProviderID(db_provider.provider)) db_provider.provider = str(ToolProviderID(db_provider.provider))
# find provider # find provider
def find_provider(provider): def find_provider(provider):
return next(filter(lambda db_provider: db_provider.provider == provider, db_providers), None) return next(filter(lambda db_provider: db_provider.provider == provider, db_providers), None)
result: list[ToolProviderApiEntity] = [] result: list[ToolProviderApiEntity] = []
for provider_controller in provider_controllers: for provider_controller in provider_controllers:
try: try:
# handle include, exclude # handle include, exclude
if is_filtered( if is_filtered(
include_set=dify_config.POSITION_TOOL_INCLUDES_SET, # type: ignore include_set=dify_config.POSITION_TOOL_INCLUDES_SET, # type: ignore
exclude_set=dify_config.POSITION_TOOL_EXCLUDES_SET, # type: ignore exclude_set=dify_config.POSITION_TOOL_EXCLUDES_SET, # type: ignore
data=provider_controller, data=provider_controller,
name_func=lambda x: x.identity.name, name_func=lambda x: x.identity.name,
): ):
continue continue
# convert provider controller to user provider # convert provider controller to user provider
user_builtin_provider = ToolTransformService.builtin_provider_to_user_provider( user_builtin_provider = ToolTransformService.builtin_provider_to_user_provider(
provider_controller=provider_controller, provider_controller=provider_controller,
db_provider=find_provider(provider_controller.entity.identity.name), db_provider=find_provider(provider_controller.entity.identity.name),
decrypt_credentials=True, decrypt_credentials=True,
)
# add icon
ToolTransformService.repack_provider(tenant_id=tenant_id, provider=user_builtin_provider)
tools = provider_controller.get_tools()
for tool in tools or []:
user_builtin_provider.tools.append(
ToolTransformService.convert_tool_entity_to_api_entity(
tenant_id=tenant_id,
tool=tool,
labels=ToolLabelManager.get_tool_labels(provider_controller),
)
) )
# add icon result.append(user_builtin_provider)
ToolTransformService.repack_provider(tenant_id=tenant_id, provider=user_builtin_provider) except Exception as e:
raise e
tools = provider_controller.get_tools()
for tool in tools or []:
user_builtin_provider.tools.append(
ToolTransformService.convert_tool_entity_to_api_entity(
tenant_id=tenant_id,
tool=tool,
labels=ToolLabelManager.get_tool_labels(provider_controller),
)
)
result.append(user_builtin_provider)
except Exception as e:
raise e
return BuiltinToolProviderSort.sort(result) return BuiltinToolProviderSort.sort(result)
@@ -604,7 +603,7 @@ class BuiltinToolManageService:
1.if the default provider exists, return the default provider 1.if the default provider exists, return the default provider
2.if the default provider does not exist, return the oldest provider 2.if the default provider does not exist, return the oldest provider
""" """
with Session(db.engine) as session: with Session(db.engine).no_autoflush as session:
try: try:
full_provider_name = provider_name full_provider_name = provider_name
provider_id_entity = ToolProviderID(provider_name) provider_id_entity = ToolProviderID(provider_name)

View File

@@ -7,7 +7,6 @@ import { pick, uniq } from 'lodash-es'
import { import {
RiArchive2Line, RiArchive2Line,
RiDeleteBinLine, RiDeleteBinLine,
RiDownloadLine,
RiEditLine, RiEditLine,
RiEqualizer2Line, RiEqualizer2Line,
RiLoopLeftLine, RiLoopLeftLine,
@@ -35,7 +34,6 @@ import type { ColorMap, IndicatorProps } from '@/app/components/header/indicator
import Indicator from '@/app/components/header/indicator' import Indicator from '@/app/components/header/indicator'
import { asyncRunSafe } from '@/utils' import { asyncRunSafe } from '@/utils'
import { formatNumber } from '@/utils/format' import { formatNumber } from '@/utils/format'
import { useDocumentDownload } from '@/service/knowledge/use-document'
import NotionIcon from '@/app/components/base/notion-icon' import NotionIcon from '@/app/components/base/notion-icon'
import ProgressBar from '@/app/components/base/progress-bar' import ProgressBar from '@/app/components/base/progress-bar'
import { ChunkingMode, DataSourceType, DocumentActionType, type DocumentDisplayStatus, type SimpleDocumentDetail } from '@/models/datasets' import { ChunkingMode, DataSourceType, DocumentActionType, type DocumentDisplayStatus, type SimpleDocumentDetail } from '@/models/datasets'
@@ -189,7 +187,6 @@ export const OperationAction: FC<{
scene?: 'list' | 'detail' scene?: 'list' | 'detail'
className?: string className?: string
}> = ({ embeddingAvailable, datasetId, detail, onUpdate, scene = 'list', className = '' }) => { }> = ({ embeddingAvailable, datasetId, detail, onUpdate, scene = 'list', className = '' }) => {
const downloadDocument = useDocumentDownload()
const { id, enabled = false, archived = false, data_source_type, display_status } = detail || {} const { id, enabled = false, archived = false, data_source_type, display_status } = detail || {}
const [showModal, setShowModal] = useState(false) const [showModal, setShowModal] = useState(false)
const [deleting, setDeleting] = useState(false) const [deleting, setDeleting] = useState(false)
@@ -298,31 +295,6 @@ export const OperationAction: FC<{
)} )}
{embeddingAvailable && ( {embeddingAvailable && (
<> <>
<Tooltip
popupContent={t('datasetDocuments.list.action.download')}
popupClassName='text-text-secondary system-xs-medium'
>
<button
className={cn('mr-2 cursor-pointer rounded-lg',
!isListScene
? 'shadow-shadow-3 border-[0.5px] border-components-button-secondary-border bg-components-button-secondary-bg p-2 shadow-xs backdrop-blur-[5px] hover:border-components-button-secondary-border-hover hover:bg-components-button-secondary-bg-hover'
: 'p-0.5 hover:bg-state-base-hover')}
onClick={() => {
downloadDocument.mutateAsync({
datasetId,
documentId: detail.id,
}).then((response) => {
if (response.download_url)
window.location.href = response.download_url
}).catch((error) => {
console.error(error)
notify({ type: 'error', message: t('common.actionMsg.downloadFailed') })
})
}}
>
<RiDownloadLine className='h-4 w-4 text-components-button-secondary-text' />
</button>
</Tooltip>
<Tooltip <Tooltip
popupContent={t('datasetDocuments.list.action.settings')} popupContent={t('datasetDocuments.list.action.settings')}
popupClassName='text-text-secondary system-xs-medium' popupClassName='text-text-secondary system-xs-medium'
@@ -526,7 +498,7 @@ const DocumentList: FC<IDocumentListProps> = ({
const result = aValue.localeCompare(bValue) const result = aValue.localeCompare(bValue)
return sortOrder === 'asc' ? result : -result return sortOrder === 'asc' ? result : -result
} }
else { else {
const result = aValue - bValue const result = aValue - bValue
return sortOrder === 'asc' ? result : -result return sortOrder === 'asc' ? result : -result
} }
@@ -539,7 +511,7 @@ const DocumentList: FC<IDocumentListProps> = ({
if (sortField === field) { if (sortField === field) {
setSortOrder(sortOrder === 'asc' ? 'desc' : 'asc') setSortOrder(sortOrder === 'asc' ? 'desc' : 'asc')
} }
else { else {
setSortField(field) setSortField(field)
setSortOrder('desc') setSortOrder('desc')
} }

View File

@@ -32,7 +32,6 @@ const translation = {
sync: 'Sync', sync: 'Sync',
pause: 'Pause', pause: 'Pause',
resume: 'Resume', resume: 'Resume',
download: 'Download File',
}, },
index: { index: {
enable: 'Enable', enable: 'Enable',

View File

@@ -8,9 +8,7 @@ import type { MetadataType, SortType } from '../datasets'
import { pauseDocIndexing, resumeDocIndexing } from '../datasets' import { pauseDocIndexing, resumeDocIndexing } from '../datasets'
import type { DocumentDetailResponse, DocumentListResponse, UpdateDocumentBatchParams } from '@/models/datasets' import type { DocumentDetailResponse, DocumentListResponse, UpdateDocumentBatchParams } from '@/models/datasets'
import { DocumentActionType } from '@/models/datasets' import { DocumentActionType } from '@/models/datasets'
import type { CommonResponse, FileDownloadResponse } from '@/models/common' import type { CommonResponse } from '@/models/common'
// Download document with authentication (sends Authorization header)
import Toast from '@/app/components/base/toast'
const NAME_SPACE = 'knowledge/document' const NAME_SPACE = 'knowledge/document'
@@ -97,21 +95,6 @@ export const useSyncDocument = () => {
}) })
} }
// Download document with authentication (sends Authorization header)
export const useDocumentDownload = () => {
return useMutation({
mutationFn: async ({ datasetId, documentId }: { datasetId: string; documentId: string }) => {
// The get helper automatically adds the Authorization header from localStorage
return get<FileDownloadResponse>(`/datasets/${datasetId}/documents/${documentId}/upload-file`)
},
onError: (error: any) => {
// Show a toast notification if download fails
const message = error?.message || 'Download failed.'
Toast.notify({ type: 'error', message })
},
})
}
export const useSyncWebsite = () => { export const useSyncWebsite = () => {
return useMutation({ return useMutation({
mutationFn: ({ datasetId, documentId }: UpdateDocumentBatchParams) => { mutationFn: ({ datasetId, documentId }: UpdateDocumentBatchParams) => {