Skip to content

Commit

Permalink
🐛 fix: fix issues for client fetch (lobehub#2753)
Browse files Browse the repository at this point in the history 
* 🚸 feat: improve UX for clientfetch

* 🚨 fix: circular dependence

* 💡 docs: update comments

* ✅ test: client fetch enabled

* ✅ test: client fetch

* ?? fix: wrong condition

* ✅ test: update tests

* 🐛 fix: wrong selector

* ♻️ refactor: modify vaults selectors

* ⏪ revert: style changed

* ⏪ revert: cancel style fix

* 🧪 test: isProviderEndpointNotEmpty

* 🧪 test: isProviderApiKeyNotEmpty

* ✅ test: isProviderEndpointNotEmpty

* ♻️ refactor: endpoint for aws bedrock

---------

Co-authored-by: Arvin Xu <arvinx@foxmail.com>
  • Loading branch information
@cy948 @arvinxx
cy948 and arvinxx authored Jun 16, 2024
1 parent 9174b29 commit 6f5be5d
Show file tree
Hide file tree
Showing 9 changed files with 281 additions and 10 deletions.
2 changes: 1 addition & 1 deletion docs/self-hosting/platform/zeabur.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,4 @@ You can create a subdomain provided by Zeabur, or choose to bind a custom domain

### Zeabur shall start auto build and you should be able to access it by the domain of your choice after a while.

</Steps>
</Steps>
2 changes: 1 addition & 1 deletion docs/self-hosting/platform/zeabur.zh-CN.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,4 @@ tags:

### Zeabur 将开始自动构建，您应该可以在一段时间后通过您选择的域名访问它。

</Steps>
</Steps>
12 changes: 11 additions & 1 deletion src/app/(main)/settings/llm/components/ProviderConfig/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,14 @@ const ProviderConfig = memo<ProviderConfigProps>(
enabled,
isFetchOnClient,
isProviderEndpointNotEmpty,
isProviderApiKeyNotEmpty,
] = useUserStore((s) => [
s.toggleProviderEnabled,
s.setSettings,
modelConfigSelectors.isProviderEnabled(id)(s),
modelConfigSelectors.isProviderFetchOnClient(id)(s),
keyVaultsConfigSelectors.isProviderEndpointNotEmpty(id)(s),
keyVaultsConfigSelectors.isProviderApiKeyNotEmpty(id)(s),
]);

useSyncSettings(form);
Expand Down Expand Up @@ -122,7 +124,15 @@ const ProviderConfig = memo<ProviderConfigProps>(
label: proxyUrl?.title || t('llm.proxyUrl.title'),
name: [KeyVaultsConfigKey, id, LLMProviderBaseUrlKey],
},
(showBrowserRequest || (showEndpoint && isProviderEndpointNotEmpty)) && {
/*
* Conditions to show Client Fetch Switch
* 1. Component props
* 2. Provider allow to edit endpoint and the value of endpoint is not empty
* 3. There is an apikey provided by user
*/
(showBrowserRequest ||
(showEndpoint && isProviderEndpointNotEmpty) ||
(showApiKey && isProviderApiKeyNotEmpty)) && {
children: (
<Switch
onChange={(enabled) => {
Expand Down
2 changes: 1 addition & 1 deletion src/app/api/middleware/auth/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ChatErrorType } from '@/types/fetch';
import { checkAuthMethod, getJWTPayload } from './utils';

type CreateRuntime = (jwtPayload: JWTPayload) => AgentRuntime;
type RequestOptions = { createRuntime?: CreateRuntime, params: { provider: string }; };
type RequestOptions = { createRuntime?: CreateRuntime; params: { provider: string } };

export type RequestHandler = (
req: Request,
Expand Down
201 changes: 201 additions & 0 deletions src/store/user/slices/modelList/selectors/keyVaults.test.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
import { describe, expect, it } from 'vitest';

import { UserStore } from '@/store/user';
import {
AWSBedrockKeyVault,
AzureOpenAIKeyVault,
OpenAICompatibleKeyVault,
} from '@/types/user/settings';
import { merge } from '@/utils/merge';

import { initialSettingsState } from '../../settings/initialState';
import { keyVaultsConfigSelectors } from './keyVaults';

describe('keyVaultsConfigSelectors', () => {
describe('isProviderEndpointNotEmpty', () => {
describe('OpenAICompatibleKeyVault', () => {
it('should return true if provider endpoint is not empty', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
openai: {
endpoint: 'endpoint',
} as OpenAICompatibleKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderEndpointNotEmpty('openai')(s)).toBe(true);
});

it('should return false if provider endpoint is empty', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
openai: {
endpoint: undefined,
} as OpenAICompatibleKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderEndpointNotEmpty('openai')(s)).toBe(false);
});
});

describe('AzureOpenAIKeyVault', () => {
it('should return true if provider endpoint is not empty', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
azure: {
baseURL: 'baseURL',
} as AzureOpenAIKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderEndpointNotEmpty('azure')(s)).toBe(true);
});

it('should return false if provider endpoint is empty', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
azure: {
baseURL: undefined,
} as AzureOpenAIKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderEndpointNotEmpty('azure')(s)).toBe(false);
});
});

// Always return false for AWSBedrockKeyVault
describe('AWSBedrockKeyVault', () => {
it('should return false if provider region is not empty for AWSBedrockKeyVault', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
bedrock: {
region: 'region',
} as AWSBedrockKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderEndpointNotEmpty('bedrock')(s)).toBe(false);
});

it('should return false if provider region is empty for AWSBedrockKeyVault', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
bedrock: {
region: undefined,
} as AWSBedrockKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderEndpointNotEmpty('bedrock')(s)).toBe(false);
});
});
});

describe('isProviderApiKeyNotEmpty', () => {
describe('OpenAICompatibleKeyVault', () => {
it('should return true if provider apikey is not empty', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
openai: {
apiKey: 'apikey',
} as OpenAICompatibleKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderApiKeyNotEmpty('openai')(s)).toBe(true);
});

it('should return false if provider apikey is empty', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
openai: {
apiKey: undefined,
} as OpenAICompatibleKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderApiKeyNotEmpty('openai')(s)).toBe(false);
});
});

describe('AzureOpenAIKeyVault', () => {
it('should return true if provider apikey is not empty', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
azure: {
apiKey: 'apikey',
} as AzureOpenAIKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderApiKeyNotEmpty('azure')(s)).toBe(true);
});

it('should return false if provider apikey is empty', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
azure: {
apiKey: undefined,
} as AzureOpenAIKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderApiKeyNotEmpty('azure')(s)).toBe(false);
});
});

describe('AWSBedrockKeyVault', () => {
it('should return true if provider accessKeyId is not empty for AWSBedrockKeyVault', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
bedrock: {
accessKeyId: 'accessKeyId',
} as AWSBedrockKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderApiKeyNotEmpty('bedrock')(s)).toBe(true);
});

it('should return true if provider secretAccessKey is not empty for AWSBedrockKeyVault', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
bedrock: {
secretAccessKey: 'secretAccessKey',
} as AWSBedrockKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderApiKeyNotEmpty('bedrock')(s)).toBe(true);
});

it('should return false if provider accessKeyId and secretAccessKey are both empty for AWSBedrockKeyVault', () => {
const s = merge(initialSettingsState, {
settings: {
keyVaults: {
bedrock: {
accessKeyId: undefined,
secretAccessKey: undefined,
} as AWSBedrockKeyVault,
},
},
}) as unknown as UserStore;
expect(keyVaultsConfigSelectors.isProviderApiKeyNotEmpty('bedrock')(s)).toBe(false);
});
});
});
});
18 changes: 15 additions & 3 deletions src/store/user/slices/modelList/selectors/keyVaults.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { UserStore } from '@/store/user';
import {
AWSBedrockKeyVault,
AzureOpenAIKeyVault,
GlobalLLMProviderKey,
OpenAICompatibleKeyVault,
UserKeyVaults,
Expand All @@ -15,17 +17,27 @@ const bedrockConfig = (s: UserStore) => keyVaultsSettings(s).bedrock || {};
const ollamaConfig = (s: UserStore) => keyVaultsSettings(s).ollama || {};
const azureConfig = (s: UserStore) => keyVaultsSettings(s).azure || {};
const getVaultByProvider = (provider: GlobalLLMProviderKey) => (s: UserStore) =>
(keyVaultsSettings(s)[provider] || {}) as OpenAICompatibleKeyVault;
(keyVaultsSettings(s)[provider] || {}) as OpenAICompatibleKeyVault &
AzureOpenAIKeyVault &
AWSBedrockKeyVault;

const isProviderEndpointNotEmpty = (provider: string) => (s: UserStore) =>
!!getVaultByProvider(provider as GlobalLLMProviderKey)(s)?.baseURL;
const isProviderEndpointNotEmpty = (provider: string) => (s: UserStore) => {
const vault = getVaultByProvider(provider as GlobalLLMProviderKey)(s);
return !!vault?.baseURL || !!vault?.endpoint;
};

const isProviderApiKeyNotEmpty = (provider: string) => (s: UserStore) => {
const vault = getVaultByProvider(provider as GlobalLLMProviderKey)(s);
return !!vault?.apiKey || !!vault?.accessKeyId || !!vault?.secretAccessKey;
};

const password = (s: UserStore) => keyVaultsSettings(s).password || '';

export const keyVaultsConfigSelectors = {
azureConfig,
bedrockConfig,
getVaultByProvider,
isProviderApiKeyNotEmpty,
isProviderEndpointNotEmpty,
ollamaConfig,
openAIConfig,
Expand Down
30 changes: 29 additions & 1 deletion src/store/user/slices/modelList/selectors/modelConfig.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('modelConfigSelectors', () => {
});

describe('isProviderFetchOnClient', () => {
// The next 4 case are base on the rules on https://github.com/lobehub/lobe-chat/pull/2753
it('client fetch should disabled on default', () => {
const s = merge(initialSettingsState, {
settings: {
Expand All @@ -46,16 +47,43 @@ describe('modelConfigSelectors', () => {
},
},
} as UserSettingsState) as unknown as UserStore;
expect(modelConfigSelectors.isProviderFetchOnClient('azure')(s)).toBe(false);
});

it('client fetch should disabled if no apikey or endpoint provided even user set it enabled', () => {
const s = merge(initialSettingsState, {
settings: {
languageModel: {
azure: { fetchOnClient: true },
},
},
} as UserSettingsState) as unknown as UserStore;
expect(modelConfigSelectors.isProviderFetchOnClient('azure')(s)).toBe(false);
});

it('client fetch should enabled if user set it enabled', () => {
it('client fetch should enable if only endpoint provided', () => {
const s = merge(initialSettingsState, {
settings: {
languageModel: {
azure: { fetchOnClient: false },
},
keyVaults: {
azure: { endpoint: 'https://example.com' },
},
},
} as UserSettingsState) as unknown as UserStore;
expect(modelConfigSelectors.isProviderFetchOnClient('azure')(s)).toBe(true);
});

it('client fetch should control by user when a apikey or endpoint provided', () => {
const s = merge(initialSettingsState, {
settings: {
languageModel: {
azure: { fetchOnClient: true },
},
keyVaults: {
azure: { apiKey: 'some-key' },
},
},
} as UserSettingsState) as unknown as UserStore;
expect(modelConfigSelectors.isProviderFetchOnClient('azure')(s)).toBe(true);
Expand Down
22 changes: 21 additions & 1 deletion src/store/user/slices/modelList/selectors/modelConfig.ts
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,35 @@
import { UserStore } from '@/store/user';
import { GlobalLLMProviderKey } from '@/types/user/settings';

import { UserStore } from '../../../store';
import { currentLLMSettings, getProviderConfigById } from '../../settings/selectors/settings';
import { keyVaultsConfigSelectors } from './keyVaults';

const isProviderEnabled = (provider: GlobalLLMProviderKey) => (s: UserStore) =>
getProviderConfigById(provider)(s)?.enabled || false;

/**
* @description The conditions to enable client fetch
* 1. If no baseUrl and apikey input, force on Server.
* 2. If only contains baseUrl, force on Client
* 3. Follow the user settings.
* 4. On Server, by default.
*/
const isProviderFetchOnClient = (provider: GlobalLLMProviderKey | string) => (s: UserStore) => {
const config = getProviderConfigById(provider)(s);

// 1. If no baseUrl and apikey input, force on Server.
const isProviderEndpointNotEmpty =
keyVaultsConfigSelectors.isProviderEndpointNotEmpty(provider)(s);
const isProviderApiKeyNotEmpty = keyVaultsConfigSelectors.isProviderApiKeyNotEmpty(provider)(s);
if (!isProviderEndpointNotEmpty && !isProviderApiKeyNotEmpty) return false;

// 2. If only contains baseUrl, force on Client
if (isProviderEndpointNotEmpty && !isProviderApiKeyNotEmpty) return true;

// 3. Follow the user settings.
if (typeof config?.fetchOnClient !== 'undefined') return config?.fetchOnClient;

// 4. On Server, by default.
return false;
};

Expand Down
2 changes: 1 addition & 1 deletion src/types/user/settings/keyVaults.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export interface OpenAICompatibleKeyVault {
baseURL?: string;
}

interface AzureOpenAIKeyVault {
export interface AzureOpenAIKeyVault {
apiKey?: string;
apiVersion?: string;
endpoint?: string;
Expand Down

0 comments on commit 6f5be5d

Please sign in to comment.