Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: refactor http as a configurable service #834

Draft
wants to merge 1 commit into
base: refactor/develop
Choose a base branch
from
Draft

feat: refactor http as a configurable service #834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions designer-demo/registry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ import {
Canvas,
EditorInfoService,
AppService,
GenerateCodeService
GenerateCodeService,
http
} from '@opentiny/tiny-engine'
import engineConfig from './engine.config'
import { httpConfig } from './src/http'

export default {
root: {
Expand Down Expand Up @@ -101,5 +103,6 @@ export default {
plugins: [Materials, Tree, Page, Block, Datasource, Bridge, I18n, Script, State, Schema, Help, Robot],
dsls: [{ id: 'engine.dsls.dslvue' }],
settings: [Props, Styles, Events],
canvas: Canvas
canvas: Canvas,
http: [http, { options: httpConfig }]
}
0 packages/http/src/Login.vue → designer-demo/src/http/Login.vue
View file
Open in desktop
File renamed without changes.
127 changes: 127 additions & 0 deletions designer-demo/src/http/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { createApp } from 'vue'
import { useBroadcastChannel } from '@vueuse/core'
import { constants } from '@opentiny/tiny-engine-utils'
import { getMetaApi, http } from '@opentiny/tiny-engine'
import Login from './Login.vue'
import mockData from './mock'

const { BROADCAST_CHANNEL } = constants

const { post: globalNotify } = useBroadcastChannel({ name: BROADCAST_CHANNEL.Notify })
const showError = (url, message) => {
globalNotify({
type: 'error',
title: '接口报错',
message: `报错接口: ${url} \n报错信息: ${message ?? ''}`
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure Error Messages Are Properly Sanitized

When displaying error messages that include dynamic data such as url and message, ensure that they are properly sanitized to prevent potential Cross-Site Scripting (XSS) attacks.

Consider sanitizing the inputs or using a library that handles this automatically.

})
}

const loginDom = document.createElement('div')
document.body.appendChild(loginDom)
const loginVM = createApp(Login).mount(loginDom)

const procession = {
promiseLogin: null,
mePromise: {}
}

window.lowcode = {
platformCenter: {
Session: {
rebuiltCallback: function () {
loginVM.closeLogin()

procession.mePromise.resolve('login ok')
procession.promiseLogin = null
procession.mePromise = {}
}
}
}
}



const LOGIN_EXPIRED_CODE = 401

export const errorResponse = (error) => {
// 用户信息失效时，弹窗提示登录
const { response } = error

Comment on lines +48 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle Undefined response Gracefully

In the errorResponse function, you destructure response from error, but response might be undefined. This could lead to runtime errors when accessing response.status or response.headers.

Apply this diff to add a check for response:

 export const errorResponse = (error) => {
   // 用户信息失效时，弹窗提示登录
-  const { response } = error
+  const { response } = error || {}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { response } = error
export const errorResponse = (error) => {
// 用户信息失效时，弹窗提示登录
const { response } = error || {}

const openLogin = () => {
return new Promise((resolve, reject) => {
if (!procession.promiseLogin) {
procession.promiseLogin = loginVM.openLogin(procession, '/api/rebuildSession')
procession.promiseLogin.then(() => {
getMetaApi('engine.service.http').request(response.config).then(resolve, reject)
// http.request(response.config).then(resolve, reject)
})
}
Comment on lines +52 to +58
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid Potential Race Conditions in openLogin Function

The openLogin function checks if procession.promiseLogin is falsy before initializing it. However, there might be a race condition if multiple calls happen simultaneously.

Consider using a locking mechanism or initializing procession.promiseLogin atomically to prevent multiple logins from being initiated.

})
}

if (response?.status === LOGIN_EXPIRED_CODE) {
// vscode 插件环境弹出输入框提示登录
if (window.vscodeBridge) {
return Promise.resolve(true)
}

// 浏览器环境弹出小窗登录
if (response?.headers['x-login-url']) {
return openLogin()
}
}

// 默认的 error response 显示接口错误
showError(error.config?.url, error?.message)

return response?.data.error ? Promise.reject(response.data.error) : Promise.reject(error.message)
}

function getConfig(env = import.meta.env) {
const baseURL = env.VITE_ORIGIN || ''

// 仅在本地开发时，启用 withCredentials
const dev = env.MODE?.includes('dev')

// 获取租户 id
const getTenant = () => new URLSearchParams(location.search).get('tenant')
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure location Object Is Available

The getTenant function uses location.search, which may not be available in non-browser environments like Node.js or server-side rendering.

Consider adding a check to ensure location is defined:

 const getTenant = () => {
+  if (typeof location === 'undefined') return null
   return new URLSearchParams(location.search).get('tenant')
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const getTenant = () => new URLSearchParams(location.search).get('tenant')
const getTenant = () => {
if (typeof location === 'undefined') return null
return new URLSearchParams(location.search).get('tenant')
}


return {
baseURL,
withCredentials: false,
headers: {
'x-lowcode-mode': dev ? 'develop' : null,
'x-lowcode-org': getTenant()
}
}
}

const isVsCodeEnv = window.vscodeBridge
let environment = import.meta.env
const isDevelopEnv = environment.MODE?.includes('dev')
const isMock = environment.VITE_API_MOCK === 'mock'

const preRequest = (config) => {
if (isDevelopEnv && config.url.match(/\/generate\//)) {
config.baseURL = ''
}

if (isVsCodeEnv) {
config.baseURL = ''
}

return config
}

const { interceptors } = http

export const httpConfig = {
config: getConfig,
interceptors: {
request: [preRequest],
response: [interceptors.response[0], errorResponse]
},
mock: isMock,
mockData: isMock ? mockData : []
}

0 packages/http/src/mock.js → designer-demo/src/http/mock.js
View file
Open in desktop
File renamed without changes.
2 changes: 2 additions & 0 deletions packages/design-core/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export { GenerateCodeService, PluginPanel } from '@opentiny/tiny-engine-common'

export { default as defaultRegistry } from './registry'

export { default as http } from '@opentiny/tiny-engine-http'

Comment on lines +40 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing @opentiny/tiny-engine-http in package.json Dependencies.

The @opentiny/tiny-engine-http module is being exported and imported across multiple files but is not listed in the package.json dependencies. This can lead to installation and runtime issues.

  • Add @opentiny/tiny-engine-http to dependencies in package.json.
🔗 Analysis chain

New HTTP module export aligns with PR objectives.

The addition of the http module export from @opentiny/tiny-engine-http is consistent with the PR's objective to refactor HTTP as a configurable service. This change allows users to import HTTP-related functionalities on demand, which is in line with the modular structure of the file.

To ensure this change doesn't introduce any conflicts or issues, let's verify the module's existence and its usage across the codebase:

This script will help us confirm that the module is properly included in the project dependencies and identify any existing usage or potential conflicts.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the existence of the @opentiny/tiny-engine-http module
echo "Checking for @opentiny/tiny-engine-http in package.json:"
jq '.dependencies["@opentiny/tiny-engine-http"] // .devDependencies["@opentiny/tiny-engine-http"]' package.json

# Check for any existing imports of the http module
echo "Checking for existing imports of the http module:"
rg --type js --type ts 'import.*from.*@opentiny/tiny-engine-http'

# Look for any potential conflicts or duplicate exports
echo "Checking for potential conflicts or duplicate exports:"
rg --type js --type ts 'export.*http.*from'

Length of output: 3414

export * from '@opentiny/tiny-engine-meta-register'

export { EditorInfoService, AppService } from '@opentiny/tiny-engine-common'
5 changes: 0 additions & 5 deletions packages/http/index.js
View file
Open in desktop

This file was deleted.

29 changes: 0 additions & 29 deletions packages/http/src/config.js
View file
Open in desktop

This file was deleted.

169 changes: 55 additions & 114 deletions packages/http/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,14 @@
* A PARTICULAR PURPOSE. SEE THE APPLICABLE LICENSES FOR MORE DETAILS.
*
*/

/* eslint-disable no-undef */
import axios, { globalDefaults } from './axios'
import { createApp } from 'vue'
import { useBroadcastChannel } from '@vueuse/core'
import Login from './Login.vue'
import { getConfig } from './config'
import mockData from './mock'
import { constants } from '@opentiny/tiny-engine-utils'
import { initHook, HOOK_NAME } from '@opentiny/tiny-engine-meta-register'
import axios from './axios'

const { BROADCAST_CHANNEL } = constants

const { post: globalNotify } = useBroadcastChannel({ name: BROADCAST_CHANNEL.Notify })

const procession = {
promiseLogin: null,
mePromise: {}
}

const LOGIN_EXPIRED_CODE = 401

const loginDom = document.createElement('div')
document.body.appendChild(loginDom)
const loginVM = createApp(Login).mount(loginDom)

const showError = (url, message) => {
globalNotify({
type: 'error',
Expand All @@ -43,116 +25,75 @@ const showError = (url, message) => {
})
}

window.lowcode = {
platformCenter: {
Session: {
rebuiltCallback: function () {
loginVM.closeLogin()
const preResponse = (res) => {
if (res.data?.error) {
showError(res.config?.url, res?.data?.error?.message)

procession.mePromise.resolve('login ok')
procession.promiseLogin = null
procession.mePromise = {}
}
}
return Promise.reject(res.data.error)
}

return res.data?.data
}

let http // 封装axios的http实例
let environment = import.meta.env // 当前设计器运行环境变量
const errorResponse = (error) => {
// 用户信息失效时，弹窗提示登录
const { response } = error

const isVsCodeEnv = window.vscodeBridge
const isMock = () => environment.VITE_API_MOCK === 'mock'
// 默认的 error response 显示接口错误
showError(error.config?.url, error?.message)

export const createHttp = (options) => {
// 缓存http实例，避免每个请求重新创建实例
if (http && !options.force) {
return http
}
const isDevelopEnv = environment.MODE?.includes('dev')
const axiosConfig = getConfig(environment)
http = axios(axiosConfig)

// 如果未指定是否启用 mock，则本地开发时默认启用，模拟数据在 public/mock 目录下
const { enableMock = isDevelopEnv } = options
enableMock && http.mock(mockData)
return response?.data.error ? Promise.reject(response.data.error) : Promise.reject(error.message)
}

const preRequest = (config) => {
if (isDevelopEnv && config.url.match(/\/generate\//)) {
config.baseURL = ''
}
const initialState = {
http: null
}

if (isVsCodeEnv) {
config.baseURL = ''
export default {
id: 'engine.service.http',
options: {
config: {},
interceptors: {
request: [],
response: [preResponse, errorResponse]
},
mock: false,
mockData: []
},
type: 'MetaService',
initialState,
apis: ({ state }) => ({
http: state.http,
request: state.http.request,
get: state.http.get,
post: state.http.post,
put: state.http.put,
delete: state.http.delete
}),
init: ({ state, options }) => {
const { config, mock = false, mockData, interceptors } = options

let axiosConfig = config

// config 支持函数
if (typeof axiosConfig === 'function') {
axiosConfig = axiosConfig()
}

return config
}

// 请求拦截器
http.interceptors.request.use(preRequest)

const preResponse = (res) => {
if (res.data?.error) {
showError(res.config?.url, res?.data?.error?.message)
const http = axios(axiosConfig)

return Promise.reject(res.data.error)
if (mock) {
http.mock(mockData)
}

return res.data?.data
}

const openLogin = () => {
return new Promise((resolve, reject) => {
if (!procession.promiseLogin) {
procession.promiseLogin = loginVM.openLogin(procession, '/api/rebuildSession')
procession.promiseLogin.then(() => {
http.request(response.config).then(resolve, reject)
})
}
})
}

const errorResponse = (error) => {
// 用户信息失效时，弹窗提示登录
const { response } = error
if (response?.status === LOGIN_EXPIRED_CODE) {
// vscode 插件环境弹出输入框提示登录
if (window.vscodeBridge) {
return Promise.resolve(true)
}

// 浏览器环境弹出小窗登录
if (response?.headers['x-login-url']) {
return openLogin()
}
if (Array.isArray(interceptors.request)) {
http.interceptors.request.use(...interceptors.request)
}

showError(error.config?.url, error?.message)

return response?.data.error ? Promise.reject(response.data.error) : Promise.reject(error.message)
}

// 响应拦截器
http.interceptors.response.use(preResponse, errorResponse)

return http
}
if (Array.isArray(interceptors.response)) {
http.interceptors.request.use(...interceptors.response)
}
Comment on lines +94 to +95
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incorrectly attaching response interceptors to request interceptors

At line 94, response interceptors are being added to the request interceptors chain using http.interceptors.request.use(...interceptors.response). This will result in the response interceptors not being called appropriately and could cause unexpected behaviors in request handling.

Apply this diff to fix the issue:

-          http.interceptors.request.use(...interceptors.response)
+          http.interceptors.response.use(...interceptors.response)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
http.interceptors.request.use(...interceptors.response)
}
http.interceptors.response.use(...interceptors.response)
}


/**
* 根据环境不同初始化设置http参数
* @param {*} env: 当前环境变量
*/
export const initHttp = ({ env }) => {
if (Object.keys(env).length) {
environment = env
state.http = http
}
const baseURL = environment.VITE_ORIGIN
// 调用初始化方法前可能已经存在已经实例化的http，需要设置baseURL
http?.defaults('baseURL', baseURL)
globalDefaults('baseURL', baseURL)
http = createHttp({ force: true, enableMock: isMock() })
}

export const useHttp = () => createHttp({ enableMock: isMock() })

initHook(HOOK_NAME.useHttp, useHttp, { useDefaultExport: true })
Loading
Loading