mirror of
https://github.com/apache/superset.git
synced 2026-08-20 07:01:17 +00:00
Compare commits
29
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
28db79db4c | ||
|
|
e08fc8905f | ||
|
|
8a8940557d | ||
|
|
faf7c34c0a | ||
|
|
b8fca2145d | ||
|
|
c10054f521 | ||
|
|
8c500ccee1 | ||
|
|
6d77efad29 | ||
|
|
8222db3340 | ||
|
|
01ce8358a6 | ||
|
|
53a8a0e140 | ||
|
|
eafbff9f8d | ||
|
|
1339bcd9da | ||
|
|
334e280489 | ||
|
|
c07f3ebf2d | ||
|
|
1569915096 | ||
|
|
fde0ba26d1 | ||
|
|
097c99b19c | ||
|
|
5ce52e531d | ||
|
|
34cd50cc48 | ||
|
|
c0ab5f3385 | ||
|
|
7d4f30574f | ||
|
|
e4ea6e23d8 | ||
|
|
a13a5f1af4 | ||
|
|
f994602096 | ||
|
|
086b4af65d | ||
|
|
fd063d17bf | ||
|
|
60e1802c52 | ||
|
|
2d1daac11a |
+1
-1
@@ -29,7 +29,7 @@
|
||||
"dependencies:python":
|
||||
- changed-files:
|
||||
- any-glob-to-any-file:
|
||||
- 'superset/requirements/**'
|
||||
- 'requirements/**'
|
||||
- 'superset/translations/requirements.txt'
|
||||
- 'RELEASING/requirements.txt'
|
||||
|
||||
|
||||
@@ -48,7 +48,7 @@ jobs:
|
||||
python-version: "3.11"
|
||||
|
||||
- name: Install uv
|
||||
uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0
|
||||
uses: astral-sh/setup-uv@ae62891fec2bb8e7d6c99fc78c9fec3a63790f8d # v10.0.0
|
||||
with:
|
||||
python-version: "3.11"
|
||||
enable-cache: true
|
||||
|
||||
@@ -24,6 +24,8 @@ assists people when migrating to a new version.
|
||||
|
||||
## Next
|
||||
|
||||
- `SAMPLES_ROW_LIMIT` is now the default for `/datasource/samples` requests without a valid explicit `per_page`, rather than a hard per-request ceiling; explicit limits are honored up to the existing global row-limit ceiling, matching `/chart/data` SAMPLES requests.
|
||||
|
||||
### OAuth2 database callback metrics include their outcome
|
||||
|
||||
The unqualified `DatabaseRestApi.oauth2` StatsD counter has been replaced with
|
||||
|
||||
@@ -400,7 +400,7 @@ Once enabled, each user manages their own keys from their profile page:
|
||||
1. Open the user menu (top-right) and click **Info** to navigate to the User Info page
|
||||
2. Expand the **API Keys** section
|
||||
3. Click **+ API Key**
|
||||
4. Enter a name and (optionally) an expiration date
|
||||
4. Enter a name and optionally select resource scopes
|
||||
5. Copy the generated token — it is shown only once
|
||||
|
||||
Only users with the `can_read` and `can_write` permissions on `ApiKey` (granted by default to Admins) can manage API keys.
|
||||
@@ -415,6 +415,18 @@ Authorization: Bearer <your-api-key>
|
||||
|
||||
This works for all REST API endpoints and the MCP server. The request is executed with the permissions of the user who created the key.
|
||||
|
||||
#### API Key Scopes
|
||||
|
||||
The creation dialog can restrict an API key to MCP resource actions such as
|
||||
`superset:dashboard:read` or `superset:chart:write`. A scope is an additional
|
||||
restriction: it never grants a permission that the creating user does not
|
||||
already have through Superset RBAC. Write scopes also cover update and delete
|
||||
operations for that resource; `superset:sqllab:write` covers SQL execution.
|
||||
|
||||
Keys created without scopes retain legacy RBAC-only behavior. The scoped-key
|
||||
restrictions described here are enforced by the MCP server; regular REST API
|
||||
routes continue to apply their existing Superset RBAC checks.
|
||||
|
||||
#### Use Cases
|
||||
|
||||
- **CI/CD pipelines** — automated chart/dashboard exports and imports
|
||||
|
||||
+4
-4
@@ -64,8 +64,8 @@
|
||||
"antd": "^6.6.0",
|
||||
"baseline-browser-mapping": "^2.11.13",
|
||||
"caniuse-lite": "^1.0.30001809",
|
||||
"docusaurus-plugin-openapi-docs": "^5.1.3",
|
||||
"docusaurus-theme-openapi-docs": "^5.1.3",
|
||||
"docusaurus-plugin-openapi-docs": "^5.2.0",
|
||||
"docusaurus-theme-openapi-docs": "^5.2.0",
|
||||
"js-yaml": "^5.2.3",
|
||||
"json-bigint": "^1.0.0",
|
||||
"prism-react-renderer": "^2.4.1",
|
||||
@@ -78,7 +78,7 @@
|
||||
"remark-import-partial": "^0.0.2",
|
||||
"reselect": "^5.2.0",
|
||||
"storybook": "^10.5.7",
|
||||
"swagger-ui-react": "^5.32.12",
|
||||
"swagger-ui-react": "^5.32.13",
|
||||
"swc-loader": "^0.2.7",
|
||||
"tinycolor2": "^1.4.2",
|
||||
"unist-util-visit": "^5.1.0"
|
||||
@@ -93,7 +93,7 @@
|
||||
"@typescript-eslint/parser": "^8.67.0",
|
||||
"eslint": "^9.39.2",
|
||||
"eslint-plugin-react": "^7.37.5",
|
||||
"globals": "^17.9.0",
|
||||
"globals": "^17.10.0",
|
||||
"oxfmt": "^0.63.0",
|
||||
"typescript": "~6.0.3",
|
||||
"typescript-eslint": "^8.67.0",
|
||||
|
||||
Vendored
+44
-32
@@ -3407,22 +3407,26 @@
|
||||
"nullable": true,
|
||||
"type": "string"
|
||||
},
|
||||
"description": {
|
||||
"nullable": true,
|
||||
"type": "string"
|
||||
},
|
||||
"editors": {
|
||||
"items": {
|
||||
"$ref": "#/components/schemas/DashboardRestApi.get_list.Subject"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"id": {
|
||||
"type": "integer"
|
||||
},
|
||||
"is_managed_externally": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"owners": {
|
||||
"$ref": "#/components/schemas/DashboardRestApi.get_list.User2"
|
||||
},
|
||||
"published": {
|
||||
"nullable": true,
|
||||
"type": "boolean"
|
||||
},
|
||||
"roles": {
|
||||
"$ref": "#/components/schemas/DashboardRestApi.get_list.Role"
|
||||
},
|
||||
"slug": {
|
||||
"maxLength": 255,
|
||||
"nullable": true,
|
||||
@@ -3432,10 +3436,10 @@
|
||||
"readOnly": true
|
||||
},
|
||||
"tags": {
|
||||
"$ref": "#/components/schemas/DashboardRestApi.get_list.Tag"
|
||||
},
|
||||
"thumbnail_url": {
|
||||
"readOnly": true
|
||||
"items": {
|
||||
"$ref": "#/components/schemas/DashboardRestApi.get_list.Tag"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"url": {
|
||||
"readOnly": true
|
||||
@@ -3444,21 +3448,46 @@
|
||||
"format": "uuid",
|
||||
"nullable": true,
|
||||
"type": "string"
|
||||
},
|
||||
"viewers": {
|
||||
"items": {
|
||||
"$ref": "#/components/schemas/DashboardRestApi.get_list.Subject1"
|
||||
},
|
||||
"type": "array"
|
||||
}
|
||||
},
|
||||
"type": "object"
|
||||
},
|
||||
"DashboardRestApi.get_list.Role": {
|
||||
"DashboardRestApi.get_list.Subject": {
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": "integer"
|
||||
},
|
||||
"name": {
|
||||
"maxLength": 64,
|
||||
"label": {
|
||||
"maxLength": 255,
|
||||
"type": "string"
|
||||
},
|
||||
"type": {
|
||||
"type": "integer"
|
||||
}
|
||||
},
|
||||
"required": ["name"],
|
||||
"required": ["label", "type"],
|
||||
"type": "object"
|
||||
},
|
||||
"DashboardRestApi.get_list.Subject1": {
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": "integer"
|
||||
},
|
||||
"label": {
|
||||
"maxLength": 255,
|
||||
"type": "string"
|
||||
},
|
||||
"type": {
|
||||
"type": "integer"
|
||||
}
|
||||
},
|
||||
"required": ["label", "type"],
|
||||
"type": "object"
|
||||
},
|
||||
"DashboardRestApi.get_list.Tag": {
|
||||
@@ -3511,23 +3540,6 @@
|
||||
"required": ["first_name", "last_name"],
|
||||
"type": "object"
|
||||
},
|
||||
"DashboardRestApi.get_list.User2": {
|
||||
"properties": {
|
||||
"first_name": {
|
||||
"maxLength": 64,
|
||||
"type": "string"
|
||||
},
|
||||
"id": {
|
||||
"type": "integer"
|
||||
},
|
||||
"last_name": {
|
||||
"maxLength": 64,
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": ["first_name", "last_name"],
|
||||
"type": "object"
|
||||
},
|
||||
"DashboardRestApi.post": {
|
||||
"properties": {
|
||||
"certification_details": {
|
||||
@@ -16506,7 +16518,7 @@
|
||||
},
|
||||
"result": {
|
||||
"items": {
|
||||
"type": "object"
|
||||
"$ref": "#/components/schemas/DashboardRestApi.get_list"
|
||||
},
|
||||
"type": "array"
|
||||
}
|
||||
|
||||
+23
-23
@@ -8014,10 +8014,10 @@ doctrine@^2.1.0:
|
||||
dependencies:
|
||||
esutils "^2.0.2"
|
||||
|
||||
docusaurus-plugin-openapi-docs@^5.1.3:
|
||||
version "5.1.3"
|
||||
resolved "https://registry.yarnpkg.com/docusaurus-plugin-openapi-docs/-/docusaurus-plugin-openapi-docs-5.1.3.tgz#b8cd5f8451aaf881deb1a744a8295685f1681865"
|
||||
integrity sha512-HnpblSBdXoR39VNTIW9zWERUsMJxXOpvdQoBKyaTkUBPwCM48Z76+ndo2yO2vADq+EhWjJlfxL1DUzCrgNjThQ==
|
||||
docusaurus-plugin-openapi-docs@^5.2.0:
|
||||
version "5.2.0"
|
||||
resolved "https://registry.yarnpkg.com/docusaurus-plugin-openapi-docs/-/docusaurus-plugin-openapi-docs-5.2.0.tgz#8318ec90cd21fed023be57696211af7d72fd81db"
|
||||
integrity sha512-MjrfRAMB64uvdxRVz6L9AXWe4QFjCdoBAzYs306yyI3nnXHsFj2lv2FnLA90JV9CAUZaGiYMvvkzBo2Nrkq/9w==
|
||||
dependencies:
|
||||
"@apidevtools/json-schema-ref-parser" "^15.3.3"
|
||||
"@redocly/openapi-core" "^2.25.2"
|
||||
@@ -8035,10 +8035,10 @@ docusaurus-plugin-openapi-docs@^5.1.3:
|
||||
swagger2openapi "^7.0.8"
|
||||
xml-formatter "^3.6.6"
|
||||
|
||||
docusaurus-theme-openapi-docs@^5.1.3:
|
||||
version "5.1.3"
|
||||
resolved "https://registry.yarnpkg.com/docusaurus-theme-openapi-docs/-/docusaurus-theme-openapi-docs-5.1.3.tgz#e23644a63785352abbc76e42760c0dfdff3669e1"
|
||||
integrity sha512-npbD1QahtjAEmrOet/86i5fTmcJX4/rPhVT+c0qKjm7StUNbyqjwchSVBQuU1rB69T51JOA9TpT/y6QcB9Xjvw==
|
||||
docusaurus-theme-openapi-docs@^5.2.0:
|
||||
version "5.2.0"
|
||||
resolved "https://registry.yarnpkg.com/docusaurus-theme-openapi-docs/-/docusaurus-theme-openapi-docs-5.2.0.tgz#6d93a74e2e3cf0ae77d24e1c4144bd2e74a52115"
|
||||
integrity sha512-L0b80LzaMUfr76a9EQXRPCf8nxkEz8Xo6Aknnke1UeE2oXsgoiVki6U+RTE7GmJRjO8zSNKXyckGmGmqqWuHeA==
|
||||
dependencies:
|
||||
"@hookform/error-message" "^2.0.1"
|
||||
"@reduxjs/toolkit" "^2.8.2"
|
||||
@@ -8123,7 +8123,7 @@ domhandler@^5.0.2, domhandler@^5.0.3:
|
||||
dependencies:
|
||||
domelementtype "^2.3.0"
|
||||
|
||||
dompurify@^3.3.3, dompurify@^3.4.12:
|
||||
dompurify@^3.3.3, dompurify@^3.4.13:
|
||||
version "3.4.13"
|
||||
resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-3.4.13.tgz#fc28949d59f92d62e28a3a764bcbeee35897a1be"
|
||||
integrity sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ==
|
||||
@@ -9174,10 +9174,10 @@ globals@^14.0.0:
|
||||
resolved "https://registry.yarnpkg.com/globals/-/globals-14.0.0.tgz#898d7413c29babcf6bafe56fcadded858ada724e"
|
||||
integrity sha512-oahGvuMGQlPw/ivIYBjVSrWAfWLBeku5tpPE2fOPLi+WHffIWbuh2tCjhyQhTBPMf5E9jDEH4FOmTYgYwbKwtQ==
|
||||
|
||||
globals@^17.9.0:
|
||||
version "17.9.0"
|
||||
resolved "https://registry.yarnpkg.com/globals/-/globals-17.9.0.tgz#e43f252d6bbe71508da43902a1709c8895a59f70"
|
||||
integrity sha512-m/MvAW61QVU5VDNF1Vj8axt016h8w7L5TU1e9zlab7XIttAT2YAlCwl75K1fOqvMM9apmD7lbCIRhpfkhmxhCg==
|
||||
globals@^17.10.0:
|
||||
version "17.10.0"
|
||||
resolved "https://registry.yarnpkg.com/globals/-/globals-17.10.0.tgz#f9dbd847ae99e236f98b13095e2426ac3b25a45c"
|
||||
integrity sha512-V0kztuWST2k8A/VbxAY8+L+7+Rgo3fyA24IHRLrZp7HOzJjV0gHSaZUjK9lpP/IrBSNite2tZ1prhRkinRu1CA==
|
||||
|
||||
globalthis@^1.0.4:
|
||||
version "1.0.4"
|
||||
@@ -10284,10 +10284,10 @@ js-levenshtein@^1.1.6:
|
||||
resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-4.0.0.tgz#19203fb59991df98e3a287050d4647cdeaf32499"
|
||||
integrity sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==
|
||||
|
||||
js-yaml@4.1.0, js-yaml@=4.3.0, js-yaml@^4.1.0, js-yaml@^4.1.1, js-yaml@^4.2.0, js-yaml@^4.3.0:
|
||||
version "4.3.0"
|
||||
resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-4.3.0.tgz#d1900572a7f7cf0b5f540c83673e60bad3436592"
|
||||
integrity sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q==
|
||||
js-yaml@4.1.0, js-yaml@=4.3.1, js-yaml@^4.1.0, js-yaml@^4.1.1, js-yaml@^4.2.0, js-yaml@^4.3.0:
|
||||
version "4.3.1"
|
||||
resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-4.3.1.tgz#01216c001d67f48e2cd560d708c7af21090a3848"
|
||||
integrity sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==
|
||||
dependencies:
|
||||
argparse "^2.0.1"
|
||||
|
||||
@@ -15103,10 +15103,10 @@ swagger-client@^3.37.8:
|
||||
"@swagger-api/apidom-parser-adapter-openapi-yaml-3-2" "^1.12.0"
|
||||
"@swagger-api/apidom-parser-adapter-yaml-1-2" "^1.12.0"
|
||||
|
||||
swagger-ui-react@^5.32.12:
|
||||
version "5.32.12"
|
||||
resolved "https://registry.yarnpkg.com/swagger-ui-react/-/swagger-ui-react-5.32.12.tgz#47525a26774eb02db0e6203af72f5b32fa6205cc"
|
||||
integrity sha512-WCdkNOQyMTZDu+z356FpwVWHf1dwZgQPUjdQPh1L4r7jULaJTKKlIItXq6WsZdYeXvsHndMdxxccEQXOAroUHQ==
|
||||
swagger-ui-react@^5.32.13:
|
||||
version "5.32.13"
|
||||
resolved "https://registry.yarnpkg.com/swagger-ui-react/-/swagger-ui-react-5.32.13.tgz#04c96140b0a2d4ea01ebec4d4cfc655d5ed9a500"
|
||||
integrity sha512-XIDl+Ny6kE1N8wpSPiOFrjPfAevs4GR4XmV6BT6NLMikkMFIbIVocWbA8pnKYyYXQe8Rccfli5o2zDfySw0FnQ==
|
||||
dependencies:
|
||||
"@babel/runtime-corejs3" "^7.27.1"
|
||||
"@scarf/scarf" "=1.4.0"
|
||||
@@ -15115,11 +15115,11 @@ swagger-ui-react@^5.32.12:
|
||||
classnames "^2.5.1"
|
||||
css.escape "1.5.1"
|
||||
deep-extend "0.6.0"
|
||||
dompurify "^3.4.12"
|
||||
dompurify "^3.4.13"
|
||||
ieee754 "^1.2.1"
|
||||
immutable "^4.3.9"
|
||||
js-file-download "^0.4.12"
|
||||
js-yaml "=4.3.0"
|
||||
js-yaml "=4.3.1"
|
||||
lodash "^4.18.1"
|
||||
prop-types "^15.8.1"
|
||||
randexp "^0.5.3"
|
||||
|
||||
Generated
+31
-22
@@ -186,7 +186,7 @@
|
||||
"@storybook/test-runner": "0.24.4",
|
||||
"@svgr/webpack": "^8.1.0",
|
||||
"@swc/core": "^1.15.47",
|
||||
"@swc/plugin-emotion": "^14.15.0",
|
||||
"@swc/plugin-emotion": "^14.19.0",
|
||||
"@swc/plugin-transform-imports": "^12.5.0",
|
||||
"@testing-library/dom": "^10.4.1",
|
||||
"@testing-library/jest-dom": "^7.0.1",
|
||||
@@ -11808,9 +11808,9 @@
|
||||
}
|
||||
},
|
||||
"node_modules/@swc/plugin-emotion": {
|
||||
"version": "14.15.0",
|
||||
"resolved": "https://registry.npmjs.org/@swc/plugin-emotion/-/plugin-emotion-14.15.0.tgz",
|
||||
"integrity": "sha512-nCsTO7mOOPz2UnT3N6YWb014uI0CVxeKg53A/KM/CvuSIE6H3KPkhaziJQ3q2jI3u3LfFuDKEnU5ZmB1330Dqg==",
|
||||
"version": "14.19.0",
|
||||
"resolved": "https://registry.npmjs.org/@swc/plugin-emotion/-/plugin-emotion-14.19.0.tgz",
|
||||
"integrity": "sha512-0/q84ro0a7kdjpYpn9Wmi5/RLHYuSwYjO638lE5ZBQfIvYpSLJxbEgLsObCmdH4KPe2stoN8plVKUpCsKPggaw==",
|
||||
"dev": true,
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
@@ -20612,7 +20612,7 @@
|
||||
"version": "0.8.0",
|
||||
"resolved": "https://registry.npmjs.org/expect-playwright/-/expect-playwright-0.8.0.tgz",
|
||||
"integrity": "sha512-+kn8561vHAY+dt+0gMqqj1oY+g5xWrsuGMk4QGxotT2WS545nVqqjs37z6hrYfIuucwqthzwJfCJUEYqixyljg==",
|
||||
"deprecated": "⚠️ The 'expect-playwright' package is deprecated. The Playwright core assertions (via @playwright/test) now cover the same functionality. Please migrate to built-in expect. See https://playwright.dev/docs/test-assertions for migration.",
|
||||
"deprecated": "\u26a0\ufe0f The 'expect-playwright' package is deprecated. The Playwright core assertions (via @playwright/test) now cover the same functionality. Please migrate to built-in expect. See https://playwright.dev/docs/test-assertions for migration.",
|
||||
"dev": true,
|
||||
"license": "MIT"
|
||||
},
|
||||
@@ -26023,7 +26023,7 @@
|
||||
"version": "0.4.0",
|
||||
"resolved": "https://registry.npmjs.org/jest-process-manager/-/jest-process-manager-0.4.0.tgz",
|
||||
"integrity": "sha512-80Y6snDyb0p8GG83pDxGI/kQzwVTkCxc7ep5FPe/F6JYdvRDhwr6RzRmPSP7SEwuLhxo80lBS/NqOdUIbHIfhw==",
|
||||
"deprecated": "⚠️ The 'jest-process-manager' package is deprecated. Please migrate to Playwright's built-in test runner (@playwright/test) which now includes full Jest-style features and parallel testing. See https://playwright.dev/docs/intro for details.",
|
||||
"deprecated": "\u26a0\ufe0f The 'jest-process-manager' package is deprecated. Please migrate to Playwright's built-in test runner (@playwright/test) which now includes full Jest-style features and parallel testing. See https://playwright.dev/docs/intro for details.",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
@@ -27185,9 +27185,9 @@
|
||||
}
|
||||
},
|
||||
"node_modules/kdbush": {
|
||||
"version": "4.0.2",
|
||||
"resolved": "https://registry.npmjs.org/kdbush/-/kdbush-4.0.2.tgz",
|
||||
"integrity": "sha512-WbCVYJ27Sz8zi9Q7Q0xHC+05iwkm3Znipc2XTlrnJbsHMYktW4hPhXUE8Ys1engBrvffoSCqbil1JQAa7clRpA==",
|
||||
"version": "4.1.0",
|
||||
"resolved": "https://registry.npmjs.org/kdbush/-/kdbush-4.1.0.tgz",
|
||||
"integrity": "sha512-e9vurzrXJQrFX6ckpHP3bvj5l+9CnYzkxDNnNQ1h2QTqdWsUAJgXiKdGNcOa1EY85dU8KbQ+z/FdQdB7P+9yfQ==",
|
||||
"license": "ISC"
|
||||
},
|
||||
"node_modules/keyv": {
|
||||
@@ -43073,6 +43073,15 @@
|
||||
"node": ">=12"
|
||||
}
|
||||
},
|
||||
"packages/superset-ui-core/node_modules/dompurify": {
|
||||
"version": "3.4.13",
|
||||
"resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.13.tgz",
|
||||
"integrity": "sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ==",
|
||||
"license": "(MPL-2.0 OR Apache-2.0)",
|
||||
"optionalDependencies": {
|
||||
"@types/trusted-types": "^2.0.7"
|
||||
}
|
||||
},
|
||||
"packages/superset-ui-core/node_modules/react-ace": {
|
||||
"version": "14.0.1",
|
||||
"resolved": "https://registry.npmjs.org/react-ace/-/react-ace-14.0.1.tgz",
|
||||
@@ -43420,22 +43429,13 @@
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"d3": "^3.5.17",
|
||||
"prop-types": "^15.8.1",
|
||||
"react": "^19.2.7"
|
||||
"prop-types": "^15.8.1"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@apache-superset/core": "*",
|
||||
"@superset-ui/chart-controls": "*",
|
||||
"@superset-ui/core": "*"
|
||||
}
|
||||
},
|
||||
"plugins/plugin-chart-chord/node_modules/react": {
|
||||
"version": "19.2.8",
|
||||
"resolved": "https://registry.npmjs.org/react/-/react-19.2.8.tgz",
|
||||
"integrity": "sha512-PWaYA1L/q9u2u7xYQi+Y3L3Yfnie7XyLeaJICV1MGD6LprsBxcAqGjYyr0eY3p+QdsA+x/Irkt4Qif8D63+Sbw==",
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=0.10.0"
|
||||
"@superset-ui/core": "*",
|
||||
"react": "^18.3.0"
|
||||
}
|
||||
},
|
||||
"plugins/plugin-chart-country-map": {
|
||||
@@ -43659,7 +43659,7 @@
|
||||
"mapbox-gl": "^3.28.1",
|
||||
"maplibre-gl": "^5.24.0",
|
||||
"react-map-gl": "^8.1.2",
|
||||
"supercluster": "^8.0.1"
|
||||
"supercluster": "^9.0.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@apache-superset/core": "*",
|
||||
@@ -43669,6 +43669,15 @@
|
||||
"react-dom": "^18.3.0"
|
||||
}
|
||||
},
|
||||
"plugins/plugin-chart-point-cluster-map/node_modules/supercluster": {
|
||||
"version": "9.0.0",
|
||||
"resolved": "https://registry.npmjs.org/supercluster/-/supercluster-9.0.0.tgz",
|
||||
"integrity": "sha512-SaU8dQaxagTXr8+a1f67Pxg5CiRcZsq+LPQsZoV2r+FD8EF3Wpg5v7Zl8STPhml3oWHF1CQ8YaJGf8Uo8zS5qg==",
|
||||
"license": "ISC",
|
||||
"dependencies": {
|
||||
"kdbush": "^4.1.0"
|
||||
}
|
||||
},
|
||||
"plugins/plugin-chart-table": {
|
||||
"name": "@superset-ui/plugin-chart-table",
|
||||
"version": "0.20.3",
|
||||
|
||||
@@ -263,7 +263,7 @@
|
||||
"@storybook/test-runner": "0.24.4",
|
||||
"@svgr/webpack": "^8.1.0",
|
||||
"@swc/core": "^1.15.47",
|
||||
"@swc/plugin-emotion": "^14.15.0",
|
||||
"@swc/plugin-emotion": "^14.19.0",
|
||||
"@swc/plugin-transform-imports": "^12.5.0",
|
||||
"@testing-library/dom": "^10.4.1",
|
||||
"@testing-library/jest-dom": "^7.0.1",
|
||||
|
||||
@@ -130,6 +130,7 @@ export enum GenericDataType {
|
||||
String = 1,
|
||||
Temporal = 2,
|
||||
Boolean = 3,
|
||||
MultiValue = 4,
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+5
@@ -28,6 +28,7 @@ import {
|
||||
FieldBinaryOutlined,
|
||||
FieldStringOutlined,
|
||||
NumberOutlined,
|
||||
UnorderedListOutlined,
|
||||
} from '@ant-design/icons';
|
||||
import { Icons } from '@superset-ui/core/components';
|
||||
|
||||
@@ -72,6 +73,10 @@ export function ColumnTypeLabel({ type }: ColumnTypeLabelProps) {
|
||||
typeIcon = <FieldBinaryOutlined aria-label={t('boolean type icon')} />;
|
||||
} else if (type === GenericDataType.Temporal) {
|
||||
typeIcon = <ClockCircleOutlined aria-label={t('temporal type icon')} />;
|
||||
} else if (type === GenericDataType.MultiValue) {
|
||||
typeIcon = (
|
||||
<UnorderedListOutlined aria-label={t('multi-value type icon')} />
|
||||
);
|
||||
}
|
||||
|
||||
return <TypeIconWrapper>{typeIcon}</TypeIconWrapper>;
|
||||
|
||||
+17
@@ -64,4 +64,21 @@ describe('ColumnOption', () => {
|
||||
renderColumnTypeLabel({ type: GenericDataType.Temporal });
|
||||
expect(screen.getByLabelText('temporal type icon')).toBeVisible();
|
||||
});
|
||||
test('multi-value (array) type shows list icon', () => {
|
||||
renderColumnTypeLabel({ type: GenericDataType.MultiValue });
|
||||
expect(screen.getByLabelText('multi-value type icon')).toBeVisible();
|
||||
});
|
||||
});
|
||||
|
||||
describe('GenericDataType enum parity', () => {
|
||||
// These numeric values are shared with the backend enum in
|
||||
// superset/utils/core.py (GenericDataType). They must stay in sync because
|
||||
// the backend serializes columns using these integers.
|
||||
test('values match the backend contract', () => {
|
||||
expect(GenericDataType.Numeric).toBe(0);
|
||||
expect(GenericDataType.String).toBe(1);
|
||||
expect(GenericDataType.Temporal).toBe(2);
|
||||
expect(GenericDataType.Boolean).toBe(3);
|
||||
expect(GenericDataType.MultiValue).toBe(4);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -19,19 +19,71 @@
|
||||
import { t } from '@apache-superset/core/translation';
|
||||
import { sanitizeHtml } from './html';
|
||||
|
||||
export type TooltipTruncationMode = 'off' | 'end' | 'start' | 'middle';
|
||||
|
||||
export const TRUNCATION_MAX_CHARS = 40;
|
||||
|
||||
const TRUNCATION_STYLE = `
|
||||
max-width: 300px;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
`;
|
||||
|
||||
const NOWRAP_STYLE = `
|
||||
white-space: nowrap;
|
||||
`;
|
||||
|
||||
/**
|
||||
* Shortens plain text so a tooltip label stays readable, placing the ellipsis
|
||||
* where the caller asked for it.
|
||||
*
|
||||
* Only 'start' and 'middle' slice. 'end' is handled by CSS in tooltipHtml, and
|
||||
* 'off' means no truncation at all, so both return the input untouched.
|
||||
*
|
||||
* The input must be plain text. Callers are responsible for truncating before
|
||||
* any markup (such as the ECharts series marker) is prepended, and before
|
||||
* sanitization — slicing a string that already contains markup would cut into
|
||||
* a tag.
|
||||
*/
|
||||
export function truncateLabel(
|
||||
text: string,
|
||||
mode: TooltipTruncationMode = 'end',
|
||||
): string {
|
||||
if (
|
||||
(mode !== 'start' && mode !== 'middle') ||
|
||||
text.length <= TRUNCATION_MAX_CHARS
|
||||
) {
|
||||
return text;
|
||||
}
|
||||
const budget = TRUNCATION_MAX_CHARS - 1;
|
||||
if (mode === 'start') {
|
||||
return `…${text.slice(-budget)}`;
|
||||
}
|
||||
const head = Math.ceil(budget / 2);
|
||||
const tail = Math.floor(budget / 2);
|
||||
return `${text.slice(0, head)}…${text.slice(-tail)}`;
|
||||
}
|
||||
|
||||
function getTruncationStyle(mode: TooltipTruncationMode): string {
|
||||
if (mode === 'end') {
|
||||
return TRUNCATION_STYLE;
|
||||
}
|
||||
if (mode === 'off') {
|
||||
return '';
|
||||
}
|
||||
// 'start' and 'middle' are already sliced upstream; keep them on one line.
|
||||
return NOWRAP_STYLE;
|
||||
}
|
||||
|
||||
export function tooltipHtml(
|
||||
data: string[][],
|
||||
title?: string,
|
||||
focusedRow?: number,
|
||||
truncation: TooltipTruncationMode = 'end',
|
||||
) {
|
||||
const truncationStyle = getTruncationStyle(truncation);
|
||||
const titleRow = title
|
||||
? `<span style="font-weight: 700;${TRUNCATION_STYLE}">${title}</span>`
|
||||
? `<span style="font-weight: 700;${truncationStyle}">${title}</span>`
|
||||
: '';
|
||||
return sanitizeHtml(`
|
||||
<div>
|
||||
@@ -46,7 +98,7 @@ export function tooltipHtml(
|
||||
const cellStyle = `
|
||||
text-align: ${j > 0 ? 'right' : 'left'};
|
||||
padding-left: ${j === 0 ? 0 : 16}px;
|
||||
${TRUNCATION_STYLE}
|
||||
${truncationStyle}
|
||||
`;
|
||||
return `<td style="${cellStyle}">${cell}</td>`;
|
||||
});
|
||||
|
||||
@@ -16,7 +16,12 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { sanitizeHtml, tooltipHtml } from '@superset-ui/core';
|
||||
import {
|
||||
sanitizeHtml,
|
||||
tooltipHtml,
|
||||
truncateLabel,
|
||||
TRUNCATION_MAX_CHARS,
|
||||
} from '@superset-ui/core';
|
||||
|
||||
const TITLE_STYLE =
|
||||
'style="font-weight: 700;max-width:300px;overflow:hidden;text-overflow:ellipsis;"';
|
||||
@@ -182,3 +187,88 @@ test('should preserve table styling after sanitization (fixes ECharts tooltip fo
|
||||
expect(html).toContain('padding-left:16px');
|
||||
expect(html).toContain('max-width:300px');
|
||||
});
|
||||
|
||||
describe('truncateLabel', () => {
|
||||
const long = 'prod-us-east-1-service-checkout-latency-p99'; // 43 chars
|
||||
|
||||
test('returns text unchanged for off and end', () => {
|
||||
expect(truncateLabel(long, 'off')).toBe(long);
|
||||
expect(truncateLabel(long, 'end')).toBe(long);
|
||||
});
|
||||
|
||||
test('defaults to end, which does not slice', () => {
|
||||
expect(truncateLabel(long)).toBe(long);
|
||||
});
|
||||
|
||||
test('truncates the start, keeping the distinguishing suffix', () => {
|
||||
expect(truncateLabel(long, 'start')).toBe(
|
||||
'…-us-east-1-service-checkout-latency-p99',
|
||||
);
|
||||
expect(truncateLabel(long, 'start')).toHaveLength(TRUNCATION_MAX_CHARS);
|
||||
});
|
||||
|
||||
test('truncates the middle, keeping both ends', () => {
|
||||
expect(truncateLabel(long, 'middle')).toBe(
|
||||
'prod-us-east-1-servi…heckout-latency-p99',
|
||||
);
|
||||
expect(truncateLabel(long, 'middle')).toHaveLength(TRUNCATION_MAX_CHARS);
|
||||
});
|
||||
|
||||
test('leaves text at or under the limit untouched', () => {
|
||||
const atLimit = 'x'.repeat(TRUNCATION_MAX_CHARS);
|
||||
expect(truncateLabel(atLimit, 'start')).toBe(atLimit);
|
||||
expect(truncateLabel(atLimit, 'middle')).toBe(atLimit);
|
||||
expect(truncateLabel('short', 'start')).toBe('short');
|
||||
expect(truncateLabel('', 'middle')).toBe('');
|
||||
});
|
||||
|
||||
test('truncates text one character over the limit', () => {
|
||||
const overLimit = 'x'.repeat(TRUNCATION_MAX_CHARS + 1);
|
||||
expect(truncateLabel(overLimit, 'start')).toBe(
|
||||
`…${'x'.repeat(TRUNCATION_MAX_CHARS - 1)}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('tooltipHtml truncation modes', () => {
|
||||
const rows = [['label', 'value']];
|
||||
|
||||
// sanitizeHtml normalizes spacing inside style attributes, and it does so
|
||||
// differently across versions, so compare with whitespace stripped.
|
||||
const styles = (
|
||||
title: string | undefined,
|
||||
truncation?: 'off' | 'end' | 'start' | 'middle',
|
||||
) => removeWhitespaces(tooltipHtml(rows, title, undefined, truncation));
|
||||
|
||||
test('emits the 300px cap for end and for the default', () => {
|
||||
expect(styles('Title', 'end')).toContain('max-width:300px');
|
||||
expect(tooltipHtml(rows, 'Title')).toBe(
|
||||
tooltipHtml(rows, 'Title', undefined, 'end'),
|
||||
);
|
||||
});
|
||||
|
||||
test('emits no truncation style for off', () => {
|
||||
const html = styles('Title', 'off');
|
||||
expect(html).not.toContain('max-width');
|
||||
expect(html).not.toContain('text-overflow');
|
||||
expect(html).not.toContain('white-space');
|
||||
});
|
||||
|
||||
test.each(['start', 'middle'] as const)(
|
||||
'emits nowrap instead of a cap for %s',
|
||||
mode => {
|
||||
const html = styles('Title', mode);
|
||||
expect(html).toContain('white-space:nowrap');
|
||||
expect(html).not.toContain('max-width');
|
||||
},
|
||||
);
|
||||
|
||||
test('never slices cell text itself, whatever the mode', () => {
|
||||
const longCell = 'y'.repeat(TRUNCATION_MAX_CHARS + 20);
|
||||
(['off', 'end', 'start', 'middle'] as const).forEach(mode => {
|
||||
expect(tooltipHtml([[longCell]], undefined, undefined, mode)).toContain(
|
||||
longCell,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -30,12 +30,12 @@
|
||||
},
|
||||
"dependencies": {
|
||||
"d3": "^3.5.17",
|
||||
"prop-types": "^15.8.1",
|
||||
"react": "^19.2.7"
|
||||
"prop-types": "^15.8.1"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@apache-superset/core": "*",
|
||||
"@superset-ui/chart-controls": "*",
|
||||
"@superset-ui/core": "*"
|
||||
"@superset-ui/core": "*",
|
||||
"react": "^18.3.0"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -17,6 +17,10 @@
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
import { getNumberFormatter } from '@superset-ui/core';
|
||||
import { render, fireEvent } from '../../../../spec/helpers/testing-library';
|
||||
import BigNumberVis from './BigNumberViz';
|
||||
|
||||
/**
|
||||
* Tests for the color threshold formatter logic in BigNumberViz.
|
||||
*
|
||||
@@ -83,3 +87,33 @@ describe('BigNumberViz color formatters', () => {
|
||||
expect(getColorFromValue).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('BigNumberViz context menu', () => {
|
||||
test('invokes onContextMenu and stops the event bubbling to ancestor handlers', () => {
|
||||
const onContextMenu = jest.fn();
|
||||
const ancestorHandler = jest.fn();
|
||||
|
||||
const { container } = render(
|
||||
<div onContextMenu={ancestorHandler}>
|
||||
<BigNumberVis
|
||||
width={200}
|
||||
height={100}
|
||||
bigNumber={42}
|
||||
headerFormatter={getNumberFormatter()}
|
||||
headerFontSize={0.3}
|
||||
subheaderFontSize={0.125}
|
||||
subtitleFontSize={0.125}
|
||||
subtitle=""
|
||||
refs={{}}
|
||||
onContextMenu={onContextMenu}
|
||||
/>
|
||||
</div>,
|
||||
);
|
||||
|
||||
const headerLine = container.querySelector('.header-line');
|
||||
fireEvent.contextMenu(headerLine!, { clientX: 10, clientY: 20 });
|
||||
|
||||
expect(onContextMenu).toHaveBeenCalledWith(10, 20);
|
||||
expect(ancestorHandler).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -224,6 +224,7 @@ function BigNumberVis({
|
||||
const handleContextMenu = (e: MouseEvent<HTMLDivElement>) => {
|
||||
if (onContextMenu) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
onContextMenu(e.nativeEvent.clientX, e.nativeEvent.clientY);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -390,6 +390,7 @@ export default function transformProps(chartProps: EchartsGanttChartProps) {
|
||||
[GenericDataType.String]: undefined,
|
||||
[GenericDataType.Temporal]: tooltipTimeFormatter,
|
||||
[GenericDataType.Boolean]: undefined,
|
||||
[GenericDataType.MultiValue]: undefined,
|
||||
};
|
||||
|
||||
const echartOptions: EChartsCoreOption = {
|
||||
|
||||
+9
-1
@@ -40,6 +40,7 @@ import {
|
||||
TimeseriesChartDataResponseResult,
|
||||
TimeseriesDataRecord,
|
||||
tooltipHtml,
|
||||
truncateLabel,
|
||||
ValueFormatter,
|
||||
} from '@superset-ui/core';
|
||||
import { GenericDataType } from '@apache-superset/core/common';
|
||||
@@ -207,6 +208,7 @@ export default function transformProps(
|
||||
zoomable,
|
||||
richTooltip,
|
||||
tooltipSortByMetric,
|
||||
tooltipTruncation,
|
||||
xAxisBounds,
|
||||
xAxisLabelRotation,
|
||||
xAxisLabelInterval,
|
||||
@@ -907,13 +909,19 @@ export default function transformProps(
|
||||
formatter: primarySeries.has(key)
|
||||
? tooltipFormatter
|
||||
: tooltipFormatterSecondary,
|
||||
truncation: tooltipTruncation,
|
||||
});
|
||||
rows.push(row);
|
||||
if (key === focusedSeries) {
|
||||
focusedRow = rows.length - 1;
|
||||
}
|
||||
});
|
||||
return tooltipHtml(rows, tooltipFormatter(xValue), focusedRow);
|
||||
return tooltipHtml(
|
||||
rows,
|
||||
truncateLabel(tooltipFormatter(xValue), tooltipTruncation),
|
||||
focusedRow,
|
||||
tooltipTruncation,
|
||||
);
|
||||
},
|
||||
},
|
||||
legend: {
|
||||
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
ContributionType,
|
||||
TimeFormatter,
|
||||
AxisType,
|
||||
TooltipTruncationMode,
|
||||
} from '@superset-ui/core';
|
||||
import {
|
||||
BaseChartProps,
|
||||
@@ -59,6 +60,7 @@ export type EchartsMixedTimeseriesFormData = QueryFormData & {
|
||||
timeGrainSqla?: TimeGranularity;
|
||||
forceMaxInterval?: boolean;
|
||||
tooltipTimeFormat?: string;
|
||||
tooltipTruncation?: TooltipTruncationMode;
|
||||
zoomable: boolean;
|
||||
richTooltip: boolean;
|
||||
showQueryIdentifiers?: boolean;
|
||||
@@ -108,6 +110,7 @@ export const DEFAULT_FORM_DATA: EchartsMixedTimeseriesFormData = {
|
||||
yAxisFormatSecondary: TIMESERIES_DEFAULTS.yAxisFormat,
|
||||
yAxisTitleSecondary: DEFAULT_TITLE_FORM_DATA.yAxisTitle,
|
||||
tooltipTimeFormat: TIMESERIES_DEFAULTS.tooltipTimeFormat,
|
||||
tooltipTruncation: TIMESERIES_DEFAULTS.tooltipTruncation,
|
||||
xAxisBounds: TIMESERIES_DEFAULTS.xAxisBounds,
|
||||
xAxisForceCategorical: TIMESERIES_DEFAULTS.xAxisForceCategorical,
|
||||
xAxisTimeFormat: TIMESERIES_DEFAULTS.xAxisTimeFormat,
|
||||
|
||||
@@ -73,6 +73,7 @@ export const DEFAULT_FORM_DATA: EchartsTimeseriesFormData = {
|
||||
seriesType: EchartsTimeseriesSeriesType.Line,
|
||||
stack: false,
|
||||
tooltipTimeFormat: 'smart_date',
|
||||
tooltipTruncation: 'end',
|
||||
xAxisTimeFormat: 'smart_date',
|
||||
xAxisNumberFormat: 'SMART_NUMBER',
|
||||
truncateXAxis: true,
|
||||
|
||||
@@ -30,6 +30,7 @@ import {
|
||||
DTTM_ALIAS,
|
||||
ensureIsArray,
|
||||
tooltipHtml,
|
||||
truncateLabel,
|
||||
getCustomFormatter,
|
||||
getMetricLabel,
|
||||
getNumberFormatter,
|
||||
@@ -303,6 +304,7 @@ export default function transformProps(
|
||||
tooltipSortByMetric,
|
||||
showTooltipTotal,
|
||||
showTooltipPercentage,
|
||||
tooltipTruncation,
|
||||
truncateXAxis,
|
||||
truncateYAxis,
|
||||
xAxis: xAxisOrig,
|
||||
@@ -1449,6 +1451,7 @@ export default function transformProps(
|
||||
seriesName: key,
|
||||
formatter,
|
||||
marker,
|
||||
truncation: tooltipTruncation,
|
||||
});
|
||||
|
||||
const annotationRow = annotationLayers.some(
|
||||
@@ -1482,7 +1485,12 @@ export default function transformProps(
|
||||
}
|
||||
rows.push(totalRow);
|
||||
}
|
||||
return tooltipHtml(rows, tooltipFormatter(xValue), focusedRow);
|
||||
return tooltipHtml(
|
||||
rows,
|
||||
truncateLabel(tooltipFormatter(xValue), tooltipTruncation),
|
||||
focusedRow,
|
||||
tooltipTruncation,
|
||||
);
|
||||
},
|
||||
},
|
||||
legend: {
|
||||
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
QueryFormMetric,
|
||||
TimeFormatter,
|
||||
TimeGranularity,
|
||||
TooltipTruncationMode,
|
||||
} from '@superset-ui/core';
|
||||
import {
|
||||
BaseChartProps,
|
||||
@@ -82,6 +83,7 @@ export type EchartsTimeseriesFormData = QueryFormData & {
|
||||
tooltipTimeFormat?: string;
|
||||
showTooltipTotal?: boolean;
|
||||
showTooltipPercentage?: boolean;
|
||||
tooltipTruncation?: TooltipTruncationMode;
|
||||
truncateXAxis: boolean;
|
||||
truncateYAxis: boolean;
|
||||
yAxisFormat?: string;
|
||||
|
||||
@@ -315,6 +315,27 @@ const tooltipPercentageControl: ControlSetItem = {
|
||||
},
|
||||
};
|
||||
|
||||
const tooltipTruncationControl: ControlSetItem = {
|
||||
name: 'tooltipTruncation',
|
||||
config: {
|
||||
type: 'SelectControl',
|
||||
freeForm: false,
|
||||
label: t('Truncate labels'),
|
||||
renderTrigger: true,
|
||||
default: 'end',
|
||||
clearable: false,
|
||||
choices: [
|
||||
['off', t('Off')],
|
||||
['end', t('End')],
|
||||
['start', t('Start')],
|
||||
['middle', t('Middle')],
|
||||
],
|
||||
description: t(
|
||||
'Where to place the ellipsis when a tooltip label is too long. Choose Off to always show the full label, or Start when labels share a common prefix.',
|
||||
),
|
||||
},
|
||||
};
|
||||
|
||||
export const richTooltipSection: ControlSetRow[] = [
|
||||
[<ControlSubSectionHeader>{t('Tooltip')}</ControlSubSectionHeader>],
|
||||
[richTooltipControl],
|
||||
@@ -322,6 +343,7 @@ export const richTooltipSection: ControlSetRow[] = [
|
||||
[tooltipPercentageControl],
|
||||
[tooltipSortByMetricControl],
|
||||
[tooltipTimeFormatControl],
|
||||
[tooltipTruncationControl],
|
||||
];
|
||||
|
||||
const sortSeriesType: ControlSetItem = {
|
||||
|
||||
@@ -16,7 +16,13 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { DataRecord, DTTM_ALIAS, ValueFormatter } from '@superset-ui/core';
|
||||
import {
|
||||
DataRecord,
|
||||
DTTM_ALIAS,
|
||||
truncateLabel,
|
||||
TooltipTruncationMode,
|
||||
ValueFormatter,
|
||||
} from '@superset-ui/core';
|
||||
import type { OptionName, SeriesOption } from 'echarts/types/src/util/types';
|
||||
import type { TooltipMarker } from 'echarts/types/src/util/format';
|
||||
import {
|
||||
@@ -91,12 +97,16 @@ export const formatForecastTooltipSeries = ({
|
||||
forecastUpper,
|
||||
marker,
|
||||
formatter,
|
||||
truncation = 'end',
|
||||
}: ForecastValue & {
|
||||
seriesName: string;
|
||||
marker: TooltipMarker;
|
||||
formatter: ValueFormatter;
|
||||
truncation?: TooltipTruncationMode;
|
||||
}): string[] => {
|
||||
const name = `${marker}${sanitizeHtml(seriesName)}`;
|
||||
// Truncate before sanitizing and before the marker is prepended: slicing a
|
||||
// string that already contains markup would cut into the marker's tag.
|
||||
const name = `${marker}${sanitizeHtml(truncateLabel(seriesName, truncation))}`;
|
||||
let value = typeof observation === 'number' ? formatter(observation) : '';
|
||||
// Use finite-number checks rather than truthiness so that legitimate
|
||||
// zero values (e.g. a forecast that crosses zero, or a confidence bound of
|
||||
|
||||
+57
@@ -27,6 +27,7 @@ import {
|
||||
VizType,
|
||||
ChartDataResponseResult,
|
||||
TimeGranularity,
|
||||
TooltipTruncationMode,
|
||||
} from '@superset-ui/core';
|
||||
import { GenericDataType } from '@apache-superset/core/common';
|
||||
import {
|
||||
@@ -1295,3 +1296,59 @@ test('y-axis title position: non-Left sets nameLocation to end', () => {
|
||||
expect(yAxis[1].nameGap).toEqual(30);
|
||||
expect(yAxis[1].nameLocation).toEqual('end');
|
||||
});
|
||||
describe('EchartsMixedTimeseries tooltip truncation', () => {
|
||||
const longSeriesName = 'prod-us-east-1-service-checkout-latency-p99';
|
||||
const marker = '<span style="background-color:#1f77b4;"></span>';
|
||||
|
||||
const buildTooltip = (tooltipTruncation?: TooltipTruncationMode) => {
|
||||
const chartProps = createEchartsTimeseriesTestChartProps<
|
||||
EchartsMixedTimeseriesFormData,
|
||||
EchartsMixedTimeseriesProps
|
||||
>({
|
||||
...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
|
||||
defaultQueriesData: queriesData,
|
||||
formData: {
|
||||
...formData,
|
||||
...(tooltipTruncation ? { tooltipTruncation } : {}),
|
||||
},
|
||||
queriesData,
|
||||
});
|
||||
const { echartOptions } = transformProps(chartProps);
|
||||
const { formatter } = echartOptions.tooltip as {
|
||||
formatter: (params: unknown) => string;
|
||||
};
|
||||
// richTooltip is false in this fixture, so the trigger is 'item' and the
|
||||
// formatter receives a single param object rather than an array.
|
||||
return formatter({
|
||||
seriesId: longSeriesName,
|
||||
seriesName: longSeriesName,
|
||||
value: [599616000000, 1],
|
||||
marker,
|
||||
});
|
||||
};
|
||||
|
||||
test('keeps full text with the CSS cap by default', () => {
|
||||
const html = buildTooltip();
|
||||
expect(html.replace(/\s/g, '')).toContain('max-width:300px');
|
||||
expect(html).toContain(longSeriesName);
|
||||
});
|
||||
|
||||
test('removes the cap and keeps full text when off', () => {
|
||||
const html = buildTooltip('off');
|
||||
expect(html).not.toContain('max-width');
|
||||
expect(html).toContain(longSeriesName);
|
||||
});
|
||||
|
||||
test('drops the shared prefix when truncating from the start', () => {
|
||||
const html = buildTooltip('start');
|
||||
expect(html).not.toContain('prod-us-east');
|
||||
expect(html).toContain('latency-p99');
|
||||
expect(html).toContain('background-color:#1f77b4');
|
||||
});
|
||||
|
||||
test('keeps both ends when truncating the middle', () => {
|
||||
const html = buildTooltip('middle');
|
||||
expect(html).toContain('prod-us-east-1-servi…heckout-latency-p99');
|
||||
expect(html).not.toContain(longSeriesName);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -32,6 +32,7 @@ import {
|
||||
TimeseriesAnnotationLayer,
|
||||
ChartDataResponseResult,
|
||||
TimeGranularity,
|
||||
TooltipTruncationMode,
|
||||
} from '@superset-ui/core';
|
||||
import { GenericDataType } from '@apache-superset/core/common';
|
||||
import { supersetTheme } from '@apache-superset/core/theme';
|
||||
@@ -2437,3 +2438,94 @@ test('honors the snake_case flag the compare-chart migration stores in params',
|
||||
[BASE_TIMESTAMP + 300000000, 2],
|
||||
]);
|
||||
});
|
||||
describe('EchartsTimeseries tooltip truncation', () => {
|
||||
const longSeriesName = 'prod-us-east-1-service-checkout-latency-p99';
|
||||
const marker = '<span style="background-color:#1f77b4;"></span>';
|
||||
|
||||
const buildTooltip = (
|
||||
tooltipTruncation?: TooltipTruncationMode,
|
||||
xValue: string | number = 599616000000,
|
||||
) => {
|
||||
const chartProps = new ChartProps({
|
||||
formData: {
|
||||
colorScheme: 'bnbColors',
|
||||
datasource: '3__table',
|
||||
granularity_sqla: 'ds',
|
||||
metric: 'sum__num',
|
||||
groupby: ['foo'],
|
||||
viz_type: 'my_viz',
|
||||
...(tooltipTruncation ? { tooltipTruncation } : {}),
|
||||
} as SqlaFormData,
|
||||
width: 800,
|
||||
height: 600,
|
||||
queriesData: [
|
||||
{
|
||||
data: [
|
||||
{ [longSeriesName]: 1, __timestamp: 599616000000 },
|
||||
{ [longSeriesName]: 3, __timestamp: 599916000000 },
|
||||
],
|
||||
},
|
||||
],
|
||||
theme: supersetTheme,
|
||||
});
|
||||
const { echartOptions } = transformProps(
|
||||
chartProps as EchartsTimeseriesChartProps,
|
||||
);
|
||||
const { formatter } = echartOptions.tooltip as {
|
||||
formatter: (params: unknown) => string;
|
||||
};
|
||||
return formatter([
|
||||
{
|
||||
seriesId: longSeriesName,
|
||||
seriesName: longSeriesName,
|
||||
value: [xValue, 1],
|
||||
marker,
|
||||
},
|
||||
]);
|
||||
};
|
||||
|
||||
test('applies the CSS cap and keeps full text by default', () => {
|
||||
const html = buildTooltip();
|
||||
expect(html).toContain(longSeriesName);
|
||||
// sanitizeHtml normalizes spacing inside style attributes, so compare with
|
||||
// whitespace stripped rather than hard-coding one version's formatting.
|
||||
expect(html.replace(/\s/g, '')).toContain('max-width:300px');
|
||||
});
|
||||
|
||||
test('removes the cap and keeps full text when off', () => {
|
||||
const html = buildTooltip('off');
|
||||
expect(html).not.toContain('max-width');
|
||||
expect(html).toContain(longSeriesName);
|
||||
});
|
||||
|
||||
test('drops the shared prefix when truncating from the start', () => {
|
||||
const html = buildTooltip('start');
|
||||
expect(html).not.toContain('prod-us-east');
|
||||
expect(html).toContain('latency-p99');
|
||||
expect(html.replace(/\s/g, '')).toContain('white-space:nowrap');
|
||||
});
|
||||
|
||||
test('keeps both ends when truncating the middle', () => {
|
||||
const html = buildTooltip('middle');
|
||||
expect(html).toContain('prod-us-east-1-servi…heckout-latency-p99');
|
||||
expect(html).not.toContain(longSeriesName);
|
||||
});
|
||||
|
||||
test('preserves the echarts marker in every mode', () => {
|
||||
(['off', 'end', 'start', 'middle'] as const).forEach(mode => {
|
||||
expect(buildTooltip(mode)).toContain('background-color:#1f77b4');
|
||||
});
|
||||
});
|
||||
|
||||
test('truncates a long non-temporal x-axis title', () => {
|
||||
const longCategory = 'prod-us-east-1-service-checkout-cohort-2026';
|
||||
const html = buildTooltip('start', longCategory);
|
||||
expect(html).not.toContain(longCategory);
|
||||
expect(html).toContain('cohort-2026');
|
||||
});
|
||||
|
||||
test('leaves a long title alone in the default mode', () => {
|
||||
const longCategory = 'prod-us-east-1-service-checkout-cohort-2026';
|
||||
expect(buildTooltip(undefined, longCategory)).toContain(longCategory);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,7 +16,11 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { getNumberFormatter, NumberFormats } from '@superset-ui/core';
|
||||
import {
|
||||
getNumberFormatter,
|
||||
NumberFormats,
|
||||
TRUNCATION_MAX_CHARS,
|
||||
} from '@superset-ui/core';
|
||||
import { SeriesOption } from 'echarts';
|
||||
import {
|
||||
extractForecastSeriesContext,
|
||||
@@ -411,3 +415,52 @@ test('formatForecastTooltipSeries should skip non-finite forecast values', () =>
|
||||
}),
|
||||
).toEqual(['<img>qwerty', '10']);
|
||||
});
|
||||
|
||||
describe('formatForecastTooltipSeries truncation', () => {
|
||||
const marker =
|
||||
'<span style="display:inline-block;width:10px;height:10px;background-color:#1f77b4;"></span>';
|
||||
const longName = 'prod-us-east-1-service-checkout-latency-p99'; // 43 chars
|
||||
const intFormatter = getNumberFormatter(NumberFormats.INTEGER);
|
||||
|
||||
const format = (truncation?: 'off' | 'end' | 'start' | 'middle') =>
|
||||
formatForecastTooltipSeries({
|
||||
seriesName: longName,
|
||||
observation: 1,
|
||||
marker,
|
||||
formatter: intFormatter,
|
||||
...(truncation ? { truncation } : {}),
|
||||
})[0];
|
||||
|
||||
test('leaves the name intact by default and for off/end', () => {
|
||||
expect(format()).toContain(longName);
|
||||
expect(format('off')).toContain(longName);
|
||||
expect(format('end')).toContain(longName);
|
||||
});
|
||||
|
||||
test('slices the start of the name without harming the marker', () => {
|
||||
const cell = format('start');
|
||||
expect(cell).toContain(marker);
|
||||
expect(cell).toContain('…-us-east-1-service-checkout-latency-p99');
|
||||
expect(cell).not.toContain('prod-us-east');
|
||||
});
|
||||
|
||||
test('slices the middle of the name without harming the marker', () => {
|
||||
const cell = format('middle');
|
||||
expect(cell).toContain(marker);
|
||||
expect(cell).toContain('prod-us-east-1-servi…heckout-latency-p99');
|
||||
});
|
||||
|
||||
test('measures the budget against the name, not the marker markup', () => {
|
||||
// The marker alone is far longer than the budget. If truncation were
|
||||
// applied to the concatenated cell, a short name would be mangled.
|
||||
expect(marker.length).toBeGreaterThan(TRUNCATION_MAX_CHARS);
|
||||
const [cell] = formatForecastTooltipSeries({
|
||||
seriesName: 'cpu',
|
||||
observation: 1,
|
||||
marker,
|
||||
formatter: intFormatter,
|
||||
truncation: 'start',
|
||||
});
|
||||
expect(cell).toBe(`${marker}cpu`);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -33,7 +33,7 @@
|
||||
"mapbox-gl": "^3.28.1",
|
||||
"maplibre-gl": "^5.24.0",
|
||||
"react-map-gl": "^8.1.2",
|
||||
"supercluster": "^8.0.1"
|
||||
"supercluster": "^9.0.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@apache-superset/core": "*",
|
||||
|
||||
+5
@@ -17,6 +17,7 @@
|
||||
* under the License.
|
||||
*/
|
||||
import { useState } from 'react';
|
||||
import fetchMock from 'fetch-mock';
|
||||
import {
|
||||
cleanup,
|
||||
render,
|
||||
@@ -35,6 +36,10 @@ import { useDrillDetailMenuItems, DrillDetailMenuItemsProps } from './index';
|
||||
|
||||
/* eslint jest/expect-expect: ["warn", { "assertFunctionNames": ["expect*"] }] */
|
||||
|
||||
// Opening the context menu logs an event, and an unmatched request makes
|
||||
// fetch-mock throw inside the component.
|
||||
fetchMock.post('glob:*/log/?*', {});
|
||||
|
||||
jest.mock(
|
||||
'../DrillDetail/DrillDetailPane',
|
||||
() =>
|
||||
|
||||
+1
-3
@@ -1627,9 +1627,7 @@ function DatasourceEditor({
|
||||
{t(
|
||||
'Default URL to redirect to when accessing from the dataset list page. Accepts relative URLs such as',
|
||||
)}{' '}
|
||||
<Typography.Text code>
|
||||
/superset/dashboard/{'{id}'}/
|
||||
</Typography.Text>
|
||||
<Typography.Text code>/dashboard/{'{id}'}/</Typography.Text>
|
||||
</>
|
||||
}
|
||||
control={<TextControl controlId="default_endpoint" />}
|
||||
|
||||
+11
@@ -71,6 +71,17 @@ test('renders Tabs', async () => {
|
||||
expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('recommends a registered client route for the default URL', async () => {
|
||||
await asyncRender(createProps());
|
||||
|
||||
userEvent.click(screen.getByRole('tab', { name: 'Settings' }));
|
||||
|
||||
expect(await screen.findByText('/dashboard/{id}/')).toBeInTheDocument();
|
||||
expect(
|
||||
screen.queryByText('/superset/dashboard/{id}/'),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('can sync columns from source', async () => {
|
||||
const testProps = createProps();
|
||||
await asyncRender({
|
||||
|
||||
+37
-1
@@ -16,8 +16,20 @@
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import {
|
||||
ChartCustomizationType,
|
||||
type ChartCustomization,
|
||||
} from '@superset-ui/core';
|
||||
import { LabeledValue } from '@superset-ui/core/components';
|
||||
import { createLabelSortComparator } from './GroupByFilterCard';
|
||||
import { render, screen } from 'spec/helpers/testing-library';
|
||||
import GroupByFilterCard, {
|
||||
createLabelSortComparator,
|
||||
} from './GroupByFilterCard';
|
||||
|
||||
jest.mock('src/utils/cachedSupersetGet', () => ({
|
||||
// Never resolves, pinning the card in its column-loading state.
|
||||
cachedSupersetGet: jest.fn(() => new Promise(() => {})),
|
||||
}));
|
||||
|
||||
const apple: LabeledValue = { value: 'a', label: 'Apple' };
|
||||
const banana: LabeledValue = { value: 'b', label: 'Banana' };
|
||||
@@ -39,3 +51,27 @@ test('preserves source order when sortAscending is unset', () => {
|
||||
expect(compare(apple, banana)).toBe(0);
|
||||
expect(compare(banana, apple)).toBe(0);
|
||||
});
|
||||
|
||||
const groupByCustomization: ChartCustomization = {
|
||||
id: 'groupby-1',
|
||||
name: 'Group By',
|
||||
filterType: 'filter_groupby',
|
||||
type: ChartCustomizationType.ChartCustomization,
|
||||
targets: [{ datasetId: 1 }],
|
||||
scope: { rootPath: [], excluded: [] },
|
||||
controlValues: {},
|
||||
defaultDataMask: {},
|
||||
};
|
||||
|
||||
test('renders the column-loading spinner small and muted', async () => {
|
||||
render(<GroupByFilterCard customizationItem={groupByCustomization} />, {
|
||||
useRedux: true,
|
||||
initialState: {
|
||||
dataMask: {},
|
||||
nativeFilters: { filters: {} },
|
||||
},
|
||||
});
|
||||
const spinner = await screen.findByTestId('loading-indicator');
|
||||
expect(spinner).toHaveClass('inline');
|
||||
expect(spinner).toHaveStyle({ opacity: 0.25, width: '40px' });
|
||||
});
|
||||
|
||||
+1
-1
@@ -645,7 +645,7 @@ const GroupByFilterCard: FC<GroupByFilterCardProps> = ({
|
||||
|
||||
{loading && (
|
||||
<div style={{ textAlign: 'center', marginTop: 8 }}>
|
||||
<Loading position="inline" />
|
||||
<Loading position="inline" size="s" muted />
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
+10
@@ -72,3 +72,13 @@ test('omits datasourceType when undefined', () => {
|
||||
});
|
||||
expect(target).not.toHaveProperty('datasourceType');
|
||||
});
|
||||
|
||||
test('omits datasourceType when there is no dataset', () => {
|
||||
// The modal stamps a hidden ``datasourceType`` field on every filter form,
|
||||
// including dataset-less types. Without a dataset there is nothing for it to
|
||||
// describe, and emitting it would diverge from the ``{}`` target the import
|
||||
// and seed paths write.
|
||||
expect(
|
||||
buildNativeFilterTarget({ datasourceType: DatasourceType.Table }),
|
||||
).toEqual({});
|
||||
});
|
||||
|
||||
+8
-4
@@ -33,9 +33,9 @@ export interface TargetFormInputs {
|
||||
* Build the ``NativeFilterTarget`` carried by a native filter or chart
|
||||
* customization from its form inputs.
|
||||
*
|
||||
* Consolidates what used to live in three places — ``filterTransformer``,
|
||||
* ``customizationTransformer``, and ``createHandleSave`` — so changes to the
|
||||
* target shape only need to happen here.
|
||||
* Consolidates what used to live in ``filterTransformer`` and
|
||||
* ``customizationTransformer`` so changes to the target shape only need to
|
||||
* happen here.
|
||||
*/
|
||||
export function buildNativeFilterTarget(
|
||||
formInputs: TargetFormInputs,
|
||||
@@ -49,7 +49,11 @@ export function buildNativeFilterTarget(
|
||||
: formInputs.dataset;
|
||||
}
|
||||
|
||||
if (formInputs.datasourceType) {
|
||||
// ``datasourceType`` describes the selected dataset, so it only belongs on a
|
||||
// target that has one. Emitting it for a dataset-less filter (e.g.
|
||||
// ``filter_time``) would make a UI save serialize a target the import and
|
||||
// seed paths write as ``{}``.
|
||||
if (formInputs.dataset != null && formInputs.datasourceType) {
|
||||
target.datasourceType = formInputs.datasourceType;
|
||||
}
|
||||
|
||||
|
||||
+102
@@ -0,0 +1,102 @@
|
||||
/**
|
||||
* Licensed to the Apache Software Foundation (ASF) under one
|
||||
* or more contributor license agreements. See the NOTICE file
|
||||
* distributed with this work for additional information
|
||||
* regarding copyright ownership. The ASF licenses this file
|
||||
* to you under the Apache License, Version 2.0 (the
|
||||
* "License"); you may not use this file except in compliance
|
||||
* with the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { ChartCustomization, ChartCustomizationType } from '@superset-ui/core';
|
||||
import { ChartCustomizationsFormItem } from '../types';
|
||||
import { transformCustomizationForSave } from './customizationTransformer';
|
||||
|
||||
const baseFormItem = {
|
||||
type: ChartCustomizationType.ChartCustomization,
|
||||
scope: { rootPath: ['ROOT_ID'], excluded: [] },
|
||||
controlValues: {},
|
||||
requiredFirst: {},
|
||||
defaultValue: null,
|
||||
defaultDataMask: { filterState: {}, extraFormData: {} },
|
||||
sortMetric: null,
|
||||
description: '',
|
||||
// form-only field that must never leak into the saved customization
|
||||
defaultValueQueriesData: null,
|
||||
} as unknown as ChartCustomizationsFormItem;
|
||||
|
||||
test('serializes a dataset-less customization into a full ChartCustomization', () => {
|
||||
// Customization plugins declaring ``datasourceCount: 0`` render no dataset
|
||||
// control, so their form item carries neither ``dataset`` nor ``targets``.
|
||||
const formItem = {
|
||||
...baseFormItem,
|
||||
name: 'Layer visibility',
|
||||
filterType: 'customization_deckgl_layer_visibility',
|
||||
} as unknown as ChartCustomizationsFormItem;
|
||||
|
||||
const result = transformCustomizationForSave(
|
||||
'CHART_CUSTOMIZATION-abc',
|
||||
formItem,
|
||||
) as ChartCustomization;
|
||||
|
||||
expect(result.targets).toEqual([{}]);
|
||||
expect(result.defaultDataMask).toBeDefined();
|
||||
expect(result.removed).toBe(false);
|
||||
expect(result).not.toHaveProperty('defaultValueQueriesData');
|
||||
});
|
||||
|
||||
test('serializes a dataset-backed customization into a full ChartCustomization', () => {
|
||||
const formItem = {
|
||||
...baseFormItem,
|
||||
name: 'Group by',
|
||||
filterType: 'customization_dynamic_group_by',
|
||||
dataset: { value: 42, label: 'sales' },
|
||||
column: 'region',
|
||||
} as unknown as ChartCustomizationsFormItem;
|
||||
|
||||
const result = transformCustomizationForSave(
|
||||
'CHART_CUSTOMIZATION-def',
|
||||
formItem,
|
||||
) as ChartCustomization;
|
||||
|
||||
expect(result.targets).toEqual([
|
||||
{ datasetId: 42, column: { name: 'region' } },
|
||||
]);
|
||||
expect(result).not.toHaveProperty('defaultValueQueriesData');
|
||||
});
|
||||
|
||||
test('passes an already-saved ChartCustomization through untouched', () => {
|
||||
const saved: ChartCustomization = {
|
||||
id: 'CHART_CUSTOMIZATION-ghi',
|
||||
name: 'Group by',
|
||||
filterType: 'customization_dynamic_group_by',
|
||||
type: ChartCustomizationType.ChartCustomization,
|
||||
targets: [{ datasetId: 42, column: { name: 'region' } }],
|
||||
defaultDataMask: { filterState: {}, extraFormData: {} },
|
||||
controlValues: {},
|
||||
scope: { rootPath: ['ROOT_ID'], excluded: [] },
|
||||
description: ' needs trim ',
|
||||
chartsInScope: [1, 2],
|
||||
tabsInScope: ['TAB-1'],
|
||||
};
|
||||
|
||||
const result = transformCustomizationForSave(
|
||||
'CHART_CUSTOMIZATION-ghi',
|
||||
saved,
|
||||
) as ChartCustomization;
|
||||
|
||||
expect(result.targets).toEqual([
|
||||
{ datasetId: 42, column: { name: 'region' } },
|
||||
]);
|
||||
expect(result.chartsInScope).toEqual([1, 2]);
|
||||
expect(result.tabsInScope).toEqual(['TAB-1']);
|
||||
expect(result.description).toBe('needs trim');
|
||||
});
|
||||
+4
-1
@@ -69,7 +69,10 @@ function isDividerType(
|
||||
function isFormInput(
|
||||
formInputs: ChartCustomizationFormOrSaved,
|
||||
): formInputs is ChartCustomizationsFormItem {
|
||||
return 'dataset' in formInputs && typeof formInputs.dataset === 'object';
|
||||
// Mirrors `filterTransformer`: a saved customization always carries a
|
||||
// serialized `targets` array, and dataset-less types (e.g. the deck.gl layer
|
||||
// visibility customization) have no `dataset` to discriminate on.
|
||||
return !('targets' in formInputs);
|
||||
}
|
||||
|
||||
function transformCustomizationDivider(
|
||||
|
||||
+156
@@ -0,0 +1,156 @@
|
||||
/**
|
||||
* Licensed to the Apache Software Foundation (ASF) under one
|
||||
* or more contributor license agreements. See the NOTICE file
|
||||
* distributed with this work for additional information
|
||||
* regarding copyright ownership. The ASF licenses this file
|
||||
* to you under the Apache License, Version 2.0 (the
|
||||
* "License"); you may not use this file except in compliance
|
||||
* with the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { DatasourceType, Filter, NativeFilterType } from '@superset-ui/core';
|
||||
import { NativeFiltersFormItem } from '../types';
|
||||
import { transformFilterForSave } from './filterTransformer';
|
||||
|
||||
const baseFormItem = {
|
||||
type: NativeFilterType.NativeFilter,
|
||||
scope: { rootPath: ['ROOT_ID'], excluded: [] },
|
||||
controlValues: {},
|
||||
requiredFirst: {},
|
||||
defaultValue: null,
|
||||
defaultDataMask: { filterState: {}, extraFormData: {} },
|
||||
description: '',
|
||||
// form-only fields that must never leak into the saved filter
|
||||
defaultValueQueriesData: null,
|
||||
} as unknown as NativeFiltersFormItem;
|
||||
|
||||
test('serializes a dataset-less filter (filter_time) into a full Filter', () => {
|
||||
// A ``filter_time`` filter has no dataset/column controls, so its form item
|
||||
// carries neither a ``dataset`` nor a ``targets`` key. It must still be
|
||||
// transformed like any other native filter rather than persisted verbatim.
|
||||
const formItem: NativeFiltersFormItem = {
|
||||
...baseFormItem,
|
||||
name: 'Time Range',
|
||||
filterType: 'filter_time',
|
||||
dependencies: ['NATIVE_FILTER-parent'],
|
||||
// the modal stamps this on every filter form, dataset or not
|
||||
datasourceType: DatasourceType.Table,
|
||||
};
|
||||
|
||||
const result = transformFilterForSave(
|
||||
'NATIVE_FILTER-abc',
|
||||
formItem,
|
||||
) as Filter;
|
||||
|
||||
// Keys the bug used to strip are present and well-formed. The target matches
|
||||
// the ``{}`` the import and seed paths write, so one logical filter has one
|
||||
// serialization regardless of provenance.
|
||||
expect(result.targets).toEqual([{}]);
|
||||
expect(result.defaultDataMask).toBeDefined();
|
||||
expect(result.cascadeParentIds).toEqual(['NATIVE_FILTER-parent']);
|
||||
|
||||
// Form-only keys must not leak into the persisted config.
|
||||
expect(result).not.toHaveProperty('defaultValueQueriesData');
|
||||
expect(result).not.toHaveProperty('dependencies');
|
||||
// Empty requiredFirst collapses to undefined instead of the raw form object.
|
||||
expect(result.requiredFirst).toBeUndefined();
|
||||
|
||||
// A dataset-less filter has no sort metric control, so the persisted document
|
||||
// must not gain a ``sortMetric`` key it never had. Asserted on the serialized
|
||||
// form because ``undefined`` values survive in the object but not in JSON.
|
||||
expect(JSON.parse(JSON.stringify(result))).not.toHaveProperty('sortMetric');
|
||||
|
||||
expect(result.name).toBe('Time Range');
|
||||
expect(result.filterType).toBe('filter_time');
|
||||
});
|
||||
|
||||
test('serializes a dataset-backed filter (filter_select) into a full Filter', () => {
|
||||
const formItem: NativeFiltersFormItem = {
|
||||
...baseFormItem,
|
||||
name: 'Region',
|
||||
filterType: 'filter_select',
|
||||
dataset: { value: 42, label: 'sales' },
|
||||
column: 'region',
|
||||
dependencies: [],
|
||||
};
|
||||
|
||||
const result = transformFilterForSave(
|
||||
'NATIVE_FILTER-def',
|
||||
formItem,
|
||||
) as Filter;
|
||||
|
||||
expect(result.targets).toEqual([
|
||||
{ datasetId: 42, column: { name: 'region' } },
|
||||
]);
|
||||
expect(result.defaultDataMask).toBeDefined();
|
||||
expect(result.cascadeParentIds).toEqual([]);
|
||||
expect(result).not.toHaveProperty('defaultValueQueriesData');
|
||||
});
|
||||
|
||||
test('passes an already-saved Filter through untouched (aside from trimming)', () => {
|
||||
// Values coming from the stored filter config map (e.g. cascade-parent
|
||||
// cleanup) already carry a ``targets`` array and must be preserved as-is.
|
||||
const savedFilter: Filter = {
|
||||
id: 'NATIVE_FILTER-ghi',
|
||||
name: 'Time Range',
|
||||
filterType: 'filter_time',
|
||||
type: NativeFilterType.NativeFilter,
|
||||
targets: [{}],
|
||||
defaultDataMask: { filterState: {}, extraFormData: {} },
|
||||
cascadeParentIds: ['NATIVE_FILTER-parent'],
|
||||
controlValues: {},
|
||||
scope: { rootPath: ['ROOT_ID'], excluded: [] },
|
||||
description: ' needs trim ',
|
||||
chartsInScope: [1, 2],
|
||||
tabsInScope: ['TAB-1'],
|
||||
};
|
||||
|
||||
const result = transformFilterForSave(
|
||||
'NATIVE_FILTER-ghi',
|
||||
savedFilter,
|
||||
) as Filter;
|
||||
|
||||
expect(result.targets).toEqual([{}]);
|
||||
expect(result.cascadeParentIds).toEqual(['NATIVE_FILTER-parent']);
|
||||
expect(result.chartsInScope).toEqual([1, 2]);
|
||||
expect(result.tabsInScope).toEqual(['TAB-1']);
|
||||
expect(result.description).toBe('needs trim');
|
||||
});
|
||||
|
||||
test('rebuilds a saved filter whose targets were already stripped', () => {
|
||||
// Dashboards affected by this bug hold ``filter_time`` entries with no
|
||||
// ``targets``. They no longer match the saved-filter branch, so they take the
|
||||
// form-item path and are repaired on the next save. ``cascadeParentIds`` is
|
||||
// read from the form's ``dependencies``, which such an entry does not carry —
|
||||
// the same write that stripped ``targets`` stripped ``cascadeParentIds`` too.
|
||||
const strippedFilter = {
|
||||
id: 'NATIVE_FILTER-jkl',
|
||||
name: 'Time Range',
|
||||
filterType: 'filter_time',
|
||||
type: NativeFilterType.NativeFilter,
|
||||
scope: { rootPath: ['ROOT_ID'], excluded: [] },
|
||||
controlValues: { timeShift: false },
|
||||
description: '',
|
||||
requiredFirst: { 'NATIVE_FILTER-jkl': true },
|
||||
defaultValueQueriesData: null,
|
||||
} as unknown as NativeFiltersFormItem;
|
||||
|
||||
const result = transformFilterForSave(
|
||||
'NATIVE_FILTER-jkl',
|
||||
strippedFilter,
|
||||
) as Filter;
|
||||
|
||||
expect(result.targets).toEqual([{}]);
|
||||
expect(result.defaultDataMask).toBeDefined();
|
||||
expect(result.requiredFirst).toBe(true);
|
||||
expect(result.cascadeParentIds).toEqual([]);
|
||||
expect(result).not.toHaveProperty('defaultValueQueriesData');
|
||||
});
|
||||
+5
-2
@@ -67,7 +67,10 @@ function isDividerType(
|
||||
function isFormInput(
|
||||
formInputs: NativeFilterFormOrSaved,
|
||||
): formInputs is NativeFiltersFormItem {
|
||||
return 'dataset' in formInputs;
|
||||
// A saved filter always carries a serialized `targets` array; a form item
|
||||
// never does. Keying this off `dataset` misclassified filter types with no
|
||||
// dataset control (e.g. `filter_time`) as already saved.
|
||||
return !('targets' in formInputs);
|
||||
}
|
||||
|
||||
function transformDivider(
|
||||
@@ -115,7 +118,7 @@ function transformFormInput(
|
||||
adhoc_filters: formInputs.adhoc_filters,
|
||||
time_range: formInputs.time_range,
|
||||
granularity_sqla: formInputs.granularity_sqla,
|
||||
sortMetric: formInputs.sortMetric ?? null,
|
||||
sortMetric: formInputs.sortMetric,
|
||||
requiredFirst: formInputs.requiredFirst
|
||||
? Object.values(formInputs.requiredFirst).find(rf => rf)
|
||||
: undefined,
|
||||
|
||||
@@ -18,21 +18,15 @@
|
||||
*/
|
||||
import type { FormInstance } from '@superset-ui/core/components';
|
||||
import { nanoid } from 'nanoid';
|
||||
import { getInitialDataMask } from 'src/dataMask/reducer';
|
||||
import {
|
||||
FilterConfiguration,
|
||||
NativeFilterType,
|
||||
NativeFilterTarget,
|
||||
Filter,
|
||||
Divider,
|
||||
ChartCustomizationType,
|
||||
ChartCustomizationConfiguration,
|
||||
ChartCustomization,
|
||||
ChartCustomizationDivider,
|
||||
} from '@superset-ui/core';
|
||||
import { logging } from '@apache-superset/core/utils';
|
||||
import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants';
|
||||
import { buildNativeFilterTarget } from './transformers/buildTarget';
|
||||
import {
|
||||
ChartCustomizationsForm,
|
||||
FilterChangesType,
|
||||
@@ -101,70 +95,6 @@ export const validateForm = async (
|
||||
}
|
||||
};
|
||||
|
||||
export const createHandleSave =
|
||||
(
|
||||
saveForm: Function,
|
||||
filterChanges: FilterChangesType,
|
||||
values: NativeFiltersForm,
|
||||
filterConfigMap: Record<string, Filter | Divider>,
|
||||
) =>
|
||||
async () => {
|
||||
const transformFilter = (id: string) => {
|
||||
const formInputs = values.filters?.[id] || filterConfigMap[id];
|
||||
if (!formInputs) {
|
||||
return undefined;
|
||||
}
|
||||
if (formInputs.type === NativeFilterType.Divider) {
|
||||
return {
|
||||
id,
|
||||
type: NativeFilterType.Divider,
|
||||
scope: {
|
||||
rootPath: [DASHBOARD_ROOT_ID],
|
||||
excluded: [],
|
||||
},
|
||||
title: formInputs.title,
|
||||
description: formInputs.description,
|
||||
};
|
||||
}
|
||||
|
||||
const target: Partial<NativeFilterTarget> =
|
||||
buildNativeFilterTarget(formInputs);
|
||||
|
||||
return {
|
||||
id,
|
||||
adhoc_filters: formInputs.adhoc_filters,
|
||||
time_range: formInputs.time_range,
|
||||
controlValues: formInputs.controlValues ?? {},
|
||||
granularity_sqla: formInputs.granularity_sqla,
|
||||
...(formInputs.time_grains?.length
|
||||
? { time_grains: formInputs.time_grains }
|
||||
: {}),
|
||||
requiredFirst: Object.values(formInputs.requiredFirst ?? {}).find(
|
||||
rf => rf,
|
||||
),
|
||||
name: formInputs.name,
|
||||
filterType: formInputs.filterType,
|
||||
targets: [target],
|
||||
defaultDataMask: formInputs.defaultDataMask ?? getInitialDataMask(),
|
||||
cascadeParentIds: formInputs.dependencies || [],
|
||||
scope: formInputs.scope,
|
||||
sortMetric: formInputs.sortMetric,
|
||||
type: formInputs.type,
|
||||
description: (formInputs.description || '').trim(),
|
||||
};
|
||||
};
|
||||
|
||||
const transformedModified = filterChanges.modified
|
||||
.map(transformFilter)
|
||||
.filter(Boolean);
|
||||
|
||||
const newFilterChanges = {
|
||||
...filterChanges,
|
||||
modified: transformedModified,
|
||||
};
|
||||
await saveForm(newFilterChanges);
|
||||
};
|
||||
|
||||
export const createHandleRemoveItem =
|
||||
(
|
||||
setRemovedFilters: (
|
||||
|
||||
@@ -251,4 +251,11 @@ export const DEFAULT_CONFIG_FORM_LAYOUT: ColumnConfigFormLayout = {
|
||||
{ name: 'horizontalAlign', override: { defaultValue: 'left' } },
|
||||
],
|
||||
],
|
||||
[GenericDataType.MultiValue]: [
|
||||
[
|
||||
'columnWidth',
|
||||
{ name: 'horizontalAlign', override: { defaultValue: 'left' } },
|
||||
],
|
||||
['truncateLongCells'],
|
||||
],
|
||||
};
|
||||
|
||||
+68
@@ -270,6 +270,74 @@ describe('AdhocFilter', () => {
|
||||
});
|
||||
expect(adhocFilter.comparator).toBe(undefined);
|
||||
});
|
||||
// Charts saved before #32701 persisted `==` as the operation for IS_TRUE and
|
||||
// IS_FALSE, alongside a boolean comparator. `translateToSql` and the backend
|
||||
// both key off `operator`, so dropping the comparator would render such a
|
||||
// filter as `col =` and query it as `col IS NULL`.
|
||||
test('keeps the legacy boolean comparator for IS_TRUE', () => {
|
||||
const adhocFilter = new AdhocFilter({
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
subject: 'col',
|
||||
operator: '==',
|
||||
operatorId: Operators.IsTrue,
|
||||
comparator: true,
|
||||
clause: Clauses.Where,
|
||||
});
|
||||
expect(adhocFilter.operator).toBe('==');
|
||||
expect(adhocFilter.comparator).toBe(true);
|
||||
expect(adhocFilter.translateToSql()).toBe("col = 'TRUE'");
|
||||
});
|
||||
test('keeps the legacy boolean comparator for IS_FALSE', () => {
|
||||
const adhocFilter = new AdhocFilter({
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
subject: 'col',
|
||||
operator: '==',
|
||||
operatorId: Operators.IsFalse,
|
||||
comparator: false,
|
||||
clause: Clauses.Where,
|
||||
});
|
||||
expect(adhocFilter.operator).toBe('==');
|
||||
expect(adhocFilter.comparator).toBe(false);
|
||||
expect(adhocFilter.translateToSql()).toBe("col = 'FALSE'");
|
||||
});
|
||||
test('restores the boolean even when the stored comparator is missing', () => {
|
||||
const adhocFilter = new AdhocFilter({
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
subject: 'col',
|
||||
operator: '==',
|
||||
operatorId: Operators.IsTrue,
|
||||
clause: Clauses.Where,
|
||||
});
|
||||
expect(adhocFilter.comparator).toBe(true);
|
||||
});
|
||||
test('keeps a legacy boolean filter intact when the control re-posts it', () => {
|
||||
const stored = {
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
subject: 'col',
|
||||
operator: '==',
|
||||
operatorId: Operators.IsTrue,
|
||||
comparator: true,
|
||||
clause: Clauses.Where,
|
||||
};
|
||||
// DndFilterSelect wraps props.value and hands those instances to onChange
|
||||
const posted = JSON.parse(JSON.stringify(new AdhocFilter(stored)));
|
||||
expect(posted.operator).toBe('==');
|
||||
expect(posted.comparator).toBe(true);
|
||||
expect(posted.operatorId).toBe(Operators.IsTrue);
|
||||
});
|
||||
test('leaves a genuine equality filter on a boolean value alone', () => {
|
||||
const adhocFilter = new AdhocFilter({
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
subject: 'col',
|
||||
operator: '==',
|
||||
operatorId: Operators.Equals,
|
||||
comparator: true,
|
||||
clause: Clauses.Where,
|
||||
});
|
||||
expect(adhocFilter.operator).toBe('==');
|
||||
expect(adhocFilter.comparator).toBe(true);
|
||||
expect(adhocFilter.translateToSql()).toBe("col = 'TRUE'");
|
||||
});
|
||||
test('sets the label properly if subject is a string', () => {
|
||||
const adhocFilter = new AdhocFilter({
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
|
||||
@@ -30,6 +30,15 @@ const CUSTOM_OPERATIONS = [...CUSTOM_OPERATORS].map(
|
||||
op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation,
|
||||
);
|
||||
|
||||
// Charts saved before #32701 store `==` for IS_TRUE/IS_FALSE with the boolean
|
||||
// in the comparator; blanking it makes them query `col IS NULL`. Restoring it
|
||||
// leaves the emitted SQL untouched -- reconciling `operator` to `IS TRUE`
|
||||
// would not, and Druid rejects that predicate on VARCHAR columns.
|
||||
const LEGACY_BOOLEAN_COMPARATORS = new Map<string, boolean>([
|
||||
[Operators.IsTrue, true],
|
||||
[Operators.IsFalse, false],
|
||||
]);
|
||||
|
||||
interface AdhocFilterInput {
|
||||
expressionType?: string;
|
||||
subject?: string | { column_name?: string; [key: string]: unknown } | null;
|
||||
@@ -77,6 +86,16 @@ export default class AdhocFilter {
|
||||
) {
|
||||
this.comparator = undefined;
|
||||
}
|
||||
if (
|
||||
this.operator ===
|
||||
OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.Equals].operation &&
|
||||
adhocFilter.operatorId &&
|
||||
LEGACY_BOOLEAN_COMPARATORS.has(adhocFilter.operatorId)
|
||||
) {
|
||||
this.comparator = LEGACY_BOOLEAN_COMPARATORS.get(
|
||||
adhocFilter.operatorId,
|
||||
);
|
||||
}
|
||||
this.clause = adhocFilter.clause || Clauses.Where;
|
||||
this.sqlExpression = null;
|
||||
} else if (this.expressionType === ExpressionTypes.Sql) {
|
||||
|
||||
+16
-2
@@ -367,8 +367,22 @@ function AdhocFilterEditPopover({
|
||||
</ErrorBoundary>
|
||||
),
|
||||
},
|
||||
...(datasource?.type === 'semantic_view'
|
||||
? []
|
||||
...(datasource?.type === 'semantic_view' ||
|
||||
[
|
||||
Operators.ContainsAny,
|
||||
Operators.ContainsAll,
|
||||
Operators.IsEmpty,
|
||||
Operators.IsNotEmpty,
|
||||
Operators.LengthEquals,
|
||||
Operators.LengthGreaterThan,
|
||||
Operators.LengthLessThan,
|
||||
Operators.LengthGreaterThanOrEqual,
|
||||
Operators.LengthLessThanOrEqual,
|
||||
].includes(adhocFilter.operatorId as Operators)
|
||||
? // Hide the Custom SQL tab for element-level array operators: they
|
||||
// have no portable SQL representation, and converting one would
|
||||
// silently turn the filter into invalid raw SQL.
|
||||
[]
|
||||
: [
|
||||
{
|
||||
key: ExpressionTypes.Sql,
|
||||
|
||||
+138
@@ -35,6 +35,7 @@ import {
|
||||
} from 'src/explore/constants';
|
||||
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
|
||||
import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core';
|
||||
import { GenericDataType } from '@apache-superset/core/common';
|
||||
import fetchMock from 'fetch-mock';
|
||||
|
||||
import { TestDataset, Dataset } from '@superset-ui/chart-controls';
|
||||
@@ -252,6 +253,78 @@ test('shows boolean only operators when subject is number', () => {
|
||||
].map(operator => expect(isOperatorRelevant(operator, 'value')).toBe(true));
|
||||
});
|
||||
|
||||
test('shows array operators (tier 1 + tier 2) when subject is multi-value', () => {
|
||||
const props = setup({
|
||||
adhocFilter: new AdhocFilter({
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
subject: 'skills',
|
||||
operatorId: undefined,
|
||||
operator: undefined,
|
||||
comparator: undefined,
|
||||
clause: undefined,
|
||||
}),
|
||||
datasource: {
|
||||
columns: [
|
||||
{
|
||||
id: 3,
|
||||
column_name: 'skills',
|
||||
type: 'Array(String)',
|
||||
type_generic: GenericDataType.MultiValue,
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
const { isOperatorRelevant } = useSimpleTabFilterProps(
|
||||
props as unknown as Props,
|
||||
);
|
||||
// Tier 1 (whole-array) + Tier 2 (element-level) are all relevant.
|
||||
[
|
||||
Operators.Equals,
|
||||
Operators.NotEquals,
|
||||
Operators.In,
|
||||
Operators.NotIn,
|
||||
Operators.IsNull,
|
||||
Operators.IsNotNull,
|
||||
Operators.ContainsAny,
|
||||
Operators.ContainsAll,
|
||||
Operators.IsEmpty,
|
||||
Operators.IsNotEmpty,
|
||||
].forEach(operator =>
|
||||
expect(isOperatorRelevant(operator, 'skills')).toBe(true),
|
||||
);
|
||||
// scalar-only operators are hidden for array columns
|
||||
[Operators.GreaterThan, Operators.LessThan, Operators.Like].forEach(
|
||||
operator => expect(isOperatorRelevant(operator, 'skills')).toBe(false),
|
||||
);
|
||||
});
|
||||
|
||||
test('hides element-level array operators for non multi-value columns', () => {
|
||||
const props = setup({
|
||||
adhocFilter: new AdhocFilter({
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
subject: 'value',
|
||||
operatorId: undefined,
|
||||
operator: undefined,
|
||||
comparator: undefined,
|
||||
clause: undefined,
|
||||
}),
|
||||
datasource: {
|
||||
columns: [{ id: 3, column_name: 'value', type: 'STRING' }],
|
||||
},
|
||||
});
|
||||
const { isOperatorRelevant } = useSimpleTabFilterProps(
|
||||
props as unknown as Props,
|
||||
);
|
||||
[
|
||||
Operators.ContainsAny,
|
||||
Operators.ContainsAll,
|
||||
Operators.IsEmpty,
|
||||
Operators.IsNotEmpty,
|
||||
].forEach(operator =>
|
||||
expect(isOperatorRelevant(operator, 'value')).toBe(false),
|
||||
);
|
||||
});
|
||||
|
||||
test('will convert from individual comparator to array if the operator changes to multi', () => {
|
||||
const props = setup();
|
||||
const { onOperatorChange } = useSimpleTabFilterProps(
|
||||
@@ -309,6 +382,49 @@ test('will convert from array to individual comparators if the operator changes
|
||||
);
|
||||
});
|
||||
|
||||
test('resets the comparator when switching between array value families', () => {
|
||||
// Equal to (whole-array literal) -> Contains all (individual elements):
|
||||
// the value spaces are incompatible, so the stale value must be cleared.
|
||||
const wholeArrayFilter = new AdhocFilter({
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
subject: 'scores',
|
||||
operatorId: Operators.Equals,
|
||||
operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.Equals].operation,
|
||||
comparator: '[5,6,7]',
|
||||
clause: Clauses.Where,
|
||||
});
|
||||
const props = setup({ adhocFilter: wholeArrayFilter });
|
||||
const { onOperatorChange } = useSimpleTabFilterProps(
|
||||
props as unknown as Props,
|
||||
);
|
||||
onOperatorChange(Operators.ContainsAll);
|
||||
const lastCall =
|
||||
props.onChange.mock.calls[props.onChange.mock.calls.length - 1][0];
|
||||
expect(lastCall.operatorId).toEqual(Operators.ContainsAll);
|
||||
expect(lastCall.comparator).toBeUndefined();
|
||||
});
|
||||
|
||||
test('keeps the value when switching within the element family', () => {
|
||||
// Contains any <-> Contains all both take individual elements, so the
|
||||
// selected elements should carry over.
|
||||
const elementFilter = new AdhocFilter({
|
||||
expressionType: ExpressionTypes.Simple,
|
||||
subject: 'scores',
|
||||
operatorId: Operators.ContainsAny,
|
||||
operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.ContainsAny].operation,
|
||||
comparator: ['5', '6'],
|
||||
clause: Clauses.Where,
|
||||
});
|
||||
const props = setup({ adhocFilter: elementFilter });
|
||||
const { onOperatorChange } = useSimpleTabFilterProps(
|
||||
props as unknown as Props,
|
||||
);
|
||||
onOperatorChange(Operators.ContainsAll);
|
||||
const lastCall =
|
||||
props.onChange.mock.calls[props.onChange.mock.calls.length - 1][0];
|
||||
expect(lastCall.comparator).toEqual(['5', '6']);
|
||||
});
|
||||
|
||||
test('passes the new adhocFilter to onChange after onComparatorChange', () => {
|
||||
const props = setup();
|
||||
const { onComparatorChange } = useSimpleTabFilterProps(
|
||||
@@ -399,6 +515,28 @@ test('will not display boolean operators when column type is string', () => {
|
||||
});
|
||||
});
|
||||
|
||||
test.each(['STRING', 'DATE'])(
|
||||
'will not display boolean operators when an expression column declares type %s',
|
||||
type => {
|
||||
const props = setup({
|
||||
datasource: {
|
||||
type: 'table' as const,
|
||||
datasource_name: 'table1',
|
||||
schema: 'schema',
|
||||
columns: [{ column_name: 'value', type, expression: '"value"' }],
|
||||
},
|
||||
adhocFilter: simpleAdhocFilter,
|
||||
});
|
||||
const { isOperatorRelevant } = useSimpleTabFilterProps(
|
||||
props as unknown as Props,
|
||||
);
|
||||
const booleanOnlyOperators = [Operators.IsTrue, Operators.IsFalse];
|
||||
booleanOnlyOperators.forEach(operator => {
|
||||
expect(isOperatorRelevant(operator, 'value')).toBe(false);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
test('will display boolean operators when column is an expression', () => {
|
||||
const props = setup({
|
||||
datasource: {
|
||||
|
||||
+112
-14
@@ -32,6 +32,7 @@ import {
|
||||
isDefined,
|
||||
SupersetClient,
|
||||
} from '@superset-ui/core';
|
||||
import { GenericDataType } from '@apache-superset/core/common';
|
||||
import { styled, useTheme, css } from '@apache-superset/core/theme';
|
||||
import {
|
||||
Operators,
|
||||
@@ -118,6 +119,8 @@ export const useSimpleTabFilterProps = (props: Props) => {
|
||||
const isColumnNumber =
|
||||
!!column && (column.type === 'INT' || column.type === 'INTEGER');
|
||||
const isColumnFunction = !!column && !!column.expression;
|
||||
const isColumnMultiValue =
|
||||
!!column && column.type_generic === GenericDataType.MultiValue;
|
||||
|
||||
if (operator && operator === Operators.LatestPartition) {
|
||||
const { partitionColumn } = props;
|
||||
@@ -127,8 +130,41 @@ export const useSimpleTabFilterProps = (props: Props) => {
|
||||
// hide the TEMPORAL_RANGE operator
|
||||
return false;
|
||||
}
|
||||
// Element-level array operators only apply to multi-value columns.
|
||||
const arrayElementOperators = [
|
||||
Operators.ContainsAny,
|
||||
Operators.ContainsAll,
|
||||
Operators.IsEmpty,
|
||||
Operators.IsNotEmpty,
|
||||
Operators.LengthEquals,
|
||||
Operators.LengthGreaterThan,
|
||||
Operators.LengthLessThan,
|
||||
Operators.LengthGreaterThanOrEqual,
|
||||
Operators.LengthLessThanOrEqual,
|
||||
];
|
||||
if (arrayElementOperators.includes(operator)) {
|
||||
return isColumnMultiValue;
|
||||
}
|
||||
if (isColumnMultiValue) {
|
||||
// Array columns support whole-array operators (=, !=, In, Not in, null
|
||||
// checks) plus the element-level operators above. Scalar-only operators
|
||||
// (Like, <, >, <=, >=) are hidden because they aren't valid on an array.
|
||||
return [
|
||||
Operators.Equals,
|
||||
Operators.NotEquals,
|
||||
Operators.In,
|
||||
Operators.NotIn,
|
||||
Operators.IsNull,
|
||||
Operators.IsNotNull,
|
||||
...arrayElementOperators,
|
||||
].includes(operator);
|
||||
}
|
||||
if (operator === Operators.IsTrue || operator === Operators.IsFalse) {
|
||||
return isColumnBoolean || isColumnNumber || isColumnFunction;
|
||||
// An expression column may evaluate to a boolean, but that is only a
|
||||
// safe assumption while its type is unknown; a declared type wins.
|
||||
return (
|
||||
isColumnBoolean || isColumnNumber || (isColumnFunction && !column?.type)
|
||||
);
|
||||
}
|
||||
if (isColumnBoolean) {
|
||||
return operator === Operators.IsNull || operator === Operators.IsNotNull;
|
||||
@@ -167,9 +203,19 @@ export const useSimpleTabFilterProps = (props: Props) => {
|
||||
].operation
|
||||
: null;
|
||||
if (!isDefined(operator)) {
|
||||
// if operator is `null`, use the `IN` and reset the comparator.
|
||||
operator = Operators.In;
|
||||
operatorId = Operators.In;
|
||||
// The previous operator is not relevant for the new subject; pick a
|
||||
// sensible default and reset the comparator. Multi-value (array) columns
|
||||
// default to "Contains any" (element membership) rather than the
|
||||
// scalar-only IN.
|
||||
const newColumn = props.datasource.columns?.find(
|
||||
col => col.column_name === subject,
|
||||
);
|
||||
const defaultOperator =
|
||||
newColumn?.type_generic === GenericDataType.MultiValue
|
||||
? Operators.ContainsAny
|
||||
: Operators.In;
|
||||
operator = defaultOperator;
|
||||
operatorId = defaultOperator;
|
||||
comparator = undefined;
|
||||
}
|
||||
|
||||
@@ -193,10 +239,38 @@ export const useSimpleTabFilterProps = (props: Props) => {
|
||||
};
|
||||
const onOperatorChange = (operatorId: Operators) => {
|
||||
const currentComparator = props.adhocFilter.comparator;
|
||||
// The value space differs between operator families: element-level array
|
||||
// ops (Contains any/all) take individual elements, whole-array/scalar ops
|
||||
// (=, In, …) take whole arrays or scalars, Length ops take a count, and the
|
||||
// unary ops take nothing. A value from one family is meaningless in another,
|
||||
// so reset the value when the family changes (e.g. Equal to -> Contains all).
|
||||
const comparatorKind = (op?: Operators): string => {
|
||||
if (!op) return 'none';
|
||||
if (op === Operators.ContainsAny || op === Operators.ContainsAll) {
|
||||
return 'element';
|
||||
}
|
||||
if (
|
||||
op === Operators.LengthEquals ||
|
||||
op === Operators.LengthGreaterThan ||
|
||||
op === Operators.LengthLessThan ||
|
||||
op === Operators.LengthGreaterThanOrEqual ||
|
||||
op === Operators.LengthLessThanOrEqual
|
||||
) {
|
||||
return 'length';
|
||||
}
|
||||
if (DISABLE_INPUT_OPERATORS.includes(op)) return 'none';
|
||||
return 'value';
|
||||
};
|
||||
const valueFamilyChanged =
|
||||
comparatorKind(props.adhocFilter.operatorId as Operators | undefined) !==
|
||||
comparatorKind(operatorId);
|
||||
|
||||
let newComparator;
|
||||
// convert between list of comparators and individual comparators
|
||||
// (e.g. `in ('North America', 'Africa')` to `== 'North America'`)
|
||||
if (MULTI_OPERATORS.has(operatorId)) {
|
||||
if (valueFamilyChanged) {
|
||||
newComparator = undefined;
|
||||
} else if (MULTI_OPERATORS.has(operatorId)) {
|
||||
// convert between list of comparators and individual comparators
|
||||
// (e.g. `in ('North America', 'Africa')` to `== 'North America'`)
|
||||
newComparator = Array.isArray(currentComparator)
|
||||
? currentComparator
|
||||
: [currentComparator].filter(element => element != null);
|
||||
@@ -433,19 +507,42 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
|
||||
if (loadingComparatorSuggestions) {
|
||||
controller.abort();
|
||||
}
|
||||
// Element-level array operators (Contains any / Contains all) search
|
||||
// inside the array, so suggest individual elements; whole-array
|
||||
// operators (=, In, …) keep the default distinct-array suggestions.
|
||||
const { operatorId } = props.adhocFilter;
|
||||
const arrayElements =
|
||||
operatorId === Operators.ContainsAny ||
|
||||
operatorId === Operators.ContainsAll;
|
||||
setLoadingComparatorSuggestions(true);
|
||||
SupersetClient.get({
|
||||
signal,
|
||||
endpoint: `/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/`,
|
||||
endpoint: `/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/${
|
||||
arrayElements ? '?array_elements=true' : ''
|
||||
}`,
|
||||
})
|
||||
.then(({ json }) => {
|
||||
setSuggestions(
|
||||
json.result.map(
|
||||
(suggestion: null | number | boolean | string) => ({
|
||||
value: suggestion,
|
||||
label: optionLabel(suggestion),
|
||||
}),
|
||||
),
|
||||
json.result.map((suggestion: unknown) => {
|
||||
// Complex column values arrive as JS arrays or objects: whole
|
||||
// arrays for MULTI_VALUE columns (e.g. [5, 6, 7]) and Map/Tuple
|
||||
// objects for nested-container columns (e.g. {"a": ["x","y"]}).
|
||||
// A raw array/object is neither a valid single-select value
|
||||
// (antd collapses an array to its first element) nor renderable
|
||||
// as a React child (an object throws). Render it as its literal
|
||||
// string, which is also exactly what the backend's
|
||||
// parse_array_literal expects for the whole-array operators.
|
||||
if (suggestion !== null && typeof suggestion === 'object') {
|
||||
const literal = JSON.stringify(suggestion);
|
||||
return { value: literal, label: literal };
|
||||
}
|
||||
return {
|
||||
value: suggestion as null | number | boolean | string,
|
||||
label: optionLabel(
|
||||
suggestion as null | number | boolean | string,
|
||||
),
|
||||
};
|
||||
}),
|
||||
);
|
||||
setLoadingComparatorSuggestions(false);
|
||||
})
|
||||
@@ -464,6 +561,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
|
||||
}, [
|
||||
props.adhocFilter.subject,
|
||||
props.adhocFilter.clause,
|
||||
props.adhocFilter.operatorId,
|
||||
props.datasource,
|
||||
datePicker,
|
||||
]);
|
||||
|
||||
+11
@@ -44,6 +44,17 @@ export const OPERATORS_TO_SQL = {
|
||||
'IS NULL': 'IS NULL',
|
||||
'IS TRUE': 'IS TRUE',
|
||||
'IS FALSE': 'IS FALSE',
|
||||
// Element-level array operators (shown as filter labels; not executable SQL —
|
||||
// the Custom SQL tab is hidden for these).
|
||||
CONTAINS_ANY: 'CONTAINS ANY',
|
||||
CONTAINS_ALL: 'CONTAINS ALL',
|
||||
IS_EMPTY: 'IS EMPTY',
|
||||
IS_NOT_EMPTY: 'IS NOT EMPTY',
|
||||
LENGTH_EQUALS: 'LENGTH =',
|
||||
LENGTH_GREATER_THAN: 'LENGTH >',
|
||||
LENGTH_LESS_THAN: 'LENGTH <',
|
||||
LENGTH_GREATER_THAN_OR_EQUALS: 'LENGTH >=',
|
||||
LENGTH_LESS_THAN_OR_EQUALS: 'LENGTH <=',
|
||||
'LATEST PARTITION': ({
|
||||
datasource,
|
||||
}: {
|
||||
|
||||
@@ -45,6 +45,17 @@ export enum Operators {
|
||||
IsTrue = 'IS_TRUE',
|
||||
IsFalse = 'IS_FALSE',
|
||||
TemporalRange = 'TEMPORAL_RANGE',
|
||||
// Element-level operators for multi-value (array) columns
|
||||
ContainsAny = 'CONTAINS_ANY',
|
||||
ContainsAll = 'CONTAINS_ALL',
|
||||
IsEmpty = 'IS_EMPTY',
|
||||
IsNotEmpty = 'IS_NOT_EMPTY',
|
||||
// Length (element-count) comparison operators for array columns
|
||||
LengthEquals = 'LENGTH_EQUALS',
|
||||
LengthGreaterThan = 'LENGTH_GREATER_THAN',
|
||||
LengthLessThan = 'LENGTH_LESS_THAN',
|
||||
LengthGreaterThanOrEqual = 'LENGTH_GREATER_THAN_OR_EQUALS',
|
||||
LengthLessThanOrEqual = 'LENGTH_LESS_THAN_OR_EQUALS',
|
||||
}
|
||||
|
||||
export interface OperatorType {
|
||||
@@ -89,6 +100,39 @@ export const OPERATOR_ENUM_TO_OPERATOR_TYPE: {
|
||||
display: t('TEMPORAL_RANGE'),
|
||||
operation: 'TEMPORAL_RANGE',
|
||||
},
|
||||
[Operators.ContainsAny]: {
|
||||
display: t('Contains any'),
|
||||
operation: 'CONTAINS_ANY',
|
||||
},
|
||||
[Operators.ContainsAll]: {
|
||||
display: t('Contains all'),
|
||||
operation: 'CONTAINS_ALL',
|
||||
},
|
||||
[Operators.IsEmpty]: { display: t('Is empty'), operation: 'IS_EMPTY' },
|
||||
[Operators.IsNotEmpty]: {
|
||||
display: t('Is not empty'),
|
||||
operation: 'IS_NOT_EMPTY',
|
||||
},
|
||||
[Operators.LengthEquals]: {
|
||||
display: t('Length equals (=)'),
|
||||
operation: 'LENGTH_EQUALS',
|
||||
},
|
||||
[Operators.LengthGreaterThan]: {
|
||||
display: t('Length greater than (>)'),
|
||||
operation: 'LENGTH_GREATER_THAN',
|
||||
},
|
||||
[Operators.LengthLessThan]: {
|
||||
display: t('Length less than (<)'),
|
||||
operation: 'LENGTH_LESS_THAN',
|
||||
},
|
||||
[Operators.LengthGreaterThanOrEqual]: {
|
||||
display: t('Length greater or equal (>=)'),
|
||||
operation: 'LENGTH_GREATER_THAN_OR_EQUALS',
|
||||
},
|
||||
[Operators.LengthLessThanOrEqual]: {
|
||||
display: t('Length less or equal (<=)'),
|
||||
operation: 'LENGTH_LESS_THAN_OR_EQUALS',
|
||||
},
|
||||
};
|
||||
|
||||
export const OPERATORS_OPTIONS = Object.values(Operators) as Operators[];
|
||||
@@ -105,7 +149,12 @@ export const HAVING_OPERATORS = [
|
||||
Operators.GreaterThan,
|
||||
Operators.GreaterThanOrEqual,
|
||||
];
|
||||
export const MULTI_OPERATORS = new Set([Operators.In, Operators.NotIn]);
|
||||
export const MULTI_OPERATORS = new Set([
|
||||
Operators.In,
|
||||
Operators.NotIn,
|
||||
Operators.ContainsAny,
|
||||
Operators.ContainsAll,
|
||||
]);
|
||||
// CUSTOM_OPERATORS will show operator in simple mode,
|
||||
// but will generate customized sqlExpression
|
||||
export const CUSTOM_OPERATORS = new Set([
|
||||
@@ -120,6 +169,8 @@ export const DISABLE_INPUT_OPERATORS = [
|
||||
Operators.LatestPartition,
|
||||
Operators.IsTrue,
|
||||
Operators.IsFalse,
|
||||
Operators.IsEmpty,
|
||||
Operators.IsNotEmpty,
|
||||
];
|
||||
|
||||
export const sqlaAutoGeneratedMetricNameRegex =
|
||||
|
||||
@@ -82,3 +82,14 @@ test('Should handle boolean true comparator as a string value', () => {
|
||||
"subject operator 'TRUE'",
|
||||
);
|
||||
});
|
||||
|
||||
test('Should render array-literal comparators as-is (not quoted)', () => {
|
||||
// Whole-array = filter: the pasted array literal is shown unquoted.
|
||||
expect(getSimpleSQLExpression('ingredients', '=', "['1 large egg']")).toBe(
|
||||
"ingredients = ['1 large egg']",
|
||||
);
|
||||
// IN with multiple array literals.
|
||||
expect(
|
||||
getSimpleSQLExpression('ingredients', Operators.In, ["['a']", "['b']"]),
|
||||
).toBe(`ingredients ${Operators.In} (['a'], ['b'])`);
|
||||
});
|
||||
|
||||
@@ -461,10 +461,15 @@ export const getSimpleSQLExpression = (
|
||||
if (comparatorArray.length > 0 && showComparator) {
|
||||
const formattedComparators = comparatorArray
|
||||
.map(val => optionLabel(val))
|
||||
.map(
|
||||
val =>
|
||||
`${quote}${isString ? String(val).replace(/'/g, "''") : val}${quote}`,
|
||||
);
|
||||
.map(val => {
|
||||
// Array-literal values (e.g. ['a', 'b']) are shown as-is rather than
|
||||
// quoted/escaped as a string, so array-column filters read naturally.
|
||||
const asString = String(val);
|
||||
if (asString.startsWith('[') && asString.endsWith(']')) {
|
||||
return asString;
|
||||
}
|
||||
return `${quote}${isString ? asString.replace(/'/g, "''") : val}${quote}`;
|
||||
});
|
||||
expression += ` ${prefix}${formattedComparators.join(', ')}${suffix}`;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,8 +27,15 @@ import {
|
||||
Input,
|
||||
Button,
|
||||
Modal,
|
||||
Select,
|
||||
} from '@superset-ui/core/components';
|
||||
import { useToasts } from 'src/components/MessageToasts/withToasts';
|
||||
import copyTextToClipboard from 'src/utils/copy';
|
||||
import {
|
||||
API_KEY_SCOPE_OPTIONS,
|
||||
getApiKeyScopesHelpText,
|
||||
serializeApiKeyScopes,
|
||||
} from './apiKeyScopes';
|
||||
|
||||
interface ApiKeyCreateModalProps {
|
||||
show: boolean;
|
||||
@@ -38,6 +45,7 @@ interface ApiKeyCreateModalProps {
|
||||
|
||||
interface FormValues {
|
||||
name: string;
|
||||
scopes?: string[];
|
||||
}
|
||||
|
||||
export function ApiKeyCreateModal({
|
||||
@@ -62,9 +70,13 @@ export function ApiKeyCreateModal({
|
||||
|
||||
const handleFormSubmit = async (values: FormValues) => {
|
||||
try {
|
||||
const scopes = serializeApiKeyScopes(values.scopes);
|
||||
const response = await SupersetClient.post({
|
||||
endpoint: '/api/v1/security/api_keys/',
|
||||
jsonPayload: values,
|
||||
jsonPayload: {
|
||||
name: values.name,
|
||||
...(scopes && { scopes }),
|
||||
},
|
||||
});
|
||||
const key = response.json?.result?.key;
|
||||
if (!key) {
|
||||
@@ -83,7 +95,7 @@ export function ApiKeyCreateModal({
|
||||
return;
|
||||
}
|
||||
try {
|
||||
await navigator.clipboard.writeText(createdKey);
|
||||
await copyTextToClipboard(() => Promise.resolve(createdKey));
|
||||
setCopied(true);
|
||||
if (copyTimerRef.current) {
|
||||
clearTimeout(copyTimerRef.current);
|
||||
@@ -170,6 +182,24 @@ export function ApiKeyCreateModal({
|
||||
placeholder={t('e.g., CI/CD Pipeline, Analytics Script')}
|
||||
/>
|
||||
</FormItem>
|
||||
<FormItem
|
||||
name="scopes"
|
||||
label={t('MCP scopes')}
|
||||
help={getApiKeyScopesHelpText()}
|
||||
>
|
||||
<Select
|
||||
name="scopes"
|
||||
mode="multiple"
|
||||
allowClear
|
||||
showSearch
|
||||
options={API_KEY_SCOPE_OPTIONS}
|
||||
placeholder={t('Select MCP resource scopes (optional)')}
|
||||
data-test="api-key-scopes-select"
|
||||
getPopupContainer={(trigger: HTMLElement) =>
|
||||
trigger.closest<HTMLElement>('.ant-modal-container') ?? trigger
|
||||
}
|
||||
/>
|
||||
</FormItem>
|
||||
</FormModal>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -162,6 +162,19 @@ export function ApiKeyList() {
|
||||
key: 'status',
|
||||
render: (_: unknown, record: ApiKey) => getStatusBadge(record),
|
||||
},
|
||||
{
|
||||
title: t('MCP scopes'),
|
||||
dataIndex: 'scopes',
|
||||
key: 'scopes',
|
||||
render: (scopes: string | null) =>
|
||||
scopes ? (
|
||||
<Tooltip title={scopes}>
|
||||
<Tag>{t('%s MCP scopes', scopes.split(',').length)}</Tag>
|
||||
</Tooltip>
|
||||
) : (
|
||||
<Tag>{t('RBAC only')}</Tag>
|
||||
),
|
||||
},
|
||||
{
|
||||
title: t('Actions'),
|
||||
key: 'actions',
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
/**
|
||||
* Licensed to the Apache Software Foundation (ASF) under one
|
||||
* or more contributor license agreements. See the NOTICE file
|
||||
* distributed with this work for additional information
|
||||
* regarding copyright ownership. The ASF licenses this file
|
||||
* to you under the Apache License, Version 2.0 (the
|
||||
* "License"); you may not use this file except in compliance
|
||||
* with the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import {
|
||||
API_KEY_SCOPE_OPTIONS,
|
||||
getApiKeyScopesHelpText,
|
||||
serializeApiKeyScopes,
|
||||
} from './apiKeyScopes';
|
||||
|
||||
test('offers read and write scopes for every supported resource', () => {
|
||||
expect(API_KEY_SCOPE_OPTIONS).toHaveLength(32);
|
||||
expect(API_KEY_SCOPE_OPTIONS).toContainEqual({
|
||||
label: 'superset:dashboard:read',
|
||||
value: 'superset:dashboard:read',
|
||||
});
|
||||
expect(API_KEY_SCOPE_OPTIONS).toContainEqual({
|
||||
label: 'superset:sqllab:write',
|
||||
value: 'superset:sqllab:write',
|
||||
});
|
||||
});
|
||||
|
||||
test('serializes selected scopes for the FAB API', () => {
|
||||
expect(
|
||||
serializeApiKeyScopes(['superset:dashboard:read', 'superset:chart:write']),
|
||||
).toBe('superset:dashboard:read,superset:chart:write');
|
||||
expect(serializeApiKeyScopes([])).toBeUndefined();
|
||||
expect(serializeApiKeyScopes()).toBeUndefined();
|
||||
});
|
||||
|
||||
test('explains that scopes apply to MCP rather than REST APIs', () => {
|
||||
expect(getApiKeyScopesHelpText()).toContain('MCP resources');
|
||||
expect(getApiKeyScopesHelpText()).toContain(
|
||||
'do not restrict REST API requests',
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,55 @@
|
||||
/**
|
||||
* Licensed to the Apache Software Foundation (ASF) under one
|
||||
* or more contributor license agreements. See the NOTICE file
|
||||
* distributed with this work for additional information
|
||||
* regarding copyright ownership. The ASF licenses this file
|
||||
* to you under the Apache License, Version 2.0 (the
|
||||
* "License"); you may not use this file except in compliance
|
||||
* with the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { t } from '@apache-superset/core/translation';
|
||||
|
||||
const API_KEY_SCOPE_RESOURCES = [
|
||||
'annotation',
|
||||
'chart',
|
||||
'dashboard',
|
||||
'database',
|
||||
'dataset',
|
||||
'explore',
|
||||
'query',
|
||||
'report',
|
||||
'role',
|
||||
'rls',
|
||||
'savedquery',
|
||||
'sqllab',
|
||||
'tag',
|
||||
'task',
|
||||
'theme',
|
||||
'user',
|
||||
] as const;
|
||||
|
||||
const API_KEY_SCOPE_ACTIONS = ['read', 'write'] as const;
|
||||
|
||||
export const API_KEY_SCOPE_OPTIONS = API_KEY_SCOPE_RESOURCES.flatMap(resource =>
|
||||
API_KEY_SCOPE_ACTIONS.map(action => {
|
||||
const value = `superset:${resource}:${action}`;
|
||||
return { label: value, value };
|
||||
}),
|
||||
);
|
||||
|
||||
export const serializeApiKeyScopes = (scopes?: string[]) =>
|
||||
scopes?.length ? scopes.join(',') : undefined;
|
||||
|
||||
export const getApiKeyScopesHelpText = () =>
|
||||
t(
|
||||
'Limit which MCP resources and actions this key can access. These scopes do not restrict REST API requests and never grant permissions the user does not already have. Leave empty for legacy RBAC-only behavior.',
|
||||
);
|
||||
@@ -136,6 +136,7 @@ describe('DatabaseModal', () => {
|
||||
format: 'int32',
|
||||
maximum: 65536,
|
||||
minimum: 0,
|
||||
nullable: true,
|
||||
type: 'integer',
|
||||
},
|
||||
query: {
|
||||
@@ -153,7 +154,7 @@ describe('DatabaseModal', () => {
|
||||
type: 'string',
|
||||
},
|
||||
},
|
||||
required: ['database', 'host', 'port', 'username'],
|
||||
required: ['database', 'host', 'username'],
|
||||
type: 'object',
|
||||
},
|
||||
preferred: true,
|
||||
|
||||
@@ -31,7 +31,12 @@ import {
|
||||
import { Group, Role, UserObject } from 'src/pages/UsersList/types';
|
||||
import { Actions } from 'src/constants';
|
||||
import { BaseUserListModalProps, FormValues } from './types';
|
||||
import { createUser, updateUser, atLeastOneRoleOrGroup } from './utils';
|
||||
import {
|
||||
createUser,
|
||||
updateUser,
|
||||
atLeastOneRoleOrGroup,
|
||||
handleUserError,
|
||||
} from './utils';
|
||||
|
||||
export interface UserModalProps extends BaseUserListModalProps {
|
||||
roles: Role[];
|
||||
@@ -51,36 +56,6 @@ function UserListModal({
|
||||
}: UserModalProps) {
|
||||
const { addDangerToast, addSuccessToast } = useToasts();
|
||||
const handleFormSubmit = async (values: FormValues) => {
|
||||
const handleError = async (
|
||||
err: any,
|
||||
action: Actions.CREATE | Actions.UPDATE,
|
||||
) => {
|
||||
let errorMessage =
|
||||
action === Actions.CREATE
|
||||
? t('There was an error creating the user. Please, try again.')
|
||||
: t('There was an error updating the user. Please, try again.');
|
||||
|
||||
if (err.status === 422) {
|
||||
const errorData = await err.json();
|
||||
const detail = errorData?.message || '';
|
||||
|
||||
if (detail.includes('duplicate key value')) {
|
||||
if (detail.includes('ab_user_username_key')) {
|
||||
errorMessage = t(
|
||||
'This username is already taken. Please choose another one.',
|
||||
);
|
||||
} else if (detail.includes('ab_user_email_key')) {
|
||||
errorMessage = t(
|
||||
'This email is already associated with an account. Please choose another one.',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
addDangerToast(errorMessage);
|
||||
throw err;
|
||||
};
|
||||
|
||||
if (isEditMode) {
|
||||
if (!user) {
|
||||
throw new Error('User is required in edit mode');
|
||||
@@ -89,14 +64,14 @@ function UserListModal({
|
||||
await updateUser(user.id, values);
|
||||
addSuccessToast(t('The user has been updated successfully.'));
|
||||
} catch (err) {
|
||||
await handleError(err, Actions.UPDATE);
|
||||
await handleUserError(err as Response, Actions.UPDATE, addDangerToast);
|
||||
}
|
||||
} else {
|
||||
try {
|
||||
await createUser(values);
|
||||
addSuccessToast(t('The user has been created successfully.'));
|
||||
} catch (err) {
|
||||
await handleError(err, Actions.CREATE);
|
||||
await handleUserError(err as Response, Actions.CREATE, addDangerToast);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
/**
|
||||
* Licensed to the Apache Software Foundation (ASF) under one
|
||||
* or more contributor license agreements. See the NOTICE file
|
||||
* distributed with this work for additional information
|
||||
* regarding copyright ownership. The ASF licenses this file
|
||||
* to you under the Apache License, Version 2.0 (the
|
||||
* "License"); you may not use this file except in compliance
|
||||
* with the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { Actions } from 'src/constants';
|
||||
import { handleUserError } from './utils';
|
||||
|
||||
test('shows the password validation message from a 400 response', async () => {
|
||||
const error = new Response(
|
||||
JSON.stringify({
|
||||
message: {
|
||||
password: ['Password must be at least 8 characters long.'],
|
||||
},
|
||||
}),
|
||||
{ status: 400 },
|
||||
);
|
||||
const addDangerToast = jest.fn();
|
||||
|
||||
await expect(
|
||||
handleUserError(error, Actions.CREATE, addDangerToast),
|
||||
).rejects.toBe(error);
|
||||
expect(addDangerToast).toHaveBeenCalledWith(
|
||||
'Password must be at least 8 characters long.',
|
||||
);
|
||||
});
|
||||
|
||||
test('shows a plain string message from a 400 response', async () => {
|
||||
const error = new Response(
|
||||
JSON.stringify({ message: 'User must have at least one role or group!' }),
|
||||
{ status: 400 },
|
||||
);
|
||||
const addDangerToast = jest.fn();
|
||||
|
||||
await expect(
|
||||
handleUserError(error, Actions.UPDATE, addDangerToast),
|
||||
).rejects.toBe(error);
|
||||
expect(addDangerToast).toHaveBeenCalledWith(
|
||||
'User must have at least one role or group!',
|
||||
);
|
||||
});
|
||||
|
||||
test('keeps the duplicate username message for a 422 response', async () => {
|
||||
const error = new Response(
|
||||
JSON.stringify({
|
||||
message:
|
||||
'duplicate key value violates unique constraint "ab_user_username_key"',
|
||||
}),
|
||||
{ status: 422 },
|
||||
);
|
||||
const addDangerToast = jest.fn();
|
||||
|
||||
await expect(
|
||||
handleUserError(error, Actions.CREATE, addDangerToast),
|
||||
).rejects.toBe(error);
|
||||
expect(addDangerToast).toHaveBeenCalledWith(
|
||||
'This username is already taken. Please choose another one.',
|
||||
);
|
||||
});
|
||||
|
||||
test('shows the generic message when a 422 response has no message', async () => {
|
||||
const error = new Response(JSON.stringify({ foo: 'bar' }), { status: 422 });
|
||||
const addDangerToast = jest.fn();
|
||||
|
||||
await expect(
|
||||
handleUserError(error, Actions.CREATE, addDangerToast),
|
||||
).rejects.toBe(error);
|
||||
expect(addDangerToast).toHaveBeenCalledWith(
|
||||
'There was an error creating the user. Please, try again.',
|
||||
);
|
||||
});
|
||||
|
||||
test('shows the generic message when a 400 response is not JSON', async () => {
|
||||
const error = new Response('<html>Bad request</html>', {
|
||||
status: 400,
|
||||
headers: { 'Content-Type': 'text/html' },
|
||||
});
|
||||
const addDangerToast = jest.fn();
|
||||
|
||||
await expect(
|
||||
handleUserError(error, Actions.CREATE, addDangerToast),
|
||||
).rejects.toBe(error);
|
||||
expect(addDangerToast).toHaveBeenCalledWith(
|
||||
'There was an error creating the user. Please, try again.',
|
||||
);
|
||||
});
|
||||
@@ -17,10 +17,49 @@
|
||||
* under the License.
|
||||
*/
|
||||
import { t } from '@apache-superset/core/translation';
|
||||
import { SupersetClient } from '@superset-ui/core';
|
||||
import { getClientErrorObject, SupersetClient } from '@superset-ui/core';
|
||||
import { SelectOption } from 'src/components/ListView';
|
||||
import { Actions } from 'src/constants';
|
||||
import { FormValues } from './types';
|
||||
|
||||
type AddDangerToast = (message: string) => void;
|
||||
|
||||
export const handleUserError = async (
|
||||
err: Response,
|
||||
action: Actions.CREATE | Actions.UPDATE,
|
||||
addDangerToast: AddDangerToast,
|
||||
): Promise<never> => {
|
||||
let errorMessage =
|
||||
action === Actions.CREATE
|
||||
? t('There was an error creating the user. Please, try again.')
|
||||
: t('There was an error updating the user. Please, try again.');
|
||||
|
||||
if (err.status === 400 || err.status === 422) {
|
||||
const errorData = await getClientErrorObject(err);
|
||||
const message: unknown = errorData.message;
|
||||
|
||||
if (err.status === 400 && message && errorData.error) {
|
||||
errorMessage = errorData.error;
|
||||
} else if (
|
||||
err.status === 422 &&
|
||||
errorData.error?.includes('duplicate key value')
|
||||
) {
|
||||
if (errorData.error.includes('ab_user_username_key')) {
|
||||
errorMessage = t(
|
||||
'This username is already taken. Please choose another one.',
|
||||
);
|
||||
} else if (errorData.error.includes('ab_user_email_key')) {
|
||||
errorMessage = t(
|
||||
'This email is already associated with an account. Please choose another one.',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
addDangerToast(errorMessage);
|
||||
throw err;
|
||||
};
|
||||
|
||||
export const createUser = async (values: FormValues) => {
|
||||
const { confirmPassword: _confirmPassword, ...payload } = values;
|
||||
if (payload.active == null) {
|
||||
|
||||
@@ -1157,6 +1157,34 @@ test('dataset links use internal routing when PREVENT_UNSAFE_DEFAULT_URLS_ON_DAT
|
||||
});
|
||||
});
|
||||
|
||||
test('legacy dashboard default URLs use the registered client route', async () => {
|
||||
const dataset = {
|
||||
...mockDatasets[0],
|
||||
explore_url: '/superset/dashboard/123/?standalone=1#section',
|
||||
};
|
||||
mockDatasetListEndpoints({ result: [dataset], count: 1 });
|
||||
|
||||
renderDatasetList(
|
||||
mockAdminUser,
|
||||
{},
|
||||
{
|
||||
common: {
|
||||
conf: {
|
||||
PREVENT_UNSAFE_DEFAULT_URLS_ON_DATASET: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
const datasetLink = await screen.findByRole('link', {
|
||||
name: dataset.table_name,
|
||||
});
|
||||
expect(datasetLink).toHaveAttribute(
|
||||
'href',
|
||||
'/dashboard/123/?standalone=1#section',
|
||||
);
|
||||
});
|
||||
|
||||
// Note: These delete error tests verify that the modal doesn't open when fetching
|
||||
// related_objects fails. The component's openDatasetDeleteModal error handler
|
||||
// (index.tsx:262-268) returns a string but doesn't call addDangerToast(), so no
|
||||
|
||||
@@ -54,10 +54,18 @@ import {
|
||||
|
||||
const APP_ROOT = '/superset';
|
||||
|
||||
const renderUnderSubdirectory = () => {
|
||||
const renderUnderSubdirectory = (preventUnsafeDefaultUrls = false) => {
|
||||
const defaultState = createDefaultStoreState(mockAdminUser);
|
||||
const store = createMockStore({
|
||||
...createDefaultStoreState(mockAdminUser),
|
||||
...defaultState,
|
||||
user: mockAdminUser,
|
||||
common: {
|
||||
...defaultState.common,
|
||||
conf: {
|
||||
...defaultState.common?.conf,
|
||||
PREVENT_UNSAFE_DEFAULT_URLS_ON_DATASET: preventUnsafeDefaultUrls,
|
||||
},
|
||||
},
|
||||
});
|
||||
return render(
|
||||
<Provider store={store}>
|
||||
@@ -115,6 +123,31 @@ test('explore link is single-prefixed under a subdirectory deployment', async ()
|
||||
expect(exploreLink.getAttribute('href')).not.toContain('/superset/superset');
|
||||
});
|
||||
|
||||
test('legacy dashboard default URL uses the router basename once', async () => {
|
||||
// A subdirectory user pastes the full browser path, so the saved value
|
||||
// carries both the application root and the legacy `/superset` prefix.
|
||||
// stripAppRoot removes the root and the legacy normalization removes the
|
||||
// prefix, leaving the basename to re-add the root exactly once.
|
||||
const dataset = {
|
||||
...mockDatasets[0],
|
||||
explore_url: `${APP_ROOT}/superset/dashboard/123/?standalone=1#section`,
|
||||
};
|
||||
mockDatasetListEndpoints({ result: [dataset], count: 1 });
|
||||
|
||||
renderUnderSubdirectory(true);
|
||||
|
||||
const dashboardLink = await screen.findByRole('link', {
|
||||
name: dataset.table_name,
|
||||
});
|
||||
expect(dashboardLink).toHaveAttribute(
|
||||
'href',
|
||||
`${APP_ROOT}/dashboard/123/?standalone=1#section`,
|
||||
);
|
||||
expect(dashboardLink.getAttribute('href')).not.toContain(
|
||||
'/superset/superset',
|
||||
);
|
||||
});
|
||||
|
||||
test('external default_endpoint passes through unprefixed', async () => {
|
||||
const dataset = {
|
||||
...mockDatasets[0],
|
||||
|
||||
@@ -87,7 +87,6 @@ import withToasts from 'src/components/MessageToasts/withToasts';
|
||||
import { Icons } from '@superset-ui/core/components/Icons';
|
||||
import WarningIconWithTooltip from '@superset-ui/core/components/WarningIconWithTooltip';
|
||||
import { isUserEditorOrAdmin } from 'src/dashboard/util/permissionUtils';
|
||||
|
||||
import {
|
||||
PAGE_SIZE,
|
||||
SORT_BY,
|
||||
@@ -114,6 +113,10 @@ import type {
|
||||
} from 'src/types/bootstrapTypes';
|
||||
import type User from 'src/types/User';
|
||||
|
||||
// Keep saved Default URLs compatible with the prefix-free SPA route.
|
||||
const normalizeLegacyDashboardUrl = (url: string) =>
|
||||
url.replace(/^\/superset(?=\/dashboard(?:\/|$))/, '');
|
||||
|
||||
const SEMANTIC_LAYERS_FLAG = 'SEMANTIC_LAYERS' as FeatureFlag;
|
||||
type DatasetExtra = {
|
||||
certification?: {
|
||||
@@ -722,7 +725,9 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
|
||||
// Router basename, which re-prefixes the root — so strip it here to
|
||||
// avoid a doubled `/superset/superset/...`. External
|
||||
// `default_endpoint` URLs pass through unchanged.
|
||||
const exploreTo = stripAppRoot(exploreURL);
|
||||
const exploreTo = normalizeLegacyDashboardUrl(
|
||||
stripAppRoot(exploreURL),
|
||||
);
|
||||
let titleLink: JSX.Element;
|
||||
if (PREVENT_UNSAFE_DEFAULT_URLS_ON_DATASET) {
|
||||
titleLink = (
|
||||
|
||||
Generated
+5
-160
@@ -24,11 +24,11 @@
|
||||
"@types/lodash-es": "^4.17.12",
|
||||
"@types/node": "^26.2.0",
|
||||
"@types/ws": "^8.18.1",
|
||||
"@typescript-eslint/eslint-plugin": "^8.65.0",
|
||||
"@typescript-eslint/eslint-plugin": "^8.67.0",
|
||||
"@typescript-eslint/parser": "^8.67.0",
|
||||
"eslint": "^10.8.1",
|
||||
"eslint-config-prettier": "^10.1.8",
|
||||
"globals": "^17.9.0",
|
||||
"globals": "^17.10.0",
|
||||
"oxfmt": "^0.63.0",
|
||||
"tscw-config": "^1.1.2",
|
||||
"typescript": "^6.0.3",
|
||||
@@ -1123,136 +1123,6 @@
|
||||
"typescript": ">=4.8.4 <6.1.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/project-service": {
|
||||
"version": "8.67.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.67.0.tgz",
|
||||
"integrity": "sha512-cvE8c7ulYeXN9fYuszhCeCsbzyVEXuhrRCybnBre7TUmqb5nRmBfQAwCj0O3WJFDeyAZt4VYv51vMCC9LHSdYw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/tsconfig-utils": "^8.67.0",
|
||||
"@typescript-eslint/types": "^8.67.0",
|
||||
"debug": "^4.4.3"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"typescript": ">=4.8.4 <6.1.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/scope-manager": {
|
||||
"version": "8.67.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.67.0.tgz",
|
||||
"integrity": "sha512-EgvsleTwS4E+WzzSvem8fAUubLwatMNF1B5hHSLQxcvs7q2dtRhGyujHwLJSYlG41niJ7GP24Aha2+0mb1b2kg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/types": "8.67.0",
|
||||
"@typescript-eslint/visitor-keys": "8.67.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/tsconfig-utils": {
|
||||
"version": "8.67.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/tsconfig-utils/-/tsconfig-utils-8.67.0.tgz",
|
||||
"integrity": "sha512-vV+LUSv5njUWsknE71fqKTlXUva+R76SaeORd6Zojcunk/6DvKFXONU3BrAs2H49mbygUXt6gbYunzwqNwlhdg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"typescript": ">=4.8.4 <6.1.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/types": {
|
||||
"version": "8.67.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-8.67.0.tgz",
|
||||
"integrity": "sha512-sBtgslww8nsMYUjhdPBiSyUqSzT8uR6g93A2QXnQC8+cGdjz0CyaOdqHDRJb1AtORbZCNUJBBeFA/tNR2uQmww==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/typescript-estree": {
|
||||
"version": "8.67.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-8.67.0.tgz",
|
||||
"integrity": "sha512-EKQBCE9yNlRJYm7jdTW5AhDacDUmSwQb0FAJAmK2EKYrNXIsa2vxcSZx6PvJ/dEdI6lS+Y9W+EXckLj0iPFGcw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/project-service": "8.67.0",
|
||||
"@typescript-eslint/tsconfig-utils": "8.67.0",
|
||||
"@typescript-eslint/types": "8.67.0",
|
||||
"@typescript-eslint/visitor-keys": "8.67.0",
|
||||
"debug": "^4.4.3",
|
||||
"minimatch": "^10.2.2",
|
||||
"semver": "^7.7.3",
|
||||
"tinyglobby": "^0.2.15",
|
||||
"ts-api-utils": "^2.5.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"typescript": ">=4.8.4 <6.1.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/parser/node_modules/@typescript-eslint/visitor-keys": {
|
||||
"version": "8.67.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-8.67.0.tgz",
|
||||
"integrity": "sha512-fkv8dHRDqfGtTHuJeebdrQ7cX6Ad4WAS00rgHh9UGvMycF1mjBfsxry1XsLIFhWZ6Judlh6UdzK+TYlbpCXgnA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/types": "8.67.0",
|
||||
"eslint-visitor-keys": "^5.0.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/parser/node_modules/eslint-visitor-keys": {
|
||||
"version": "5.0.1",
|
||||
"resolved": "https://registry.npmjs.org/eslint-visitor-keys/-/eslint-visitor-keys-5.0.1.tgz",
|
||||
"integrity": "sha512-tD40eHxA35h0PEIZNeIjkHoDR4YjjJp34biM0mDvplBe//mB+IHCqHDGV7pxF+7MklTvighcCPPZC7ynWyjdTA==",
|
||||
"dev": true,
|
||||
"license": "Apache-2.0",
|
||||
"engines": {
|
||||
"node": "^20.19.0 || ^22.13.0 || >=24"
|
||||
},
|
||||
"funding": {
|
||||
"url": "https://opencollective.com/eslint"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/project-service": {
|
||||
"version": "8.67.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.67.0.tgz",
|
||||
@@ -2183,9 +2053,9 @@
|
||||
}
|
||||
},
|
||||
"node_modules/globals": {
|
||||
"version": "17.9.0",
|
||||
"resolved": "https://registry.npmjs.org/globals/-/globals-17.9.0.tgz",
|
||||
"integrity": "sha512-m/MvAW61QVU5VDNF1Vj8axt016h8w7L5TU1e9zlab7XIttAT2YAlCwl75K1fOqvMM9apmD7lbCIRhpfkhmxhCg==",
|
||||
"version": "17.10.0",
|
||||
"resolved": "https://registry.npmjs.org/globals/-/globals-17.10.0.tgz",
|
||||
"integrity": "sha512-V0kztuWST2k8A/VbxAY8+L+7+Rgo3fyA24IHRLrZp7HOzJjV0gHSaZUjK9lpP/IrBSNite2tZ1prhRkinRu1CA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
@@ -3364,31 +3234,6 @@
|
||||
"typescript": ">=4.8.4 <6.1.0"
|
||||
}
|
||||
},
|
||||
"node_modules/typescript-eslint/node_modules/@typescript-eslint/parser": {
|
||||
"version": "8.66.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-8.66.0.tgz",
|
||||
"integrity": "sha512-X6ypGChaWYk6PBtUg2BwuTZEFFcHJAtGTVJ9/lCTOufhZ4i9fNolQNnktq+kkMCwMj7V8Svsq7+TxSDslmhE0g==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/scope-manager": "8.66.0",
|
||||
"@typescript-eslint/types": "8.66.0",
|
||||
"@typescript-eslint/typescript-estree": "8.66.0",
|
||||
"@typescript-eslint/visitor-keys": "8.66.0",
|
||||
"debug": "^4.4.3"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"eslint": "^8.57.0 || ^9.0.0 || ^10.0.0",
|
||||
"typescript": ">=4.8.4 <6.1.0"
|
||||
}
|
||||
},
|
||||
"node_modules/undici-types": {
|
||||
"version": "8.3.0",
|
||||
"resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.3.0.tgz",
|
||||
|
||||
@@ -32,11 +32,11 @@
|
||||
"@types/lodash-es": "^4.17.12",
|
||||
"@types/node": "^26.2.0",
|
||||
"@types/ws": "^8.18.1",
|
||||
"@typescript-eslint/eslint-plugin": "^8.65.0",
|
||||
"@typescript-eslint/eslint-plugin": "^8.67.0",
|
||||
"@typescript-eslint/parser": "^8.67.0",
|
||||
"eslint": "^10.8.1",
|
||||
"eslint-config-prettier": "^10.1.8",
|
||||
"globals": "^17.9.0",
|
||||
"globals": "^17.10.0",
|
||||
"oxfmt": "^0.63.0",
|
||||
"tscw-config": "^1.1.2",
|
||||
"typescript": "^6.0.3",
|
||||
|
||||
@@ -957,7 +957,13 @@ class AnnotationDatasource(BaseDatasource):
|
||||
def get_query_str(self, query_obj: QueryObjectDict) -> str:
|
||||
raise NotImplementedError()
|
||||
|
||||
def values_for_column(self, column_name: str, limit: int = 10000) -> list[Any]:
|
||||
def values_for_column(
|
||||
self,
|
||||
column_name: str,
|
||||
limit: int = 10000,
|
||||
denormalize_column: bool = False,
|
||||
array_elements: bool = False,
|
||||
) -> list[Any]:
|
||||
raise NotImplementedError()
|
||||
|
||||
|
||||
|
||||
@@ -418,7 +418,8 @@ class DashboardRestApi(
|
||||
result:
|
||||
type: array
|
||||
items:
|
||||
type: object
|
||||
$ref: >-
|
||||
#/components/schemas/{{self.__class__.__name__}}.get_list
|
||||
400:
|
||||
$ref: '#/components/responses/400'
|
||||
401:
|
||||
|
||||
@@ -133,6 +133,9 @@ class DatasourceRestApi(BaseSupersetApi):
|
||||
|
||||
row_limit = apply_max_row_limit(app.config["FILTER_SELECT_ROW_LIMIT"])
|
||||
denormalize_column = not datasource.normalize_columns
|
||||
# Element-level operators (Contains any / Contains all) request the
|
||||
# distinct array *elements* rather than distinct whole arrays.
|
||||
array_elements = parse_boolean_string(request.args.get("array_elements"))
|
||||
|
||||
# Cache distinct column-value results so a dashboard with many filters
|
||||
# backed by the same (often heavy) virtual dataset doesn't re-execute
|
||||
@@ -165,6 +168,7 @@ class DatasourceRestApi(BaseSupersetApi):
|
||||
"col": column_name,
|
||||
"limit": row_limit,
|
||||
"denorm": denormalize_column,
|
||||
"elements": array_elements,
|
||||
"rls": security_manager.get_rls_cache_key(datasource),
|
||||
"changed_on": str(getattr(datasource, "changed_on", "")),
|
||||
},
|
||||
@@ -189,6 +193,7 @@ class DatasourceRestApi(BaseSupersetApi):
|
||||
column_name=column_name,
|
||||
limit=row_limit,
|
||||
denormalize_column=denormalize_column,
|
||||
array_elements=array_elements,
|
||||
)
|
||||
except KeyError:
|
||||
return self.response(
|
||||
|
||||
@@ -83,7 +83,7 @@ The tables below (generated via `python superset/db_engine_specs/lib.py`) summar
|
||||
| Databricks (legacy) | 70 | Supported | Partial | Supported | Partial | Partial | Not supported |
|
||||
| StarRocks | 69 | Supported | Partial | Supported | Partial | Partial | Partial |
|
||||
| SingleStore | 68 | Supported | Partial | Supported | Not supported | Partial | Not supported |
|
||||
| ClickHouse Connect (Superset) | 61 | Supported | Partial | Partial | Partial | Partial | Not supported |
|
||||
| ClickHouse Connect (Superset) | 62 | Supported | Partial | Supported | Partial | Partial | Not supported |
|
||||
| Google Sheets | 61 | Supported | Partial | Supported | Supported | Partial | Partial |
|
||||
| Aurora MySQL (Data API) | 59 | Supported | Partial | Supported | Partial | Partial | Not supported |
|
||||
| MariaDB | 59 | Supported | Partial | Supported | Partial | Partial | Not supported |
|
||||
@@ -91,7 +91,7 @@ The tables below (generated via `python superset/db_engine_specs/lib.py`) summar
|
||||
| OceanBase | 59 | Supported | Partial | Supported | Partial | Partial | Not supported |
|
||||
| MotherDuck | 58 | Supported | Partial | Supported | Not supported | Partial | Not supported |
|
||||
| KustoSQL | 54 | Supported | Partial | Supported | Partial | Partial | Not supported |
|
||||
| ClickHouse | 51 | Supported | Partial | Partial | Partial | Partial | Not supported |
|
||||
| ClickHouse | 52 | Supported | Partial | Supported | Partial | Partial | Not supported |
|
||||
| Databend | 51 | Supported | Partial | Supported | Partial | Partial | Not supported |
|
||||
| Apache Drill | 50 | Supported | Partial | Supported | Partial | Partial | Partial |
|
||||
| Apache Druid | 47 | Partial | Partial | Supported | Partial | Partial | Not supported |
|
||||
@@ -293,8 +293,8 @@ The tables below (generated via `python superset/db_engine_specs/lib.py`) summar
|
||||
| Aurora MySQL (Data API) | True | True | True | True | True | True | True | True |
|
||||
| Aurora PostgreSQL (Data API) | True | True | True | True | True | True | True | True |
|
||||
| Azure Synapse | True | True | True | True | True | True | True | True |
|
||||
| ClickHouse | False | True | True | True | True | True | True | True |
|
||||
| ClickHouse Connect (Superset) | False | True | True | True | True | True | True | True |
|
||||
| ClickHouse | True | True | True | True | True | True | True | True |
|
||||
| ClickHouse Connect (Superset) | True | True | True | True | True | True | True | True |
|
||||
| CockroachDB | True | True | True | True | True | True | True | True |
|
||||
| Couchbase | True | True | True | True | False | True | True | True |
|
||||
| CrateDB | True | True | True | True | True | True | True | True |
|
||||
|
||||
@@ -55,7 +55,13 @@ from sqlalchemy.engine.reflection import Inspector
|
||||
from sqlalchemy.engine.url import URL
|
||||
from sqlalchemy.ext.compiler import compiles
|
||||
from sqlalchemy.sql import literal_column, quoted_name, text
|
||||
from sqlalchemy.sql.expression import BinaryExpression, ColumnClause, Select, TextClause
|
||||
from sqlalchemy.sql.expression import (
|
||||
BinaryExpression,
|
||||
ColumnClause,
|
||||
ColumnElement,
|
||||
Select,
|
||||
TextClause,
|
||||
)
|
||||
from sqlalchemy.types import TypeEngine
|
||||
|
||||
from superset import db
|
||||
@@ -528,6 +534,11 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
|
||||
time_groupby_inline = False
|
||||
limit_method = LimitMethod.FORCE_LIMIT
|
||||
supports_multivalues_insert = False
|
||||
# Whether this engine supports first-class multi-value (array-typed) columns.
|
||||
# When True, array columns are classified as ``GenericDataType.MULTI_VALUE`` and
|
||||
# the ``array_*`` capability methods below must be implemented. Defaults to
|
||||
# False so engines that have not opted in keep treating arrays as strings.
|
||||
supports_multivalue_columns = False
|
||||
allows_joins = True
|
||||
allows_subqueries = True
|
||||
allows_alias_in_select = True
|
||||
@@ -2571,6 +2582,105 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
|
||||
logger.error(ex, exc_info=True)
|
||||
raise
|
||||
|
||||
@classmethod
|
||||
def array_contains_any(cls, col: ColumnElement, values: list[Any]) -> ColumnElement:
|
||||
"""
|
||||
Build a boolean expression testing whether array column ``col`` contains
|
||||
**any** of ``values`` (element-level membership, like ``IN``). Engines
|
||||
that set ``supports_multivalue_columns = True`` must override this with
|
||||
their native function (e.g. ClickHouse ``hasAny``).
|
||||
|
||||
:param col: SQLAlchemy column element for the array column
|
||||
:param values: element values to look for inside the array
|
||||
:return: a SQLAlchemy boolean expression
|
||||
"""
|
||||
raise NotImplementedError(
|
||||
f"{cls.engine} does not support multi-value (array) columns"
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def array_contains_all(cls, col: ColumnElement, values: list[Any]) -> ColumnElement:
|
||||
"""
|
||||
Build a boolean expression testing whether array column ``col`` contains
|
||||
**all** of ``values``. Engines that set
|
||||
``supports_multivalue_columns = True`` must override this with their
|
||||
native function (e.g. ClickHouse ``hasAll``).
|
||||
|
||||
:param col: SQLAlchemy column element for the array column
|
||||
:param values: element values that must all be present
|
||||
:return: a SQLAlchemy boolean expression
|
||||
"""
|
||||
raise NotImplementedError(
|
||||
f"{cls.engine} does not support multi-value (array) columns"
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def array_length(cls, col: ColumnElement) -> ColumnElement:
|
||||
"""
|
||||
Build a numeric expression returning the number of elements in array
|
||||
column ``col``. Engines that set ``supports_multivalue_columns = True``
|
||||
must override this with their native array-length function. Used both for
|
||||
the ``Length`` filter and the ``Is empty`` / ``Is not empty`` operators.
|
||||
|
||||
:param col: SQLAlchemy column element for the array column
|
||||
:return: a SQLAlchemy numeric expression
|
||||
"""
|
||||
raise NotImplementedError(
|
||||
f"{cls.engine} does not support multi-value (array) columns"
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def array_literal(cls, values: list[Any]) -> ColumnElement:
|
||||
"""
|
||||
Build an array-literal expression from ``values`` (e.g. ClickHouse
|
||||
``array(v1, v2)`` == ``[v1, v2]``). Used for the whole-array (column-
|
||||
level) operators ``=`` / ``!=`` / ``IN`` / ``NOT IN`` where the array is
|
||||
compared as a single value. Engines that set
|
||||
``supports_multivalue_columns = True`` must override this.
|
||||
|
||||
:param values: element values that make up the array
|
||||
:return: a SQLAlchemy array-literal expression
|
||||
"""
|
||||
raise NotImplementedError(
|
||||
f"{cls.engine} does not support multi-value (array) columns"
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def array_explode(cls, col: ColumnElement) -> ColumnElement:
|
||||
"""
|
||||
Build an expression that expands array column ``col`` into one row per
|
||||
element (e.g. ClickHouse ``arrayJoin``). Used to source **element-level**
|
||||
value suggestions (``SELECT DISTINCT array_explode(col)``) for the
|
||||
``Contains any`` / ``Contains all`` filter operators, so the picker offers
|
||||
individual elements rather than whole arrays. Engines that set
|
||||
``supports_multivalue_columns = True`` must override this.
|
||||
|
||||
:param col: SQLAlchemy column element for the array column
|
||||
:return: a SQLAlchemy expression yielding one element per row
|
||||
"""
|
||||
raise NotImplementedError(
|
||||
f"{cls.engine} does not support multi-value (array) columns"
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def get_array_element_type( # pylint: disable=unused-argument
|
||||
cls, native_type: str | None
|
||||
) -> GenericDataType | None:
|
||||
"""
|
||||
Return the generic type of an array column's **element** type, derived
|
||||
from its native type string (e.g. ClickHouse ``Array(Int32)`` ->
|
||||
``NUMERIC``), or ``None`` when the engine has no array support or the
|
||||
element type cannot be resolved.
|
||||
|
||||
Callers use this to coerce filter values to the element type before
|
||||
building array expressions, so, for example, a ``Contains any`` filter on
|
||||
a numeric array compares against numbers rather than quoted strings.
|
||||
|
||||
:param native_type: native column type string of the array column
|
||||
:return: the element's :class:`GenericDataType`, or ``None``
|
||||
"""
|
||||
return None
|
||||
|
||||
@classmethod
|
||||
def get_column_spec( # pylint: disable=unused-argument
|
||||
cls,
|
||||
|
||||
@@ -26,8 +26,9 @@ from flask import current_app as app
|
||||
from flask_babel import gettext as __
|
||||
from marshmallow import fields, Schema
|
||||
from marshmallow.validate import Range
|
||||
from sqlalchemy import types
|
||||
from sqlalchemy import func, types
|
||||
from sqlalchemy.engine.url import URL
|
||||
from sqlalchemy.sql.expression import ColumnElement
|
||||
from urllib3.exceptions import NewConnectionError
|
||||
|
||||
from superset.databases.utils import make_url_safe
|
||||
@@ -55,6 +56,7 @@ class ClickHouseBaseEngineSpec(BaseEngineSpec):
|
||||
|
||||
time_groupby_inline = True
|
||||
supports_multivalues_insert = True
|
||||
supports_multivalue_columns = True
|
||||
|
||||
# ClickHouse doesn't support IS true/false syntax, use = true/false instead
|
||||
use_equality_for_boolean_filters = True
|
||||
@@ -112,6 +114,7 @@ class ClickHouseBaseEngineSpec(BaseEngineSpec):
|
||||
|
||||
_time_grain_expressions = {
|
||||
None: "{col}",
|
||||
"PT1S": "toStartOfSecond(toDateTime64({col}, 3))",
|
||||
"PT1M": "toStartOfMinute(toDateTime({col}))",
|
||||
"PT5M": "toDateTime(intDiv(toUInt32(toDateTime({col})), 300)*300)",
|
||||
"PT10M": "toDateTime(intDiv(toUInt32(toDateTime({col})), 600)*600)",
|
||||
@@ -127,12 +130,18 @@ class ClickHouseBaseEngineSpec(BaseEngineSpec):
|
||||
|
||||
column_type_mappings = (
|
||||
(
|
||||
re.compile(r".*Enum.*", re.IGNORECASE),
|
||||
# Anchor to the start so only top-level arrays match. This must be
|
||||
# ordered before the ``Enum`` entry below: ``Array(Enum8(...))`` is a
|
||||
# real array and should classify as MULTI_VALUE, not STRING. The
|
||||
# anchor also prevents over-matching nested arrays such as
|
||||
# ``Map(String, Array(String))`` or ``Tuple(Array(String))``, which
|
||||
# are not themselves array columns and must keep their own type.
|
||||
re.compile(r"^Array\(", re.IGNORECASE),
|
||||
types.String(),
|
||||
GenericDataType.STRING,
|
||||
GenericDataType.MULTI_VALUE,
|
||||
),
|
||||
(
|
||||
re.compile(r".*Array.*", re.IGNORECASE),
|
||||
re.compile(r".*Enum.*", re.IGNORECASE),
|
||||
types.String(),
|
||||
GenericDataType.STRING,
|
||||
),
|
||||
@@ -173,6 +182,56 @@ class ClickHouseBaseEngineSpec(BaseEngineSpec):
|
||||
),
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def array_contains_any(cls, col: ColumnElement, values: list[Any]) -> ColumnElement:
|
||||
# ClickHouse: hasAny(arr, [v1, v2]) -> 1 if arr shares any element.
|
||||
# func.array(*values) renders as array(v1, v2) == [v1, v2].
|
||||
return func.hasAny(col, func.array(*values))
|
||||
|
||||
@classmethod
|
||||
def array_contains_all(cls, col: ColumnElement, values: list[Any]) -> ColumnElement:
|
||||
# ClickHouse: hasAll(arr, [v1, v2]) -> 1 if arr contains all elements.
|
||||
return func.hasAll(col, func.array(*values))
|
||||
|
||||
@classmethod
|
||||
def array_length(cls, col: ColumnElement) -> ColumnElement:
|
||||
# ClickHouse: length(arr) -> number of elements
|
||||
return func.length(col)
|
||||
|
||||
@classmethod
|
||||
def array_literal(cls, values: list[Any]) -> ColumnElement:
|
||||
# ClickHouse: array(v1, v2) is equivalent to the literal [v1, v2].
|
||||
return func.array(*values)
|
||||
|
||||
@classmethod
|
||||
def array_explode(cls, col: ColumnElement) -> ColumnElement:
|
||||
# ClickHouse: arrayJoin(arr) yields one row per element, so
|
||||
# SELECT DISTINCT arrayJoin(arr) returns the distinct elements.
|
||||
return func.arrayJoin(col)
|
||||
|
||||
# Matches the element type inside a top-level ``Array(...)`` column, e.g.
|
||||
# ``Array(Int32)`` -> ``Int32``, ``Array(Nullable(String))`` -> ``String``.
|
||||
_ARRAY_ELEMENT_RE = re.compile(r"^Array\((?P<inner>.+)\)$", re.IGNORECASE)
|
||||
# Element-type wrappers that don't change the underlying generic type.
|
||||
_ELEMENT_WRAPPER_RE = re.compile(
|
||||
r"^(?:Nullable|LowCardinality)\((?P<inner>.+)\)$", re.IGNORECASE
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def get_array_element_type(cls, native_type: str | None) -> GenericDataType | None:
|
||||
if not native_type:
|
||||
return None
|
||||
match = cls._ARRAY_ELEMENT_RE.match(native_type.strip())
|
||||
if not match:
|
||||
return None
|
||||
inner = match.group("inner").strip()
|
||||
# Peel wrappers (Nullable/LowCardinality) that don't alter the generic
|
||||
# type so the inner scalar type drives classification.
|
||||
while wrapper := cls._ELEMENT_WRAPPER_RE.match(inner):
|
||||
inner = wrapper.group("inner").strip()
|
||||
spec = cls.get_column_spec(inner)
|
||||
return spec.generic_type if spec else None
|
||||
|
||||
@classmethod
|
||||
def epoch_to_dttm(cls) -> str:
|
||||
return "{col}"
|
||||
|
||||
@@ -24,6 +24,8 @@ from re import Pattern
|
||||
from typing import Any, Callable, Optional, TYPE_CHECKING
|
||||
|
||||
from flask_babel import gettext as __
|
||||
from marshmallow import fields, pre_load
|
||||
from marshmallow.validate import Range
|
||||
from sqlalchemy import text, types
|
||||
from sqlalchemy.dialects.postgresql import DOUBLE_PRECISION, ENUM, INTERVAL, JSON
|
||||
from sqlalchemy.dialects.postgresql.base import PGInspector
|
||||
@@ -37,6 +39,9 @@ from superset.db_engine_specs.base import (
|
||||
AURORA_DATA_API_KNOWN_INCOMPATIBILITIES,
|
||||
BaseEngineSpec,
|
||||
BasicParametersMixin,
|
||||
BasicParametersSchema,
|
||||
BasicParametersType,
|
||||
BasicPropertiesType,
|
||||
DatabaseCategory,
|
||||
TimestampExpression,
|
||||
)
|
||||
@@ -46,6 +51,7 @@ from superset.models.sql_lab import Query
|
||||
from superset.sql.parse import process_jinja_sql
|
||||
from superset.utils import core as utils, json
|
||||
from superset.utils.core import GenericDataType, QuerySource
|
||||
from superset.utils.network import is_hostname_valid, is_port_open
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from superset.models.core import Database # pragma: no cover
|
||||
@@ -298,6 +304,34 @@ class PostgresBaseEngineSpec(BaseEngineSpec):
|
||||
return None
|
||||
|
||||
|
||||
class PostgresParametersSchema(BasicParametersSchema):
|
||||
"""
|
||||
Same as ``BasicParametersSchema``, except ``port`` is optional: a blank
|
||||
port falls back to Postgres's own default (5432) in
|
||||
``PostgresEngineSpec.build_sqlalchemy_uri``.
|
||||
"""
|
||||
|
||||
port = fields.Integer(
|
||||
required=False,
|
||||
allow_none=True,
|
||||
metadata={"description": __("Database port")},
|
||||
validate=Range(min=0, max=2**16, max_inclusive=False),
|
||||
)
|
||||
|
||||
@pre_load
|
||||
def blank_port_to_none(self, data: Any, **kwargs: Any) -> Any:
|
||||
"""
|
||||
A cleared number input in the Connect Database form submits ``""``
|
||||
for ``port`` (HTML input values are always strings) rather than
|
||||
omitting the key or sending ``null``. Normalize it to ``None`` so it
|
||||
deserializes cleanly instead of failing with "Not a valid integer.",
|
||||
and is treated as blank -- same as an omitted port -- downstream.
|
||||
"""
|
||||
if isinstance(data, dict) and data.get("port") == "":
|
||||
data = {**data, "port": None}
|
||||
return data
|
||||
|
||||
|
||||
class PostgresEngineSpec(BasicParametersMixin, PostgresBaseEngineSpec):
|
||||
engine = "postgresql"
|
||||
engine_name = "PostgreSQL"
|
||||
@@ -309,6 +343,7 @@ class PostgresEngineSpec(BasicParametersMixin, PostgresBaseEngineSpec):
|
||||
supports_grouping_sets = True
|
||||
|
||||
default_driver = "psycopg2"
|
||||
parameters_schema = PostgresParametersSchema()
|
||||
sqlalchemy_uri_placeholder = (
|
||||
"postgresql://user:password@host:port/dbname[?key=value&key=value...]"
|
||||
)
|
||||
@@ -674,6 +709,113 @@ class PostgresEngineSpec(BasicParametersMixin, PostgresBaseEngineSpec):
|
||||
|
||||
return uri, connect_args
|
||||
|
||||
@classmethod
|
||||
def build_sqlalchemy_uri(
|
||||
cls,
|
||||
parameters: BasicParametersType,
|
||||
encrypted_extra: dict[str, str] | None = None,
|
||||
) -> str:
|
||||
"""
|
||||
Default a missing/blank port to Postgres's own default (5432) so the
|
||||
dynamic form can connect without requiring the port to be filled in.
|
||||
|
||||
Only an absent key, ``None``, or ``""`` (what a cleared number input
|
||||
submits, since this may be called directly with raw, non-schema-
|
||||
loaded parameters -- see ``ValidateDatabaseParametersCommand``) are
|
||||
treated as blank; an explicitly supplied port -- including ``0`` --
|
||||
is preserved as-is rather than overwritten by a truthiness check.
|
||||
"""
|
||||
port = parameters.get("port")
|
||||
resolved_port: int = (
|
||||
cls.metadata["default_port"] if port is None or port == "" else port
|
||||
)
|
||||
parameters_with_default_port: BasicParametersType = {
|
||||
**parameters,
|
||||
"port": resolved_port,
|
||||
}
|
||||
return super().build_sqlalchemy_uri(
|
||||
parameters_with_default_port, encrypted_extra
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def validate_parameters(
|
||||
cls, properties: BasicPropertiesType
|
||||
) -> list[SupersetError]:
|
||||
"""
|
||||
Validates any number of parameters, for progressive validation.
|
||||
|
||||
Same as ``BasicParametersMixin.validate_parameters``, except ``port``
|
||||
is not a required parameter: a blank port is valid, since
|
||||
``build_sqlalchemy_uri`` falls back to Postgres's own default. Port
|
||||
format/range/open checks still run whenever a port is present.
|
||||
"""
|
||||
errors: list[SupersetError] = []
|
||||
|
||||
required = {"host", "username", "database"}
|
||||
parameters = properties.get("parameters", {})
|
||||
present = {key for key in parameters if parameters.get(key, ())}
|
||||
|
||||
if missing := sorted(required - present):
|
||||
errors.append(
|
||||
SupersetError(
|
||||
message=f"One or more parameters are missing: {', '.join(missing)}",
|
||||
error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR,
|
||||
level=ErrorLevel.WARNING,
|
||||
extra={"missing": missing},
|
||||
),
|
||||
)
|
||||
|
||||
host = parameters.get("host", None)
|
||||
if not host:
|
||||
return errors
|
||||
if not is_hostname_valid(host):
|
||||
errors.append(
|
||||
SupersetError(
|
||||
message="The hostname provided can't be resolved.",
|
||||
error_type=SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
|
||||
level=ErrorLevel.ERROR,
|
||||
extra={"invalid": ["host"]},
|
||||
),
|
||||
)
|
||||
return errors
|
||||
|
||||
port = parameters.get("port", None)
|
||||
if not port:
|
||||
return errors
|
||||
try:
|
||||
port = int(port)
|
||||
except (ValueError, TypeError):
|
||||
errors.append(
|
||||
SupersetError(
|
||||
message="Port must be a valid integer.",
|
||||
error_type=SupersetErrorType.CONNECTION_INVALID_PORT_ERROR,
|
||||
level=ErrorLevel.ERROR,
|
||||
extra={"invalid": ["port"]},
|
||||
),
|
||||
)
|
||||
if not (isinstance(port, int) and 0 <= port < 2**16):
|
||||
errors.append(
|
||||
SupersetError(
|
||||
message=(
|
||||
"The port must be an integer between 0 and 65535 (inclusive)."
|
||||
),
|
||||
error_type=SupersetErrorType.CONNECTION_INVALID_PORT_ERROR,
|
||||
level=ErrorLevel.ERROR,
|
||||
extra={"invalid": ["port"]},
|
||||
),
|
||||
)
|
||||
elif not is_port_open(host, port):
|
||||
errors.append(
|
||||
SupersetError(
|
||||
message="The port is closed.",
|
||||
error_type=SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR,
|
||||
level=ErrorLevel.ERROR,
|
||||
extra={"invalid": ["port"]},
|
||||
),
|
||||
)
|
||||
|
||||
return errors
|
||||
|
||||
@staticmethod
|
||||
def mutate_db_for_connection_test(database: Database) -> None:
|
||||
"""
|
||||
|
||||
@@ -68,6 +68,11 @@ from superset.mcp_service.session_scope import _mcp_session_token
|
||||
from superset.mcp_service.utils.error_sanitization import (
|
||||
sanitize_for_log as _sanitize_for_log,
|
||||
)
|
||||
from superset.security.api_key_scopes import (
|
||||
get_resource_scope,
|
||||
METHOD_PERMISSION_SCOPE_ACTION,
|
||||
RESOURCE_SCOPE_NAME as RESOURCE_SCOPE_NAME,
|
||||
)
|
||||
from superset.security.guest_token import GuestUser
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -126,19 +131,24 @@ class MCPNoAuthSourceError(ValueError):
|
||||
# is a privileged, write-class operation and therefore requires the write
|
||||
# scope. When introducing a new method permission, add it here.
|
||||
_METHOD_TO_REQUIRED_SCOPE = {
|
||||
"read": "superset:read",
|
||||
# "get" is the read-class permission FAB registers on its security API
|
||||
# views (User/Role) — those views have no can_read, so tools targeting
|
||||
# them declare method_permission_name="get".
|
||||
"get": "superset:read",
|
||||
"write": "superset:write",
|
||||
"delete": "superset:write",
|
||||
# SQL execution (execute_sql, get_chart_sql) runs arbitrary queries and is
|
||||
# treated as a write-class privileged operation for scope purposes.
|
||||
"execute_sql_query": "superset:write",
|
||||
method: f"superset:{action}"
|
||||
for method, action in METHOD_PERMISSION_SCOPE_ACTION.items()
|
||||
}
|
||||
|
||||
|
||||
def _required_resource_scope(
|
||||
class_permission_name: str, method_permission_name: str
|
||||
) -> str | None:
|
||||
"""Compute the ``superset:<resource>:<action>`` scope string for a tool.
|
||||
|
||||
Returns None if either the resource or the action isn't mapped — callers
|
||||
must treat that as "no per-resource scope available," not as a grant;
|
||||
the flat ``_METHOD_TO_REQUIRED_SCOPE`` fallback still applies in that case
|
||||
(see ``_token_scope_allows``).
|
||||
"""
|
||||
return get_resource_scope(class_permission_name, method_permission_name)
|
||||
|
||||
|
||||
def _get_token_scopes() -> set[str] | None:
|
||||
"""Return the set of scopes on the current JWT access token, or None.
|
||||
|
||||
@@ -154,8 +164,13 @@ def _get_token_scopes() -> set[str] | None:
|
||||
|
||||
try:
|
||||
access_token = get_access_token()
|
||||
except Exception: # noqa: BLE001 - no JWT context for this request
|
||||
return None
|
||||
except Exception: # noqa: BLE001 - fail closed on token-context errors
|
||||
logger.exception("Unable to resolve MCP access-token scopes")
|
||||
# ``None`` means that no scoped credential was presented and enables
|
||||
# legacy RBAC-only behavior. An empty set instead makes every scope
|
||||
# check fail, so an unexpected context error cannot erase restrictions
|
||||
# carried by a credential.
|
||||
return set()
|
||||
|
||||
if access_token is None:
|
||||
return None
|
||||
@@ -167,12 +182,21 @@ def _get_token_scopes() -> set[str] | None:
|
||||
return {str(s) for s in scopes}
|
||||
|
||||
|
||||
def _token_scope_allows(method_permission_name: str) -> bool:
|
||||
def _token_scope_allows(
|
||||
method_permission_name: str, class_permission_name: str | None = None
|
||||
) -> bool:
|
||||
"""Return whether the current token's scopes permit the given method.
|
||||
|
||||
Back-compat: returns True (allow) when the token carries no scopes or there
|
||||
is no JWT context, so deployments not using scopes keep RBAC-only behavior.
|
||||
Only when the token advertises scopes is the mapped required scope enforced.
|
||||
|
||||
The per-resource scope (``superset:<resource>:<action>``, derived via
|
||||
``_required_resource_scope``) is an ALTERNATIVE grant path alongside the
|
||||
flat method scope: a token carrying either the flat scope
|
||||
(e.g. ``superset:read``) or the matching per-resource scope
|
||||
(e.g. ``superset:dashboard:read``) is allowed, so already-issued
|
||||
flat-scoped tokens keep working unchanged.
|
||||
"""
|
||||
token_scopes = _get_token_scopes()
|
||||
if token_scopes is None:
|
||||
@@ -190,7 +214,15 @@ def _token_scope_allows(method_permission_name: str) -> bool:
|
||||
method_permission_name,
|
||||
)
|
||||
return False
|
||||
return required_scope in token_scopes
|
||||
if required_scope in token_scopes:
|
||||
return True
|
||||
if class_permission_name is not None:
|
||||
resource_scope = _required_resource_scope(
|
||||
class_permission_name, method_permission_name
|
||||
)
|
||||
if resource_scope is not None and resource_scope in token_scopes:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
class MCPPermissionDeniedError(PermissionError):
|
||||
@@ -234,12 +266,20 @@ def _log_scope_denial(
|
||||
cyclomatic complexity in check.
|
||||
"""
|
||||
required_scope = _METHOD_TO_REQUIRED_SCOPE.get(method_permission_name)
|
||||
resource_scope = _required_resource_scope(
|
||||
class_permission_name, method_permission_name
|
||||
)
|
||||
scope_desc = (
|
||||
resource_scope
|
||||
or required_scope
|
||||
or f"unmapped method permission '{method_permission_name}'"
|
||||
)
|
||||
if log_denial:
|
||||
logger.warning(
|
||||
"Scope denied for user %s: token lacks required scope "
|
||||
"'%s' for %s on %s (tool: %s)",
|
||||
_sanitize_for_log(g.user.username),
|
||||
required_scope,
|
||||
scope_desc,
|
||||
permission_str,
|
||||
class_permission_name,
|
||||
func.__name__,
|
||||
@@ -248,7 +288,7 @@ def _log_scope_denial(
|
||||
logger.debug(
|
||||
"Tool hidden for user %s: token lacks required scope '%s' (tool: %s)",
|
||||
_sanitize_for_log(g.user.username),
|
||||
required_scope,
|
||||
scope_desc,
|
||||
func.__name__,
|
||||
)
|
||||
|
||||
@@ -354,8 +394,13 @@ def check_tool_permission( # noqa: C901
|
||||
)
|
||||
return False
|
||||
|
||||
method_permission_name = getattr(func, METHOD_PERMISSION_ATTR, "read")
|
||||
class_permission_name = getattr(func, CLASS_PERMISSION_ATTR, None)
|
||||
|
||||
# Token capabilities and user RBAC are independent restrictions.
|
||||
# Disabling RBAC must not discard scopes explicitly carried by a key.
|
||||
if not current_app.config.get("MCP_RBAC_ENABLED", True):
|
||||
return True
|
||||
return _token_scope_allows(method_permission_name, class_permission_name)
|
||||
|
||||
if not hasattr(g, "user") or not g.user:
|
||||
if log_denial:
|
||||
@@ -368,7 +413,6 @@ def check_tool_permission( # noqa: C901
|
||||
)
|
||||
return False
|
||||
|
||||
class_permission_name = getattr(func, CLASS_PERMISSION_ATTR, None)
|
||||
if not class_permission_name:
|
||||
# No RBAC configured for this tool; allow by default. This is a
|
||||
# supported configuration (a protected tool may intentionally
|
||||
@@ -382,9 +426,17 @@ def check_tool_permission( # noqa: C901
|
||||
"class_permission_name; allowing access without an RBAC check",
|
||||
func.__name__,
|
||||
)
|
||||
if not _token_scope_allows(method_permission_name):
|
||||
if log_denial:
|
||||
logger.warning(
|
||||
"Scope denied for permission-less tool %s: token lacks "
|
||||
"flat scope for method %s",
|
||||
func.__name__,
|
||||
method_permission_name,
|
||||
)
|
||||
return False
|
||||
return True
|
||||
|
||||
method_permission_name = getattr(func, METHOD_PERMISSION_ATTR, "read")
|
||||
permission_str = f"{PERMISSION_PREFIX}{method_permission_name}"
|
||||
|
||||
has_permission = security_manager.can_access(
|
||||
@@ -399,7 +451,9 @@ def check_tool_permission( # noqa: C901
|
||||
# advertises scopes. Tokens/deployments that don't use scopes (API keys,
|
||||
# scope-less JWTs, dev-mode) fall through to RBAC-only behavior — see
|
||||
# ``_token_scope_allows``.
|
||||
if has_permission and not _token_scope_allows(method_permission_name):
|
||||
if has_permission and not _token_scope_allows(
|
||||
method_permission_name, class_permission_name
|
||||
):
|
||||
_log_scope_denial(
|
||||
func,
|
||||
method_permission_name,
|
||||
@@ -462,7 +516,7 @@ def is_tool_visible_to_current_user(tool: Any) -> bool:
|
||||
return False
|
||||
|
||||
if not current_app.config.get("MCP_RBAC_ENABLED", True):
|
||||
return True
|
||||
return check_tool_permission(tool_func, log_denial=False)
|
||||
|
||||
from superset.mcp_service.privacy import (
|
||||
tool_requires_data_model_metadata_access,
|
||||
@@ -475,10 +529,6 @@ def is_tool_visible_to_current_user(tool: Any) -> bool:
|
||||
):
|
||||
return False
|
||||
|
||||
class_permission_name = getattr(tool_func, CLASS_PERMISSION_ATTR, None)
|
||||
if not class_permission_name:
|
||||
return True
|
||||
|
||||
return check_tool_permission(tool_func, log_denial=False)
|
||||
|
||||
except (AttributeError, RuntimeError, ValueError):
|
||||
|
||||
@@ -113,15 +113,19 @@ class CompositeTokenVerifier(TokenVerifier):
|
||||
)
|
||||
self._api_key_prefixes = tuple(valid)
|
||||
|
||||
def _validate_api_key_sync(self, token: str) -> str | None:
|
||||
"""Validate an API key against FAB and return the user's username.
|
||||
def _validate_api_key_sync(self, token: str) -> tuple[str, list[str]] | None:
|
||||
"""Validate an API key against FAB and return (username, scopes).
|
||||
|
||||
Runs synchronously inside a thread executor. Pushes a fresh Flask
|
||||
app context so that FAB's SecurityManager can access the database.
|
||||
|
||||
Returns the username on success, or ``None`` if the key is invalid,
|
||||
FAB does not support ``validate_api_key``, or an unexpected error
|
||||
occurs (fail closed).
|
||||
``scopes`` is the key's own ``ApiKey.scopes`` column, parsed from
|
||||
FAB's comma-separated string storage format into a list (empty list
|
||||
if the key has no scopes set, matching the "no scopes advertised"
|
||||
convention used elsewhere in this module and in ``auth.py``).
|
||||
|
||||
Returns ``None`` if the key is invalid, FAB does not support
|
||||
``validate_api_key``, or an unexpected error occurs (fail closed).
|
||||
"""
|
||||
if self._app is None:
|
||||
return None
|
||||
@@ -135,12 +139,21 @@ class CompositeTokenVerifier(TokenVerifier):
|
||||
)
|
||||
return None
|
||||
user = sm.validate_api_key(token)
|
||||
username = user.username if user else None
|
||||
# Unbind the local reference so this frame no longer points at
|
||||
# the raw token (defense-in-depth). Python does not zero the
|
||||
# underlying string memory on rebind.
|
||||
token = "" # noqa: S105
|
||||
return username
|
||||
if user is None:
|
||||
return None
|
||||
username = user.username
|
||||
scopes_str = (
|
||||
sm.get_api_key_scopes(token)
|
||||
if hasattr(sm, "get_api_key_scopes")
|
||||
else None
|
||||
)
|
||||
scopes = (
|
||||
[s.strip() for s in scopes_str.split(",") if s.strip()]
|
||||
if scopes_str
|
||||
else []
|
||||
)
|
||||
token = "" # noqa: S105 -- unbind raw token, defense-in-depth
|
||||
return username, scopes
|
||||
except Exception: # noqa: BLE001 — catch-all: DB errors, FAB internals, etc.
|
||||
logger.warning(
|
||||
"API key transport validation failed unexpectedly; rejecting token",
|
||||
@@ -168,21 +181,25 @@ class CompositeTokenVerifier(TokenVerifier):
|
||||
if any(token.startswith(prefix) for prefix in self._api_key_prefixes):
|
||||
if self._app is not None:
|
||||
loop = asyncio.get_running_loop()
|
||||
username = await loop.run_in_executor(
|
||||
result = await loop.run_in_executor(
|
||||
None, self._validate_api_key_sync, token
|
||||
)
|
||||
if username is None:
|
||||
if result is None:
|
||||
logger.debug(
|
||||
"API key rejected at transport layer (invalid or expired)"
|
||||
)
|
||||
return None
|
||||
username, key_scopes = result
|
||||
logger.debug(
|
||||
"API key validated at transport layer for user=%s", username
|
||||
)
|
||||
return AccessToken(
|
||||
token=token,
|
||||
client_id="api_key",
|
||||
scopes=list(self.required_scopes or []),
|
||||
# Preserve the key's own scopes exactly. An empty list
|
||||
# means "no scopes advertised" and therefore retains the
|
||||
# RBAC-only behavior for existing unscoped API keys.
|
||||
scopes=key_scopes,
|
||||
claims={
|
||||
API_KEY_PASSTHROUGH_CLAIM: True,
|
||||
API_KEY_VALIDATED_USERNAME_CLAIM: username,
|
||||
@@ -190,10 +207,11 @@ class CompositeTokenVerifier(TokenVerifier):
|
||||
)
|
||||
|
||||
# No app configured: fall back to prefix-only pass-through so
|
||||
# ``_resolve_user_from_api_key`` handles DB validation.
|
||||
# NOTE: ``MCP_REQUIRED_SCOPES`` is intentionally not enforced for
|
||||
# API-key auth — FAB API keys do not carry scopes. Authorization is
|
||||
# enforced downstream via ``check_tool_permission`` (RBAC).
|
||||
# ``_resolve_user_from_api_key`` handles DB validation. Without an
|
||||
# app there is no DB access here, so the key's own ApiKey.scopes
|
||||
# cannot be read — the verifier-global required_scopes are used
|
||||
# instead. Authorization is still enforced downstream via
|
||||
# ``check_tool_permission`` (RBAC).
|
||||
logger.debug("API key token detected (prefix match), passing through")
|
||||
return AccessToken(
|
||||
token=token,
|
||||
|
||||
@@ -63,6 +63,7 @@ from superset.mcp_service.utils import (
|
||||
sanitize_for_llm_context,
|
||||
)
|
||||
from superset.mcp_service.utils.response_utils import humanize_timestamp
|
||||
from superset.sql.parse import has_aggregate
|
||||
from superset.utils import json
|
||||
|
||||
|
||||
@@ -386,13 +387,27 @@ class CreateDatasetMetric(BaseModel):
|
||||
"""Metric definition for dataset creation."""
|
||||
|
||||
metric_name: str = Field(..., description="Name of the metric")
|
||||
expression: str = Field(..., description="SQL expression for the metric")
|
||||
expression: str = Field(
|
||||
...,
|
||||
description="Aggregate SQL expression for the metric, e.g. SUM(amount)",
|
||||
)
|
||||
verbose_name: str | None = None
|
||||
description: str | None = None
|
||||
metric_type: str | None = None
|
||||
d3format: str | None = None
|
||||
warning_text: str | None = None
|
||||
|
||||
@field_validator("expression")
|
||||
@classmethod
|
||||
def expression_must_aggregate(cls, value: str) -> str:
|
||||
if not has_aggregate(value):
|
||||
raise ValueError(
|
||||
"saved metrics must aggregate rows; wrap a row-level column in "
|
||||
"an aggregate such as MAX(column), or omit the saved metric and "
|
||||
"use the dataset column directly"
|
||||
)
|
||||
return value
|
||||
|
||||
|
||||
class CreateDatasetCalculatedColumn(BaseModel):
|
||||
"""Calculated column definition for dataset creation."""
|
||||
|
||||
@@ -21,6 +21,7 @@ from typing import Any
|
||||
from fastmcp import Context
|
||||
from superset_core.mcp.decorators import tool, ToolAnnotations
|
||||
|
||||
from superset.exceptions import SupersetGenericDBErrorException
|
||||
from superset.extensions import event_logger
|
||||
from superset.mcp_service.dataset.schemas import (
|
||||
CreateVirtualDatasetRequest,
|
||||
@@ -67,14 +68,17 @@ def _cleanup_failed_dataset(dataset_id: int) -> None:
|
||||
|
||||
|
||||
def _update_virtual_dataset(dataset_id: int, update_props: dict[str, Any]) -> Any:
|
||||
from superset.commands.dataset.exceptions import DatasetUpdateFailedError
|
||||
from superset.commands.dataset.exceptions import (
|
||||
DatasetInvalidError,
|
||||
DatasetUpdateFailedError,
|
||||
)
|
||||
from superset.commands.dataset.update import UpdateDatasetCommand
|
||||
|
||||
try:
|
||||
return UpdateDatasetCommand(dataset_id, update_props).run()
|
||||
except Exception as exc:
|
||||
_cleanup_failed_dataset(dataset_id)
|
||||
if not isinstance(exc, DatasetUpdateFailedError):
|
||||
if not isinstance(exc, (DatasetInvalidError, DatasetUpdateFailedError)):
|
||||
raise DatasetUpdateFailedError() from exc
|
||||
raise
|
||||
|
||||
@@ -89,7 +93,7 @@ def _update_virtual_dataset(dataset_id: int, update_props: dict[str, Any]) -> An
|
||||
destructiveHint=False,
|
||||
),
|
||||
)
|
||||
async def create_virtual_dataset(
|
||||
async def create_virtual_dataset( # noqa: C901
|
||||
request: CreateVirtualDatasetRequest, ctx: Context
|
||||
) -> CreateVirtualDatasetResponse:
|
||||
"""Save a SQL query as a virtual dataset so it can be charted.
|
||||
@@ -213,6 +217,18 @@ async def create_virtual_dataset(
|
||||
url=None,
|
||||
error=f"Failed to update dataset metadata (creation rolled back): {exc}",
|
||||
)
|
||||
except SupersetGenericDBErrorException as exc:
|
||||
logger.warning("Virtual dataset SQL validation failed", exc_info=True)
|
||||
await ctx.warning(f"Virtual dataset SQL failed validation: {exc}")
|
||||
return CreateVirtualDatasetResponse(
|
||||
id=None,
|
||||
dataset_name=request.dataset_name,
|
||||
sql=request.sql,
|
||||
database_id=request.database_id,
|
||||
columns=[],
|
||||
url=None,
|
||||
error=f"Dataset SQL could not be executed: {exc}",
|
||||
)
|
||||
except Exception as exc:
|
||||
await ctx.error(
|
||||
f"Unexpected error creating virtual dataset: {type(exc).__name__}: {exc}"
|
||||
|
||||
@@ -653,10 +653,9 @@ def _build_composite_verifier(
|
||||
if api_key_enabled:
|
||||
if required_scopes := app.config.get("MCP_REQUIRED_SCOPES", []):
|
||||
logger.warning(
|
||||
"MCP_REQUIRED_SCOPES is configured but API key tokens bypass "
|
||||
"scope enforcement. API key holders gain access regardless of "
|
||||
"MCP_REQUIRED_SCOPES=%r. Enforce per-key authorization via FAB "
|
||||
"roles/RBAC instead.",
|
||||
"MCP_REQUIRED_SCOPES=%r is configured, but API key tokens use "
|
||||
"the scopes stored on each key instead. Unscoped API keys "
|
||||
"retain legacy RBAC-only behavior.",
|
||||
required_scopes,
|
||||
)
|
||||
raw_prefixes: str | Sequence[str] = app.config.get(
|
||||
|
||||
@@ -30,7 +30,7 @@ from fastmcp import Context
|
||||
from superset_core.mcp.decorators import tool, ToolAnnotations
|
||||
|
||||
from superset.extensions import event_logger
|
||||
from superset.mcp_service.auth import MCPPermissionDeniedError
|
||||
from superset.mcp_service.auth import _token_scope_allows, MCPPermissionDeniedError
|
||||
from superset.mcp_service.common.schema_discovery import (
|
||||
CHART_DEFAULT_COLUMNS,
|
||||
CHART_SEARCH_COLUMNS,
|
||||
@@ -235,9 +235,10 @@ async def get_schema(
|
||||
|
||||
from superset import security_manager
|
||||
|
||||
if current_app.config.get("MCP_RBAC_ENABLED", True) and not (
|
||||
security_manager.can_access("can_read", class_permission)
|
||||
):
|
||||
rbac_allows = not current_app.config.get(
|
||||
"MCP_RBAC_ENABLED", True
|
||||
) or security_manager.can_access("can_read", class_permission)
|
||||
if not (rbac_allows and _token_scope_allows("read", class_permission)):
|
||||
user_str = getattr(getattr(g, "user", None), "username", None)
|
||||
logger.warning(
|
||||
"get_schema RBAC denied: user=%s type=%s view=%s",
|
||||
|
||||
+207
-3
@@ -19,6 +19,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
import builtins
|
||||
import copy
|
||||
import dataclasses
|
||||
@@ -417,6 +418,52 @@ UUID_NATIVE_TYPE_RE: re.Pattern[str] = re.compile(
|
||||
)
|
||||
|
||||
|
||||
def parse_array_literal(value: Any) -> list[Any]:
|
||||
"""
|
||||
Parse a user-entered array literal (e.g. ``['a', 'b']`` or ``[1, 2]``) into a
|
||||
list of elements, for the whole-array (column-level) array operators.
|
||||
|
||||
Accepts either an actual list/tuple, a bracketed literal string (parsed with
|
||||
``ast.literal_eval``), or a plain scalar (wrapped into a single-element list).
|
||||
Falls back to a single-element list when the string is not a valid literal.
|
||||
"""
|
||||
if isinstance(value, (list, tuple)):
|
||||
return list(value)
|
||||
if isinstance(value, str):
|
||||
stripped = value.strip()
|
||||
if stripped.startswith("[") and stripped.endswith("]"):
|
||||
try:
|
||||
parsed = ast.literal_eval(stripped)
|
||||
except (ValueError, SyntaxError):
|
||||
parsed = None
|
||||
if isinstance(parsed, (list, tuple)):
|
||||
return list(parsed)
|
||||
return [value]
|
||||
|
||||
|
||||
def coerce_array_values(
|
||||
values: list[Any], element_type: Optional[utils.GenericDataType]
|
||||
) -> list[Any]:
|
||||
"""
|
||||
Coerce array-element ``values`` to the array column's element type so the
|
||||
emitted literal matches the column. Array columns map to a SQLAlchemy
|
||||
``String`` type, so values arrive as strings and would otherwise build
|
||||
string literals (e.g. ``array('5')``) that fail against a numeric array on
|
||||
the server. Numeric elements are cast to numbers and boolean elements to
|
||||
booleans; every other element type (string, temporal, enum, unknown) is left
|
||||
untouched.
|
||||
|
||||
:param values: element values entered for an array filter
|
||||
:param element_type: the array's element :class:`GenericDataType`, or None
|
||||
:return: the coerced values
|
||||
"""
|
||||
if element_type == utils.GenericDataType.NUMERIC:
|
||||
return [utils.cast_to_num(v) if isinstance(v, str) else v for v in values]
|
||||
if element_type == utils.GenericDataType.BOOLEAN:
|
||||
return [utils.cast_to_boolean(v) if isinstance(v, str) else v for v in values]
|
||||
return values
|
||||
|
||||
|
||||
def is_uuid_native_type(native_type: Optional[str]) -> bool:
|
||||
"""
|
||||
Return True if a native column type represents a UUID.
|
||||
@@ -3652,6 +3699,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
|
||||
column_name: str,
|
||||
limit: int = 10000,
|
||||
denormalize_column: bool = False,
|
||||
array_elements: bool = False,
|
||||
) -> list[Any]:
|
||||
# denormalize column name before querying for values
|
||||
# unless disabled in the dataset configuration
|
||||
@@ -3666,13 +3714,25 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
|
||||
tp = self.get_template_processor()
|
||||
tbl, cte = self.get_from_clause(tp)
|
||||
|
||||
db_engine_spec = self.database.db_engine_spec
|
||||
value_expr = target_col.get_sqla_col(template_processor=tp)
|
||||
# For element-level operators (Contains any / Contains all) on a
|
||||
# multi-value (array) column, suggest the distinct **elements** rather
|
||||
# than distinct whole arrays by expanding the array first (e.g. ClickHouse
|
||||
# arrayJoin). Only when the engine supports arrays and the column is
|
||||
# actually an array column; otherwise fall back to whole-value suggestions.
|
||||
if array_elements and db_engine_spec.supports_multivalue_columns:
|
||||
col_spec = db_engine_spec.get_column_spec(native_type=target_col.type)
|
||||
if col_spec and col_spec.generic_type == GenericDataType.MULTI_VALUE:
|
||||
value_expr = db_engine_spec.array_explode(value_expr)
|
||||
|
||||
qry = (
|
||||
sa.select(
|
||||
# The alias (label) here is important because some dialects will
|
||||
# automatically add a random alias to the projection because of the
|
||||
# call to DISTINCT; others will uppercase the column names. This
|
||||
# gives us a deterministic column name in the dataframe.
|
||||
target_col.get_sqla_col(template_processor=tp).label("column_values")
|
||||
value_expr.label("column_values")
|
||||
)
|
||||
.select_from(tbl)
|
||||
.distinct()
|
||||
@@ -4359,7 +4419,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
|
||||
elif is_adhoc_column(flt_col):
|
||||
try:
|
||||
sqla_col, adhoc_generic_type = self.adhoc_column_to_sqla(
|
||||
flt_col,
|
||||
cast("AdhocColumn", flt_col),
|
||||
force_type_check=True,
|
||||
template_processor=template_processor,
|
||||
)
|
||||
@@ -4433,9 +4493,21 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
|
||||
sqla_col = Grouping(sqla_col)
|
||||
col_type = col_obj.type if col_obj else None
|
||||
col_spec = db_engine_spec.get_column_spec(native_type=col_type)
|
||||
is_multivalue_col = bool(
|
||||
col_spec and col_spec.generic_type == GenericDataType.MULTI_VALUE
|
||||
)
|
||||
# Element type of an array column (e.g. Array(Int32) -> NUMERIC),
|
||||
# used to coerce filter values before building array expressions.
|
||||
array_element_type = (
|
||||
db_engine_spec.get_array_element_type(col_type)
|
||||
if is_multivalue_col
|
||||
else None
|
||||
)
|
||||
is_list_target = op in (
|
||||
utils.FilterOperator.IN,
|
||||
utils.FilterOperator.NOT_IN,
|
||||
utils.FilterOperator.CONTAINS_ANY,
|
||||
utils.FilterOperator.CONTAINS_ALL,
|
||||
)
|
||||
|
||||
col_advanced_data_type = col_obj.advanced_data_type if col_obj else ""
|
||||
@@ -4490,7 +4562,56 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
|
||||
sqla_col, op, bus_resp["values"]
|
||||
)
|
||||
)
|
||||
elif is_list_target:
|
||||
elif is_multivalue_col and op in {
|
||||
utils.FilterOperator.EQUALS,
|
||||
utils.FilterOperator.NOT_EQUALS,
|
||||
utils.FilterOperator.IN,
|
||||
utils.FilterOperator.NOT_IN,
|
||||
}:
|
||||
# Whole-array (column-level) comparison against array
|
||||
# literal(s). The value is a pasted array literal like
|
||||
# ``['a', 'b']`` (parsed into elements): ``col = ['a', 'b']``
|
||||
# for = / !=; for IN / NOT IN each entered value is one such
|
||||
# array literal (``col IN (['a'], ['b'])``).
|
||||
if op in {
|
||||
utils.FilterOperator.EQUALS,
|
||||
utils.FilterOperator.NOT_EQUALS,
|
||||
}:
|
||||
literal = db_engine_spec.array_literal(
|
||||
coerce_array_values(
|
||||
parse_array_literal(val), array_element_type
|
||||
)
|
||||
)
|
||||
cond = (
|
||||
sqla_col != literal
|
||||
if op == utils.FilterOperator.NOT_EQUALS
|
||||
else sqla_col == literal
|
||||
)
|
||||
else:
|
||||
candidates: list[Any] = (
|
||||
list(val) if isinstance(val, (list, tuple)) else [val]
|
||||
)
|
||||
cond = sqla_col.in_(
|
||||
[
|
||||
db_engine_spec.array_literal(
|
||||
coerce_array_values(
|
||||
parse_array_literal(candidate),
|
||||
array_element_type,
|
||||
)
|
||||
)
|
||||
for candidate in candidates
|
||||
]
|
||||
)
|
||||
if op == utils.FilterOperator.NOT_IN:
|
||||
cond = ~cond
|
||||
target_clause_list.append(cond)
|
||||
elif op in {
|
||||
utils.FilterOperator.IN,
|
||||
utils.FilterOperator.NOT_IN,
|
||||
}:
|
||||
# CONTAINS_ANY/CONTAINS_ALL also produce a list ``eq`` (they
|
||||
# are in ``is_list_target``), but are element-level array ops
|
||||
# handled by their own branch below — not IN.
|
||||
assert isinstance(eq, (tuple, list))
|
||||
if len(eq) == 0:
|
||||
raise QueryObjectValidationError(
|
||||
@@ -4529,6 +4650,57 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
|
||||
target_clause_list.append(
|
||||
db_engine_spec.handle_null_filter(sqla_col, op)
|
||||
)
|
||||
elif op in {
|
||||
utils.FilterOperator.IS_EMPTY,
|
||||
utils.FilterOperator.IS_NOT_EMPTY,
|
||||
}:
|
||||
# Element-level array operators: length(col) == 0 / > 0.
|
||||
if target_generic_type != GenericDataType.MULTI_VALUE:
|
||||
raise QueryObjectValidationError(
|
||||
_(
|
||||
"The %(op)s operator is only supported for "
|
||||
"multi-value (array) columns.",
|
||||
op=op,
|
||||
)
|
||||
)
|
||||
length_expr = db_engine_spec.array_length(sqla_col)
|
||||
if op == utils.FilterOperator.IS_EMPTY:
|
||||
target_clause_list.append(length_expr == 0)
|
||||
else:
|
||||
target_clause_list.append(length_expr > 0)
|
||||
elif op in {
|
||||
utils.FilterOperator.LENGTH_EQUALS,
|
||||
utils.FilterOperator.LENGTH_GREATER_THAN,
|
||||
utils.FilterOperator.LENGTH_LESS_THAN,
|
||||
utils.FilterOperator.LENGTH_GREATER_THAN_OR_EQUALS,
|
||||
utils.FilterOperator.LENGTH_LESS_THAN_OR_EQUALS,
|
||||
}:
|
||||
# Length filter: compare the array's element count to a
|
||||
# number, e.g. length(col) > 2.
|
||||
if target_generic_type != GenericDataType.MULTI_VALUE:
|
||||
raise QueryObjectValidationError(
|
||||
_(
|
||||
"The %(op)s operator is only supported for "
|
||||
"multi-value (array) columns.",
|
||||
op=op,
|
||||
)
|
||||
)
|
||||
number = utils.cast_to_num(eq) # type: ignore[arg-type]
|
||||
if number is None:
|
||||
raise QueryObjectValidationError(
|
||||
_("The Length filter requires a numeric value.")
|
||||
)
|
||||
length_expr = db_engine_spec.array_length(sqla_col)
|
||||
length_comparisons = {
|
||||
utils.FilterOperator.LENGTH_EQUALS: length_expr == number,
|
||||
utils.FilterOperator.LENGTH_GREATER_THAN: length_expr > number,
|
||||
utils.FilterOperator.LENGTH_LESS_THAN: length_expr < number,
|
||||
utils.FilterOperator.LENGTH_GREATER_THAN_OR_EQUALS: length_expr
|
||||
>= number,
|
||||
utils.FilterOperator.LENGTH_LESS_THAN_OR_EQUALS: length_expr
|
||||
<= number,
|
||||
}
|
||||
target_clause_list.append(length_comparisons[op])
|
||||
elif op == utils.FilterOperator.IS_TRUE:
|
||||
target_clause_list.append(
|
||||
db_engine_spec.handle_boolean_filter(sqla_col, op, True)
|
||||
@@ -4586,6 +4758,38 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
|
||||
target_clause_list.append(sqla_col.not_like(eq))
|
||||
else:
|
||||
target_clause_list.append(sqla_col.not_ilike(eq))
|
||||
elif op in {
|
||||
utils.FilterOperator.CONTAINS_ANY,
|
||||
utils.FilterOperator.CONTAINS_ALL,
|
||||
}:
|
||||
# Element-level array membership. Enforce the target is
|
||||
# actually a multi-value (array) column (only classified
|
||||
# MULTI_VALUE on an array-capable engine), guarding against
|
||||
# payloads that bypass the UI gating.
|
||||
if target_generic_type != GenericDataType.MULTI_VALUE:
|
||||
raise QueryObjectValidationError(
|
||||
_(
|
||||
"The %(op)s operator is only supported for "
|
||||
"multi-value (array) columns.",
|
||||
op=op,
|
||||
)
|
||||
)
|
||||
array_values: list[Any] = coerce_array_values(
|
||||
list(eq) if isinstance(eq, (list, tuple)) else [eq],
|
||||
array_element_type,
|
||||
)
|
||||
if op == utils.FilterOperator.CONTAINS_ANY:
|
||||
target_clause_list.append(
|
||||
db_engine_spec.array_contains_any(
|
||||
sqla_col, array_values
|
||||
)
|
||||
)
|
||||
else:
|
||||
target_clause_list.append(
|
||||
db_engine_spec.array_contains_all(
|
||||
sqla_col, array_values
|
||||
)
|
||||
)
|
||||
elif (
|
||||
op == utils.FilterOperator.TEMPORAL_RANGE
|
||||
and isinstance(eq, str)
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
# Licensed to the Apache Software Foundation (ASF) under one
|
||||
# or more contributor license agreements. See the NOTICE file
|
||||
# distributed with this work for additional information
|
||||
# regarding copyright ownership. The ASF licenses this file
|
||||
# to you under the Apache License, Version 2.0 (the
|
||||
# "License"); you may not use this file except in compliance
|
||||
# with the License. You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing,
|
||||
# software distributed under the License is distributed on an
|
||||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
# KIND, either express or implied. See the License for the
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
"""Canonical resource and action mappings for scoped API keys."""
|
||||
|
||||
# Map FAB method permissions used by MCP tools to the coarser actions supported
|
||||
# by API-key scopes. Keep this explicit so an unknown permission fails closed.
|
||||
METHOD_PERMISSION_SCOPE_ACTION: dict[str, str] = {
|
||||
"read": "read",
|
||||
"get": "read",
|
||||
"write": "write",
|
||||
"update": "write",
|
||||
"delete": "write",
|
||||
"execute_sql_query": "write",
|
||||
}
|
||||
|
||||
# Map MCP/FAB class permission names to stable public resource slugs. These
|
||||
# cannot be derived by lowercasing because several names contain spaces or use
|
||||
# public spellings that differ from their internal class names.
|
||||
RESOURCE_SCOPE_NAME: dict[str, str] = {
|
||||
"Annotation": "annotation",
|
||||
"Chart": "chart",
|
||||
"Dashboard": "dashboard",
|
||||
"Database": "database",
|
||||
"Dataset": "dataset",
|
||||
"Explore": "explore",
|
||||
"Query": "query",
|
||||
"ReportSchedule": "report",
|
||||
"Role": "role",
|
||||
"Row Level Security": "rls",
|
||||
"SavedQuery": "savedquery",
|
||||
"SQLLab": "sqllab",
|
||||
"Tag": "tag",
|
||||
"Task": "task",
|
||||
"Theme": "theme",
|
||||
"User": "user",
|
||||
}
|
||||
|
||||
RESOURCE_SCOPE_CLASS: dict[str, str] = {
|
||||
resource: class_name for class_name, resource in RESOURCE_SCOPE_NAME.items()
|
||||
}
|
||||
RESOURCE_SCOPE_ACTIONS: frozenset[str] = frozenset(
|
||||
METHOD_PERMISSION_SCOPE_ACTION.values()
|
||||
)
|
||||
SCOPE_ACTION_METHOD_PERMISSIONS: dict[str, tuple[str, ...]] = {
|
||||
action: tuple(
|
||||
method
|
||||
for method, mapped_action in METHOD_PERMISSION_SCOPE_ACTION.items()
|
||||
if mapped_action == action
|
||||
)
|
||||
for action in RESOURCE_SCOPE_ACTIONS
|
||||
}
|
||||
|
||||
|
||||
def get_resource_scope(
|
||||
class_permission_name: str, method_permission_name: str
|
||||
) -> str | None:
|
||||
"""Return the resource scope required by a FAB class/method permission."""
|
||||
resource = RESOURCE_SCOPE_NAME.get(class_permission_name)
|
||||
action = METHOD_PERMISSION_SCOPE_ACTION.get(method_permission_name)
|
||||
if resource is None or action is None:
|
||||
return None
|
||||
return f"superset:{resource}:{action}"
|
||||
+213
-22
@@ -17,6 +17,7 @@
|
||||
# pylint: disable=too-many-lines
|
||||
"""A set of constants and methods to manage permissions and security"""
|
||||
|
||||
import datetime
|
||||
import logging
|
||||
import re
|
||||
import time
|
||||
@@ -36,7 +37,7 @@ from urllib.parse import quote
|
||||
|
||||
from flask import current_app, Flask, g, has_app_context, Request, Response
|
||||
from flask_appbuilder import Model
|
||||
from flask_appbuilder.api import expose, protect, safe
|
||||
from flask_appbuilder.api import expose, permission_name, protect, safe
|
||||
from flask_appbuilder.models.filters import BaseFilter
|
||||
from flask_appbuilder.security.manager import AUTH_REMOTE_USER
|
||||
from flask_appbuilder.security.sqla.apis import GroupApi, RoleApi, UserApi
|
||||
@@ -394,8 +395,11 @@ class SupersetUserApi(UserApi):
|
||||
"""
|
||||
Overriding the UserApi to sync Subject rows, filter excluded users,
|
||||
handle deletion constraints, and add audit logging.
|
||||
UserApi has custom post/put that bypass hooks, so we override them
|
||||
and sync after the parent method succeeds.
|
||||
|
||||
The Subject sync happens in ``pre_add``/``pre_update``, which FAB calls
|
||||
*before* the commit that ``self.datamodel.add``/``edit`` issues -- so the
|
||||
sync rides that same commit rather than needing one of its own after the
|
||||
fact.
|
||||
"""
|
||||
|
||||
base_filters = [["username", ExcludeUsersFilter, lambda: []]]
|
||||
@@ -415,6 +419,45 @@ class SupersetUserApi(UserApi):
|
||||
"changed_on",
|
||||
]
|
||||
|
||||
def pre_add(self, item: Model) -> None:
|
||||
"""Hash the password (FAB's own ``pre_add``), then sync the user's
|
||||
``Subject`` row before FAB's own commit.
|
||||
|
||||
``UserApi.post`` calls ``pre_add`` *before* ``self.datamodel.add``,
|
||||
which is what actually issues the commit -- so flushing the new user
|
||||
here (to obtain its id) and syncing its ``Subject`` row alongside it
|
||||
means both writes ride the same transaction and commit together,
|
||||
instead of the subject sync needing a second, separate commit after
|
||||
the fact.
|
||||
"""
|
||||
super().pre_add(item)
|
||||
from superset.daos.user import UserDAO
|
||||
|
||||
self.datamodel.session.add(item)
|
||||
self.datamodel.session.flush()
|
||||
UserDAO._sync_subject(item)
|
||||
|
||||
def pre_update(self, item: Model, data: dict[str, Any]) -> None:
|
||||
"""Same reasoning as ``pre_add``: ``UserApi.put`` calls ``pre_update``
|
||||
before ``self.datamodel.edit`` commits, so the subject sync lands in
|
||||
that same transaction.
|
||||
"""
|
||||
super().pre_update(item, data)
|
||||
from superset.daos.user import UserDAO
|
||||
|
||||
UserDAO._sync_subject(item)
|
||||
|
||||
if data.get("password"):
|
||||
# An admin-initiated password change via this endpoint must
|
||||
# invalidate the target account's other outstanding sessions,
|
||||
# the same as the self-service ``/me/`` path and the two
|
||||
# password-reset views.
|
||||
from superset.security.session_invalidation import (
|
||||
invalidate_sessions_for_user,
|
||||
)
|
||||
|
||||
invalidate_sessions_for_user(item.id)
|
||||
|
||||
@expose("/", methods=["POST"])
|
||||
@protect()
|
||||
@safe
|
||||
@@ -430,17 +473,7 @@ class SupersetUserApi(UserApi):
|
||||
500:
|
||||
description: Server error
|
||||
"""
|
||||
response = super().post()
|
||||
if response.status_code == 201:
|
||||
from superset.daos.user import UserDAO
|
||||
|
||||
user_id = response.json.get("id")
|
||||
if user_id:
|
||||
user = self.datamodel.session.get(self.datamodel.obj, user_id)
|
||||
if user:
|
||||
UserDAO._sync_subject(user)
|
||||
self.datamodel.session.commit() # pylint: disable=consider-using-transaction
|
||||
return response
|
||||
return super().post()
|
||||
|
||||
@expose("/<pk>", methods=["PUT"])
|
||||
@protect()
|
||||
@@ -464,15 +497,42 @@ class SupersetUserApi(UserApi):
|
||||
500:
|
||||
description: Server error
|
||||
"""
|
||||
response = super().put(pk)
|
||||
if response.status_code == 200:
|
||||
from superset.daos.user import UserDAO
|
||||
return super().put(pk)
|
||||
|
||||
user = self.datamodel.get(pk, self._base_filters)
|
||||
if user:
|
||||
UserDAO._sync_subject(user)
|
||||
self.datamodel.session.commit() # pylint: disable=consider-using-transaction
|
||||
return response
|
||||
@expose("/<int:pk>/sessions", methods=["DELETE"])
|
||||
@protect()
|
||||
@permission_name("put")
|
||||
@safe
|
||||
def terminate_sessions(self, pk: int) -> Response:
|
||||
"""Terminate a user's outstanding sessions without disabling their account.
|
||||
---
|
||||
delete:
|
||||
parameters:
|
||||
- in: path
|
||||
name: pk
|
||||
schema:
|
||||
type: integer
|
||||
responses:
|
||||
200:
|
||||
description: Sessions terminated
|
||||
404:
|
||||
$ref: '#/components/responses/404'
|
||||
500:
|
||||
$ref: '#/components/responses/500'
|
||||
"""
|
||||
from superset.security.session_invalidation import invalidate_sessions_for_user
|
||||
|
||||
user = self.datamodel.get(pk, self._base_filters)
|
||||
if not user:
|
||||
return self.response_404()
|
||||
|
||||
invalidate_sessions_for_user(user.id)
|
||||
self.datamodel.session.commit() # pylint: disable=consider-using-transaction
|
||||
_log_audit_event(
|
||||
"UserSessionsTerminated",
|
||||
{"target_username": user.username, "target_user_id": user.id},
|
||||
)
|
||||
return self.response(200, message="User sessions terminated.")
|
||||
|
||||
def pre_delete(self, item: Model) -> None:
|
||||
from superset.daos.user import UserDAO
|
||||
@@ -1565,9 +1625,23 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
|
||||
bypassed. We distinguish the two by comparing the acting user
|
||||
(``g.user``) against the target ``userid``: they match for a
|
||||
self-service reset and differ for an admin reset.
|
||||
|
||||
Also stamps the session-invalidation epoch for the target user, so
|
||||
any session for the account that predates this reset stops working --
|
||||
regardless of which of the two paths triggered it.
|
||||
"""
|
||||
super().reset_password(userid, password)
|
||||
|
||||
# pylint: disable=import-outside-toplevel
|
||||
from superset import db
|
||||
from superset.security.session_invalidation import invalidate_sessions_for_user
|
||||
|
||||
invalidate_sessions_for_user(int(userid))
|
||||
# ``super().reset_password`` (FAB's ``update_user``) already committed
|
||||
# its own change in a separate transaction, so the epoch stamp above
|
||||
# needs its own commit too, rather than riding an existing one.
|
||||
db.session.commit() # pylint: disable=consider-using-transaction
|
||||
|
||||
acting_user = getattr(g, "user", None)
|
||||
acting_user_id = getattr(acting_user, "id", None)
|
||||
# ``userid`` arrives as a string (the ``pk`` request arg) on the admin
|
||||
@@ -4945,6 +5019,123 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
|
||||
raw_token, secret, algorithms=[algo], audience=audience
|
||||
)
|
||||
|
||||
def get_api_key_scopes(self, api_key_string: str) -> Optional[str]:
|
||||
"""Return the ``scopes`` value for a validated API key.
|
||||
|
||||
FAB's ``validate_api_key`` resolves the matching ``ApiKey`` row
|
||||
internally (by lookup hash) but only returns the associated
|
||||
``User`` — the row's ``scopes`` column is otherwise unreachable by
|
||||
callers. This repeats the same cheap, indexed lookup so MCP's
|
||||
``CompositeTokenVerifier`` can propagate per-key scopes instead of
|
||||
silently falling back to verifier-global scopes. Call only after
|
||||
``validate_api_key`` has already succeeded for this token — this
|
||||
method does not itself verify the key hash or active status.
|
||||
"""
|
||||
lookup = self._compute_lookup_hash(api_key_string) # type: ignore[attr-defined]
|
||||
api_key = (
|
||||
self.session.query(self.api_key_model) # type: ignore[attr-defined]
|
||||
.filter(self.api_key_model.lookup_hash == lookup)
|
||||
.one_or_none()
|
||||
)
|
||||
return api_key.scopes if api_key else None
|
||||
|
||||
def _validate_requested_api_key_scopes(
|
||||
self, user: Any, scopes: Optional[str]
|
||||
) -> None:
|
||||
"""Raise if ``scopes`` would grant a user more than their own RBAC.
|
||||
|
||||
Enforces the "intersection, never broader" rule confirmed for this
|
||||
feature: a user must never be able to mint a token scoped beyond
|
||||
what their own role already permits, even if they hand-author the
|
||||
scopes string themselves at issuance time.
|
||||
|
||||
Per-resource scopes (``superset:<resource>:<action>``) are checked
|
||||
against the user's actual ``can_<method>`` RBAC grant for that
|
||||
resource. Flat scopes (``superset:read``/``superset:write``, the
|
||||
pre-per-resource form) can only be self-issued by Admins — a flat
|
||||
scope grants a method across every resource, and there's no single
|
||||
RBAC check that soundly proves a non-Admin has that for "every
|
||||
resource," so it's rejected for anyone else rather than guessed at.
|
||||
Unrecognized scope strings are rejected outright (fail closed).
|
||||
|
||||
NOTE: this only prevents the request from being honored; it does
|
||||
not (yet) produce a clean 400 response, since FAB's ``ApiKeyApi``
|
||||
has no validation hook this can plug into without replacing the API
|
||||
registration entirely. Raising here surfaces as a 500 via FAB's
|
||||
``@safe`` decorator until that's addressed — tracked as a known
|
||||
follow-up, not silently accepted.
|
||||
"""
|
||||
if not scopes:
|
||||
return
|
||||
# pylint: disable-next=import-outside-toplevel
|
||||
from superset.security.api_key_scopes import (
|
||||
RESOURCE_SCOPE_ACTIONS,
|
||||
RESOURCE_SCOPE_CLASS,
|
||||
SCOPE_ACTION_METHOD_PERMISSIONS,
|
||||
)
|
||||
|
||||
admin_role_name = get_conf()["AUTH_ROLE_ADMIN"]
|
||||
is_admin = any(
|
||||
role.name == admin_role_name for role in getattr(user, "roles", [])
|
||||
)
|
||||
for raw_scope in scopes.split(","):
|
||||
scope = raw_scope.strip()
|
||||
if not scope:
|
||||
continue
|
||||
parts = scope.split(":")
|
||||
if len(parts) == 3 and parts[0] == "superset":
|
||||
_, resource_slug, action = parts
|
||||
class_permission_name = RESOURCE_SCOPE_CLASS.get(resource_slug)
|
||||
if class_permission_name is None:
|
||||
raise ValueError(
|
||||
f"Requested scope '{scope}' names an unrecognized "
|
||||
f"resource '{resource_slug}'"
|
||||
)
|
||||
if action not in RESOURCE_SCOPE_ACTIONS:
|
||||
raise ValueError(
|
||||
f"Requested scope '{scope}' names an unrecognized "
|
||||
f"action '{action}'"
|
||||
)
|
||||
if any(
|
||||
self._has_view_access(user, f"can_{method}", class_permission_name)
|
||||
for method in SCOPE_ACTION_METHOD_PERMISSIONS[action]
|
||||
):
|
||||
continue
|
||||
raise ValueError(
|
||||
f"Requested scope '{scope}' exceeds the issuing user's "
|
||||
"own permissions"
|
||||
)
|
||||
if (
|
||||
len(parts) == 2
|
||||
and parts[0] == "superset"
|
||||
and parts[1] in RESOURCE_SCOPE_ACTIONS
|
||||
and is_admin
|
||||
):
|
||||
continue
|
||||
raise ValueError(
|
||||
f"Requested scope '{scope}' is not a recognized "
|
||||
"superset:<resource>:<action> scope, or requires Admin to "
|
||||
"self-issue as a flat scope"
|
||||
)
|
||||
|
||||
def create_api_key(
|
||||
self,
|
||||
user: Any,
|
||||
name: str,
|
||||
scopes: Optional[str] = None,
|
||||
expires_on: Optional[datetime.datetime] = None,
|
||||
) -> Optional[dict[str, Any]]:
|
||||
"""Create a new API key, enforcing the scope-intersection rule.
|
||||
|
||||
Thin wrapper around FAB's ``SecurityManager.create_api_key`` — see
|
||||
``_validate_requested_api_key_scopes`` for the actual check. FAB's
|
||||
base implementation is otherwise unchanged.
|
||||
"""
|
||||
self._validate_requested_api_key_scopes(user, scopes)
|
||||
return super().create_api_key( # type: ignore[misc]
|
||||
user=user, name=name, scopes=scopes, expires_on=expires_on
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def is_guest_user(user: Optional[Any] = None) -> bool:
|
||||
# pylint: disable=import-outside-toplevel
|
||||
|
||||
@@ -41,7 +41,7 @@ from typing import Any, Optional
|
||||
from flask import flash, session
|
||||
from flask_babel import gettext as __
|
||||
from flask_login import current_user, logout_user
|
||||
from sqlalchemy import event, inspect
|
||||
from sqlalchemy import event, inspect, or_
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from werkzeug.wrappers import Response
|
||||
|
||||
@@ -163,9 +163,20 @@ def invalidate_user_sessions(connection: Any, user_id: int) -> None:
|
||||
)
|
||||
|
||||
def _stamp_existing() -> int:
|
||||
# Guard against two concurrent writers regressing the epoch: a
|
||||
# transaction that computed an earlier ``now`` can reach this UPDATE
|
||||
# after one with a later ``now`` has already committed. Only apply
|
||||
# the write when it would advance (or initialize) the stored value,
|
||||
# so the epoch is monotonic regardless of commit order.
|
||||
return connection.execute(
|
||||
table.update()
|
||||
.where(table.c.user_id == user_id)
|
||||
.where(
|
||||
or_(
|
||||
table.c.sessions_invalidated_at.is_(None),
|
||||
table.c.sessions_invalidated_at < now,
|
||||
)
|
||||
)
|
||||
.values(sessions_invalidated_at=now, changed_on=now)
|
||||
).rowcount
|
||||
|
||||
@@ -187,6 +198,23 @@ def invalidate_user_sessions(connection: Any, user_id: int) -> None:
|
||||
_stamp_existing()
|
||||
|
||||
|
||||
def invalidate_sessions_for_user(user_id: int) -> None:
|
||||
"""Stamp the invalidation epoch for ``user_id`` from ordinary application code.
|
||||
|
||||
Convenience wrapper around ``invalidate_user_sessions`` for callers that
|
||||
don't have the raw ``Connection`` the ``after_update`` event listener
|
||||
receives -- e.g. a password-change flow. The stamp is written through the
|
||||
current session's own connection, so it participates in whatever
|
||||
transaction the caller's other pending changes belong to; it is not
|
||||
committed here, so the caller's own commit (or the next flush that
|
||||
triggers one) is what makes it durable.
|
||||
"""
|
||||
# pylint: disable=import-outside-toplevel
|
||||
from superset.extensions import db
|
||||
|
||||
invalidate_user_sessions(db.session.connection(), user_id)
|
||||
|
||||
|
||||
def _stamp_epoch_on_disable(_mapper: Any, connection: Any, target: Any) -> None:
|
||||
history = inspect(target).attrs.active.history
|
||||
# Only act when ``active`` actually changed to False — ignore the
|
||||
|
||||
+21
-1
@@ -209,7 +209,7 @@ class GenericDataType(IntEnum):
|
||||
STRING = 1
|
||||
TEMPORAL = 2
|
||||
BOOLEAN = 3
|
||||
# ARRAY = 4 # Mapping all the complex data types to STRING for now
|
||||
MULTI_VALUE = 4 # array-typed columns (e.g. ClickHouse Array, Postgres ARRAY)
|
||||
# JSON = 5 # and leaving these as a reminder.
|
||||
# MAP = 6
|
||||
# ROW = 7
|
||||
@@ -299,6 +299,17 @@ class FilterOperator(StrEnum):
|
||||
IS_TRUE = "IS TRUE"
|
||||
IS_FALSE = "IS FALSE"
|
||||
TEMPORAL_RANGE = "TEMPORAL_RANGE"
|
||||
# Element-level operators for MULTI_VALUE (array) columns
|
||||
CONTAINS_ANY = "CONTAINS_ANY"
|
||||
CONTAINS_ALL = "CONTAINS_ALL"
|
||||
IS_EMPTY = "IS_EMPTY"
|
||||
IS_NOT_EMPTY = "IS_NOT_EMPTY"
|
||||
# Length (element-count) comparison operators for array columns
|
||||
LENGTH_EQUALS = "LENGTH_EQUALS"
|
||||
LENGTH_GREATER_THAN = "LENGTH_GREATER_THAN"
|
||||
LENGTH_LESS_THAN = "LENGTH_LESS_THAN"
|
||||
LENGTH_GREATER_THAN_OR_EQUALS = "LENGTH_GREATER_THAN_OR_EQUALS"
|
||||
LENGTH_LESS_THAN_OR_EQUALS = "LENGTH_LESS_THAN_OR_EQUALS"
|
||||
|
||||
|
||||
class FilterStringOperators(StrEnum):
|
||||
@@ -317,6 +328,15 @@ class FilterStringOperators(StrEnum):
|
||||
LATEST_PARTITION = ("LATEST_PARTITION",)
|
||||
IS_TRUE = ("IS_TRUE",)
|
||||
IS_FALSE = ("IS_FALSE",)
|
||||
CONTAINS_ANY = ("CONTAINS_ANY",)
|
||||
CONTAINS_ALL = ("CONTAINS_ALL",)
|
||||
IS_EMPTY = ("IS_EMPTY",)
|
||||
IS_NOT_EMPTY = ("IS_NOT_EMPTY",)
|
||||
LENGTH_EQUALS = ("LENGTH_EQUALS",)
|
||||
LENGTH_GREATER_THAN = ("LENGTH_GREATER_THAN",)
|
||||
LENGTH_LESS_THAN = ("LENGTH_LESS_THAN",)
|
||||
LENGTH_GREATER_THAN_OR_EQUALS = ("LENGTH_GREATER_THAN_OR_EQUALS",)
|
||||
LENGTH_LESS_THAN_OR_EQUALS = ("LENGTH_LESS_THAN_OR_EQUALS",)
|
||||
|
||||
|
||||
class PostProcessingBoxplotWhiskerType(StrEnum):
|
||||
|
||||
@@ -27,8 +27,9 @@ class CustomTagsOptimizationMixin:
|
||||
|
||||
When enabled via config, this mixin:
|
||||
1. Configures list_columns to use custom_tags (filtered relationship)
|
||||
2. Rewrites frontend requests from 'tags.*' to 'custom_tags.*'
|
||||
3. Transforms responses to rename 'custom_tags' back to 'tags'
|
||||
2. Exposes custom_tags as tags in the response schema
|
||||
3. Rewrites frontend requests from 'tags.*' to 'custom_tags.*'
|
||||
4. Transforms responses to rename 'custom_tags' back to 'tags'
|
||||
|
||||
This provides SQL query optimization (97% reduction) while maintaining
|
||||
frontend compatibility.
|
||||
@@ -62,6 +63,18 @@ class CustomTagsOptimizationMixin:
|
||||
self._custom_tags_only = current_app.config.get(config_key, False)
|
||||
self.list_columns = custom_columns if self._custom_tags_only else full_columns
|
||||
|
||||
def _init_model_schemas(self) -> None:
|
||||
"""Keep the optimized relationship's public schema name stable."""
|
||||
super()._init_model_schemas() # type: ignore[misc]
|
||||
|
||||
list_model_schema = getattr(self, "list_model_schema", None)
|
||||
if (
|
||||
self._custom_tags_only
|
||||
and list_model_schema
|
||||
and "custom_tags" in list_model_schema.fields
|
||||
):
|
||||
list_model_schema.fields["custom_tags"].data_key = "tags"
|
||||
|
||||
def get_list(self, **kwargs: Any) -> Response:
|
||||
"""Override to rewrite request parameters for custom_tags optimization.
|
||||
|
||||
|
||||
@@ -28,7 +28,11 @@ from superset.common.query_context_factory import QueryContextFactory
|
||||
from superset.common.utils.query_cache_manager import QueryCacheManager
|
||||
from superset.constants import CacheRegion
|
||||
from superset.daos.datasource import DatasourceDAO
|
||||
from superset.utils.core import extract_dataframe_dtypes, QueryStatus
|
||||
from superset.utils.core import (
|
||||
apply_max_row_limit,
|
||||
extract_dataframe_dtypes,
|
||||
QueryStatus,
|
||||
)
|
||||
from superset.views.datasource.schemas import SamplesPayloadSchema
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -45,9 +49,11 @@ def get_limit_clause(page: Optional[int], per_page: Optional[int]) -> dict[str,
|
||||
|
||||
if isinstance(page, int) and isinstance(per_page, int):
|
||||
limit = int(per_page)
|
||||
if limit < 0 or limit > samples_row_limit:
|
||||
if limit < 0:
|
||||
# reset limit value if input is invalid
|
||||
limit = samples_row_limit
|
||||
elif limit:
|
||||
limit = apply_max_row_limit(limit)
|
||||
|
||||
offset = max((int(page) - 1) * limit, 0)
|
||||
|
||||
|
||||
@@ -23,11 +23,12 @@ from flask_appbuilder.security.decorators import protect
|
||||
from flask_appbuilder.security.sqla.models import User
|
||||
from marshmallow import ValidationError
|
||||
from sqlalchemy.orm.exc import NoResultFound
|
||||
from werkzeug.security import generate_password_hash
|
||||
from werkzeug.security import check_password_hash, generate_password_hash
|
||||
|
||||
from superset import is_feature_enabled
|
||||
from superset.daos.user import UserDAO
|
||||
from superset.extensions import db, event_logger
|
||||
from superset.security.session_invalidation import invalidate_sessions_for_user
|
||||
from superset.utils.slack import get_user_avatar, SlackClientError
|
||||
from superset.views.base_api import BaseSupersetApi, requires_json, statsd_metrics
|
||||
from superset.views.users.schemas import CurrentUserPutSchema, UserResponseSchema
|
||||
@@ -49,12 +50,45 @@ class CurrentUserRestApi(BaseSupersetApi):
|
||||
def pre_update(self, item: User, data: Dict[str, Any]) -> None:
|
||||
item.changed_on = datetime.now()
|
||||
item.changed_by_fk = g.user.id
|
||||
# Pop unconditionally: this key is only meaningful for verifying a
|
||||
# password change below, and it isn't a real column on the user
|
||||
# model -- it must never reach ``UserDAO.update``'s ``setattr`` loop.
|
||||
current_password = data.pop("current_password", None)
|
||||
if "password" in data and data["password"]:
|
||||
# An account with no password set yet (e.g. provisioned via an
|
||||
# external auth backend) has nothing to prove knowledge of; for
|
||||
# every other account, the caller must confirm the existing
|
||||
# password before it can be replaced.
|
||||
proof_ok = (
|
||||
item.password
|
||||
and current_password
|
||||
and check_password_hash(item.password, current_password)
|
||||
)
|
||||
if item.password and not proof_ok:
|
||||
raise ValidationError(
|
||||
{"current_password": ["Incorrect current password."]}
|
||||
)
|
||||
# Compute and assign the hash, then drop the plaintext from
|
||||
# ``data`` -- it is passed to ``UserDAO.update`` as ``attributes``
|
||||
# right after this, and ``BaseDAO.update`` sets every key in it
|
||||
# via ``setattr``. Leaving the plaintext in would overwrite the
|
||||
# hash just assigned below with the raw value.
|
||||
new_password = data.pop("password")
|
||||
item.password = generate_password_hash(
|
||||
password=data["password"],
|
||||
password=new_password,
|
||||
method=app.config.get("FAB_PASSWORD_HASH_METHOD", "scrypt"),
|
||||
salt_length=app.config.get("FAB_PASSWORD_HASH_SALT_LENGTH", 16),
|
||||
)
|
||||
# A changed password invalidates any other outstanding session
|
||||
# for this account.
|
||||
invalidate_sessions_for_user(item.id)
|
||||
elif "password" in data:
|
||||
# A falsy value (e.g. an empty string, which the complexity
|
||||
# validator lets through when password complexity is disabled)
|
||||
# skips the block above, but the key must still never reach
|
||||
# ``UserDAO.update``'s ``setattr`` loop -- it would blank out
|
||||
# the account's stored hash.
|
||||
data.pop("password")
|
||||
|
||||
@expose("/", methods=("GET",))
|
||||
@protect()
|
||||
|
||||
@@ -14,17 +14,22 @@
|
||||
# KIND, either express or implied. See the License for the
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
from typing import Any
|
||||
|
||||
from flask_appbuilder.security.sqla.apis.user.schema import User
|
||||
from flask_appbuilder.security.sqla.apis.user.validator import (
|
||||
PasswordComplexityValidator,
|
||||
)
|
||||
from marshmallow import fields, Schema
|
||||
from marshmallow import fields, Schema, validates_schema, ValidationError
|
||||
from marshmallow.fields import Boolean, Integer, String
|
||||
from marshmallow.validate import Length
|
||||
|
||||
first_name_description = "The current user's first name"
|
||||
last_name_description = "The current user's last name"
|
||||
password_description = "The current user's password for authentication" # noqa: S105
|
||||
# Required, and verified against the account's existing password, whenever
|
||||
# ``password`` is included in the payload.
|
||||
current_password_description = "The current user's existing password" # noqa: S105
|
||||
|
||||
|
||||
class UserGroupSchema(Schema):
|
||||
@@ -64,3 +69,24 @@ class CurrentUserPutSchema(Schema):
|
||||
validate=[PasswordComplexityValidator()],
|
||||
metadata={"description": password_description},
|
||||
)
|
||||
current_password = fields.String(
|
||||
required=False,
|
||||
load_only=True,
|
||||
metadata={"description": current_password_description},
|
||||
)
|
||||
|
||||
@validates_schema
|
||||
def validate_current_password_required_with_password(
|
||||
self, data: dict[str, Any], **kwargs: object
|
||||
) -> None:
|
||||
"""Require ``current_password`` whenever ``password`` is being set.
|
||||
|
||||
This only checks that the field was supplied -- whether it actually
|
||||
matches the account's existing password is verified against the
|
||||
database in ``CurrentUserRestApi.pre_update``, which has access to
|
||||
the user record this schema doesn't.
|
||||
"""
|
||||
if data.get("password") and not data.get("current_password"):
|
||||
raise ValidationError(
|
||||
{"current_password": ["This field is required to change the password."]}
|
||||
)
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
# under the License.
|
||||
"""Unit tests for Superset"""
|
||||
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import rison
|
||||
@@ -42,6 +43,57 @@ class TestOpenApiSpec(SupersetTestCase):
|
||||
response = json.loads(rv.data.decode("utf-8"))
|
||||
validate(response)
|
||||
|
||||
def test_dashboard_list_uses_generated_response_schema(self):
|
||||
"""Keep the generated dashboard list contract aligned with published docs.
|
||||
|
||||
If an intentional list schema change breaks this test, regenerate
|
||||
``docs/static/resources/openapi.json`` with ``superset update-api-docs``
|
||||
under the production-default configuration, then review the focused
|
||||
dashboard diff.
|
||||
"""
|
||||
self.login(ADMIN_USERNAME)
|
||||
rv = self.client.get("api/v1/_openapi")
|
||||
|
||||
assert rv.status_code == 200
|
||||
generated_spec = json.loads(rv.data.decode("utf-8"))
|
||||
published_spec_path = (
|
||||
Path(__file__).parents[2] / "docs" / "static" / "resources" / "openapi.json"
|
||||
)
|
||||
published_spec = json.loads(published_spec_path.read_text(encoding="utf-8"))
|
||||
|
||||
generated_result_items = generated_spec["paths"]["/api/v1/dashboard/"]["get"][
|
||||
"responses"
|
||||
]["200"]["content"]["application/json"]["schema"]["properties"]["result"][
|
||||
"items"
|
||||
]
|
||||
published_result_items = published_spec["paths"]["/api/v1/dashboard/"]["get"][
|
||||
"responses"
|
||||
]["200"]["content"]["application/json"]["schema"]["properties"]["result"][
|
||||
"items"
|
||||
]
|
||||
|
||||
assert (
|
||||
generated_result_items
|
||||
== published_result_items
|
||||
== {"$ref": "#/components/schemas/DashboardRestApi.get_list"}
|
||||
)
|
||||
|
||||
schema_prefix = "DashboardRestApi.get_list"
|
||||
generated_schemas = {
|
||||
name: schema
|
||||
for name, schema in generated_spec["components"]["schemas"].items()
|
||||
if name == schema_prefix or name.startswith(f"{schema_prefix}.")
|
||||
}
|
||||
published_schemas = {
|
||||
name: schema
|
||||
for name, schema in published_spec["components"]["schemas"].items()
|
||||
if name == schema_prefix or name.startswith(f"{schema_prefix}.")
|
||||
}
|
||||
|
||||
assert generated_schemas == published_schemas, (
|
||||
"Dashboard list OpenAPI components changed; regenerate the published spec"
|
||||
)
|
||||
|
||||
def test_info_endpoint(self):
|
||||
"""
|
||||
API: Test info endpoint
|
||||
|
||||
@@ -3516,6 +3516,7 @@ class TestDatabaseApi(SupersetTestCase):
|
||||
"description": "Database port",
|
||||
"maximum": 65536,
|
||||
"minimum": 0,
|
||||
"nullable": True,
|
||||
"type": "integer",
|
||||
},
|
||||
"query": {
|
||||
@@ -3533,7 +3534,10 @@ class TestDatabaseApi(SupersetTestCase):
|
||||
"type": "string",
|
||||
},
|
||||
},
|
||||
"required": ["database", "host", "port", "username"],
|
||||
# ``port`` is intentionally not required: a blank port falls
|
||||
# back to the default (5432) in
|
||||
# ``PostgresEngineSpec.build_sqlalchemy_uri``.
|
||||
"required": ["database", "host", "username"],
|
||||
"type": "object",
|
||||
},
|
||||
"preferred": True,
|
||||
@@ -3968,8 +3972,8 @@ class TestDatabaseApi(SupersetTestCase):
|
||||
]
|
||||
}
|
||||
|
||||
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
|
||||
@mock.patch("superset.db_engine_specs.base.is_port_open")
|
||||
@mock.patch("superset.db_engine_specs.postgres.is_hostname_valid")
|
||||
@mock.patch("superset.db_engine_specs.postgres.is_port_open")
|
||||
@mock.patch("superset.databases.api.ValidateDatabaseParametersCommand")
|
||||
def test_validate_parameters_valid_payload(
|
||||
self,
|
||||
@@ -4059,7 +4063,7 @@ class TestDatabaseApi(SupersetTestCase):
|
||||
]
|
||||
}
|
||||
|
||||
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
|
||||
@mock.patch("superset.db_engine_specs.postgres.is_hostname_valid")
|
||||
def test_validate_parameters_invalid_host(self, is_hostname_valid):
|
||||
is_hostname_valid.return_value = False
|
||||
|
||||
@@ -4119,7 +4123,7 @@ class TestDatabaseApi(SupersetTestCase):
|
||||
]
|
||||
}
|
||||
|
||||
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
|
||||
@mock.patch("superset.db_engine_specs.postgres.is_hostname_valid")
|
||||
def test_validate_parameters_invalid_port_range(self, is_hostname_valid):
|
||||
is_hostname_valid.return_value = True
|
||||
|
||||
|
||||
@@ -1063,8 +1063,8 @@ class TestTestConnectionDatabaseCommand(SupersetTestCase):
|
||||
mock_event_logger.assert_called()
|
||||
|
||||
|
||||
@patch("superset.db_engine_specs.base.is_hostname_valid")
|
||||
@patch("superset.db_engine_specs.base.is_port_open")
|
||||
@patch("superset.db_engine_specs.postgres.is_hostname_valid")
|
||||
@patch("superset.db_engine_specs.postgres.is_port_open")
|
||||
@patch("superset.commands.database.validate.DatabaseDAO")
|
||||
def test_validate(
|
||||
mock_database_dao, # noqa: N803
|
||||
@@ -1093,8 +1093,8 @@ def test_validate(
|
||||
command.run()
|
||||
|
||||
|
||||
@patch("superset.db_engine_specs.base.is_hostname_valid")
|
||||
@patch("superset.db_engine_specs.base.is_port_open")
|
||||
@patch("superset.db_engine_specs.postgres.is_hostname_valid")
|
||||
@patch("superset.db_engine_specs.postgres.is_port_open")
|
||||
def test_validate_partial(is_port_open, is_hostname_valid, app_context):
|
||||
"""
|
||||
Test parameter validation when only some parameters are present.
|
||||
@@ -1134,10 +1134,14 @@ def test_validate_partial(is_port_open, is_hostname_valid, app_context):
|
||||
]
|
||||
|
||||
|
||||
@patch("superset.db_engine_specs.base.is_hostname_valid")
|
||||
@patch("superset.db_engine_specs.postgres.is_hostname_valid")
|
||||
def test_validate_partial_invalid_hostname(is_hostname_valid, app_context):
|
||||
"""
|
||||
Test parameter validation when only some parameters are present.
|
||||
|
||||
``port`` is intentionally absent from the payload (and from the expected
|
||||
"missing" list below): it is no longer a required parameter for
|
||||
Postgres, since a blank port falls back to the default (5432).
|
||||
"""
|
||||
is_hostname_valid.return_value = False
|
||||
|
||||
@@ -1157,11 +1161,11 @@ def test_validate_partial_invalid_hostname(is_hostname_valid, app_context):
|
||||
command.run()
|
||||
assert excinfo.value.errors == [
|
||||
SupersetError(
|
||||
message="One or more parameters are missing: database, port, username",
|
||||
message="One or more parameters are missing: database, username",
|
||||
error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR,
|
||||
level=ErrorLevel.WARNING,
|
||||
extra={
|
||||
"missing": ["database", "port", "username"],
|
||||
"missing": ["database", "username"],
|
||||
"issue_codes": [
|
||||
{
|
||||
"code": 1018,
|
||||
|
||||
@@ -154,8 +154,22 @@ class TestDatasourceApi(SupersetTestCase):
|
||||
column_name="col2",
|
||||
limit=10000,
|
||||
denormalize_column=False,
|
||||
array_elements=False,
|
||||
)
|
||||
|
||||
@pytest.mark.usefixtures("app_context", "virtual_dataset")
|
||||
@patch("superset.models.helpers.ExploreMixin.values_for_column")
|
||||
def test_get_column_values_array_elements_param(self, values_for_column_mock):
|
||||
# The ?array_elements=true param (Contains any/all) is threaded through
|
||||
# so array columns can suggest individual elements.
|
||||
self.login(ADMIN_USERNAME)
|
||||
table = self.get_virtual_dataset()
|
||||
self.client.get(
|
||||
f"api/v1/datasource/table/{table.id}/column/col2/values/"
|
||||
"?array_elements=true"
|
||||
)
|
||||
assert values_for_column_mock.call_args.kwargs["array_elements"] is True
|
||||
|
||||
@pytest.mark.usefixtures("app_context", "virtual_dataset")
|
||||
@patch("superset.db_engine_specs.base.BaseEngineSpec.denormalize_name")
|
||||
def test_get_column_values_not_denormalize_column(self, denormalize_name_mock):
|
||||
@@ -176,6 +190,7 @@ class TestDatasourceApi(SupersetTestCase):
|
||||
column_name="col2",
|
||||
limit=10000,
|
||||
denormalize_column=True,
|
||||
array_elements=False,
|
||||
)
|
||||
|
||||
@pytest.mark.usefixtures("app_context", "virtual_dataset")
|
||||
|
||||
@@ -491,6 +491,7 @@ def test_base_parameters_mixin():
|
||||
"minimum": 0,
|
||||
"maximum": 65536,
|
||||
"description": "Database port",
|
||||
"nullable": True,
|
||||
},
|
||||
"password": {"type": "string", "nullable": True, "description": "Password"},
|
||||
"username": {"type": "string", "nullable": True, "description": "Username"},
|
||||
@@ -504,7 +505,9 @@ def test_base_parameters_mixin():
|
||||
"type": "boolean",
|
||||
},
|
||||
},
|
||||
"required": ["database", "host", "port", "username"],
|
||||
# ``port`` is intentionally not required: a blank port falls back to
|
||||
# Postgres's own default (5432) in ``PostgresEngineSpec.build_sqlalchemy_uri``.
|
||||
"required": ["database", "host", "username"],
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -18,7 +18,6 @@
|
||||
# isort:skip_file
|
||||
"""Unit tests for Superset"""
|
||||
|
||||
from datetime import datetime
|
||||
from io import BytesIO
|
||||
from typing import Optional
|
||||
from unittest.mock import Mock, patch
|
||||
@@ -606,7 +605,10 @@ class TestSavedQueryApi(SupersetTestCase):
|
||||
db.session.query(SavedQuery).filter(SavedQuery.label == "label1").all()[0]
|
||||
)
|
||||
self.login(ADMIN_USERNAME)
|
||||
with freeze_time(datetime.now()):
|
||||
# Freeze relative to the persisted timestamp so database-specific
|
||||
# timestamp precision cannot make the humanized value age into the
|
||||
# next bucket while the request is being handled.
|
||||
with freeze_time(saved_query.changed_on):
|
||||
uri = f"api/v1/saved_query/{saved_query.id}"
|
||||
rv = self.get_assert_metric(uri, "get")
|
||||
assert rv.status_code == 200
|
||||
|
||||
@@ -1308,3 +1308,152 @@ def test_column_ordering_without_chart_flag(login_as_admin):
|
||||
finally:
|
||||
db.session.delete(table)
|
||||
db.session.commit()
|
||||
|
||||
|
||||
def _multivalue_table() -> SqlaTable:
|
||||
"""A dataset with an ``Array(String)`` column, for multi-value query tests.
|
||||
|
||||
Built over the example database but never executed — the tests only compile
|
||||
the generated SQL, so the backing table need not physically exist.
|
||||
"""
|
||||
columns = [
|
||||
TableColumn(column_name="skills", type="Array(String)"),
|
||||
TableColumn(column_name="city", type="VARCHAR(255)"),
|
||||
]
|
||||
return SqlaTable(
|
||||
table_name="test_multivalue_jobs",
|
||||
database=get_example_database(),
|
||||
columns=columns,
|
||||
metrics=[SqlMetric(metric_name="count", expression="COUNT(*)")],
|
||||
)
|
||||
|
||||
|
||||
def _multivalue_query(
|
||||
*,
|
||||
filters: list[dict[str, Any]] | None = None,
|
||||
groupby: list[Any] | None = None,
|
||||
) -> dict[str, Any]:
|
||||
return {
|
||||
"granularity": None,
|
||||
"from_dttm": None,
|
||||
"to_dttm": None,
|
||||
"is_timeseries": False,
|
||||
"groupby": groupby if groupby is not None else ["city"],
|
||||
"metrics": ["count"],
|
||||
"filter": filters or [],
|
||||
"extras": {},
|
||||
}
|
||||
|
||||
|
||||
def _compile(table: SqlaTable, query_obj: dict[str, Any]) -> str:
|
||||
from superset.db_engine_specs.clickhouse import ClickHouseEngineSpec
|
||||
|
||||
with patch.object(
|
||||
SqlaTable, "db_engine_spec", property(lambda self: ClickHouseEngineSpec)
|
||||
):
|
||||
sqla_query = table.get_sqla_query(**query_obj)
|
||||
return table.database.compile_sqla_query(sqla_query.sqla_query).lower()
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("app_context")
|
||||
def test_multivalue_contains_any_generates_native_sql():
|
||||
"""CONTAINS_ANY compiles to ``hasAny(col, array(...))``."""
|
||||
table = _multivalue_table()
|
||||
sql = _compile(
|
||||
table,
|
||||
_multivalue_query(
|
||||
filters=[
|
||||
{
|
||||
"col": "skills",
|
||||
"op": FilterOperator.CONTAINS_ANY.value,
|
||||
"val": ["Driver", "Cook"],
|
||||
}
|
||||
]
|
||||
),
|
||||
)
|
||||
assert "hasany(skills" in sql
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("app_context")
|
||||
def test_multivalue_contains_all_generates_native_sql():
|
||||
"""CONTAINS_ALL compiles to ``hasAll(col, array(...))``."""
|
||||
table = _multivalue_table()
|
||||
sql = _compile(
|
||||
table,
|
||||
_multivalue_query(
|
||||
filters=[
|
||||
{
|
||||
"col": "skills",
|
||||
"op": FilterOperator.CONTAINS_ALL.value,
|
||||
"val": ["Driver", "Cook"],
|
||||
}
|
||||
]
|
||||
),
|
||||
)
|
||||
assert "hasall(skills" in sql
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("app_context")
|
||||
def test_multivalue_is_empty_generates_native_sql():
|
||||
"""IS_EMPTY compiles to ``length(col) = 0``."""
|
||||
table = _multivalue_table()
|
||||
sql = _compile(
|
||||
table,
|
||||
_multivalue_query(
|
||||
filters=[{"col": "skills", "op": FilterOperator.IS_EMPTY.value}]
|
||||
),
|
||||
)
|
||||
assert "length(skills) = 0" in sql
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("app_context")
|
||||
def test_multivalue_length_filter_generates_native_sql():
|
||||
"""A LENGTH_GREATER_THAN filter compiles to ``length(col) > N``."""
|
||||
table = _multivalue_table()
|
||||
sql = _compile(
|
||||
table,
|
||||
_multivalue_query(
|
||||
filters=[
|
||||
{
|
||||
"col": "skills",
|
||||
"op": FilterOperator.LENGTH_GREATER_THAN.value,
|
||||
"val": 2,
|
||||
}
|
||||
]
|
||||
),
|
||||
)
|
||||
assert "length(skills) > 2" in sql
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("app_context")
|
||||
def test_multivalue_contains_unsupported_engine_raises():
|
||||
"""CONTAINS_ANY on an engine without array support is rejected."""
|
||||
table = _multivalue_table()
|
||||
query_obj = _multivalue_query(
|
||||
filters=[
|
||||
{
|
||||
"col": "skills",
|
||||
"op": FilterOperator.CONTAINS_ANY.value,
|
||||
"val": ["Driver"],
|
||||
}
|
||||
]
|
||||
)
|
||||
with pytest.raises(QueryObjectValidationError):
|
||||
table.get_sqla_query(**query_obj)
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("app_context")
|
||||
def test_multivalue_length_filter_unsupported_engine_raises():
|
||||
"""A Length filter on an engine without array support is rejected."""
|
||||
table = _multivalue_table()
|
||||
query_obj = _multivalue_query(
|
||||
filters=[
|
||||
{
|
||||
"col": "skills",
|
||||
"op": FilterOperator.LENGTH_GREATER_THAN.value,
|
||||
"val": 2,
|
||||
}
|
||||
]
|
||||
)
|
||||
with pytest.raises(QueryObjectValidationError):
|
||||
table.get_sqla_query(**query_obj)
|
||||
|
||||
@@ -1490,3 +1490,21 @@ def test_get_public_information_exposes_ansi_identifier_quote() -> None:
|
||||
"end": '"',
|
||||
"escape_by_doubling": True,
|
||||
}
|
||||
|
||||
|
||||
def test_multivalue_columns_disabled_by_default() -> None:
|
||||
"""Engines must opt in to multi-value support; base defaults to off."""
|
||||
assert BaseEngineSpec.supports_multivalue_columns is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"method", ["array_contains_any", "array_contains_all", "array_length"]
|
||||
)
|
||||
def test_array_capabilities_raise_when_unsupported(method: str) -> None:
|
||||
"""Array capability methods raise NotImplementedError unless overridden."""
|
||||
from sqlalchemy import column
|
||||
|
||||
fn = getattr(BaseEngineSpec, method)
|
||||
args = (column("c"), ["v"]) if "contains" in method else (column("c"),)
|
||||
with pytest.raises(NotImplementedError):
|
||||
fn(*args)
|
||||
|
||||
@@ -62,6 +62,20 @@ def test_convert_dttm(
|
||||
assert_convert_dttm(spec, target_type, expected_result, dttm)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"time_grain,expected",
|
||||
[
|
||||
(None, "{col}"),
|
||||
("PT1S", "toStartOfSecond(toDateTime64({col}, 3))"),
|
||||
("PT1M", "toStartOfMinute(toDateTime({col}))"),
|
||||
],
|
||||
)
|
||||
def test_time_grain_expressions(time_grain: Optional[str], expected: str) -> None:
|
||||
from superset.db_engine_specs.clickhouse import ClickHouseBaseEngineSpec
|
||||
|
||||
assert ClickHouseBaseEngineSpec._time_grain_expressions[time_grain] == expected
|
||||
|
||||
|
||||
def test_convert_dttm_normalizes_aware_datetime_to_utc() -> None:
|
||||
from superset.db_engine_specs.clickhouse import (
|
||||
ClickHouseEngineSpec as spec, # noqa: N813
|
||||
@@ -129,7 +143,30 @@ def test_connect_convert_dttm(
|
||||
GenericDataType.STRING,
|
||||
False,
|
||||
),
|
||||
("Array(UInt8)", String, None, GenericDataType.STRING, False),
|
||||
("Array(UInt8)", String, None, GenericDataType.MULTI_VALUE, False),
|
||||
("Array(String)", String, None, GenericDataType.MULTI_VALUE, False),
|
||||
("Array(UInt64)", String, None, GenericDataType.MULTI_VALUE, False),
|
||||
(
|
||||
"Array(LowCardinality(String))",
|
||||
String,
|
||||
None,
|
||||
GenericDataType.MULTI_VALUE,
|
||||
False,
|
||||
),
|
||||
# Array(Enum(...)) is a real array and must classify as MULTI_VALUE, not
|
||||
# get short-circuited by the Enum rule (the anchored ^Array\( pattern is
|
||||
# ordered before the Enum entry).
|
||||
(
|
||||
"Array(Enum8('a' = 1, 'b' = 2))",
|
||||
String,
|
||||
None,
|
||||
GenericDataType.MULTI_VALUE,
|
||||
False,
|
||||
),
|
||||
# Arrays nested inside Map/Tuple are not top-level array columns; the
|
||||
# anchored pattern must not over-match them into MULTI_VALUE.
|
||||
("Map(String, Array(String))", String, None, GenericDataType.STRING, False),
|
||||
("Tuple(Array(String))", String, None, GenericDataType.STRING, False),
|
||||
("Enum('hello', 'world')", String, None, GenericDataType.STRING, False),
|
||||
("Enum('UInt32', 'Bool')", String, None, GenericDataType.STRING, False),
|
||||
(
|
||||
@@ -616,3 +653,115 @@ def test_use_equality_for_boolean_filters_property() -> None:
|
||||
from superset.db_engine_specs.clickhouse import ClickHouseBaseEngineSpec
|
||||
|
||||
assert ClickHouseBaseEngineSpec.use_equality_for_boolean_filters is True
|
||||
|
||||
|
||||
def _compile(expr) -> str:
|
||||
return str(expr.compile(compile_kwargs={"literal_binds": True}))
|
||||
|
||||
|
||||
def test_clickhouse_supports_multivalue_columns() -> None:
|
||||
from superset.db_engine_specs.clickhouse import ( # noqa: N813
|
||||
ClickHouseEngineSpec as spec,
|
||||
)
|
||||
|
||||
assert spec.supports_multivalue_columns is True
|
||||
|
||||
|
||||
def test_multivalue_contains_any_sql() -> None:
|
||||
from sqlalchemy import column
|
||||
|
||||
from superset.db_engine_specs.clickhouse import ( # noqa: N813
|
||||
ClickHouseEngineSpec as spec,
|
||||
)
|
||||
|
||||
expr = spec.array_contains_any(column("skills"), ["Driver", "Cook"])
|
||||
assert _compile(expr) == "hasAny(skills, array('Driver', 'Cook'))"
|
||||
|
||||
|
||||
def test_multivalue_contains_all_sql() -> None:
|
||||
from sqlalchemy import column
|
||||
|
||||
from superset.db_engine_specs.clickhouse import ( # noqa: N813
|
||||
ClickHouseEngineSpec as spec,
|
||||
)
|
||||
|
||||
expr = spec.array_contains_all(column("skills"), ["Driver", "Cook"])
|
||||
assert _compile(expr) == "hasAll(skills, array('Driver', 'Cook'))"
|
||||
|
||||
|
||||
def test_multivalue_contains_binds_parameters() -> None:
|
||||
"""Values must be bound parameters, not inlined (SQL-injection safety)."""
|
||||
from sqlalchemy import column
|
||||
|
||||
from superset.db_engine_specs.clickhouse import ( # noqa: N813
|
||||
ClickHouseEngineSpec as spec,
|
||||
)
|
||||
|
||||
expr = spec.array_contains_any(column("skills"), ["Driver"])
|
||||
compiled = expr.compile()
|
||||
assert "Driver" not in str(compiled)
|
||||
assert "Driver" in compiled.params.values()
|
||||
|
||||
|
||||
def test_multivalue_length_sql() -> None:
|
||||
from sqlalchemy import column
|
||||
|
||||
from superset.db_engine_specs.clickhouse import ( # noqa: N813
|
||||
ClickHouseEngineSpec as spec,
|
||||
)
|
||||
|
||||
expr = spec.array_length(column("skills"))
|
||||
assert _compile(expr) == "length(skills)"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"native_type,expected",
|
||||
[
|
||||
("Array(String)", GenericDataType.STRING),
|
||||
("Array(Int32)", GenericDataType.NUMERIC),
|
||||
("Array(UInt64)", GenericDataType.NUMERIC),
|
||||
("Array(Decimal(10, 2))", GenericDataType.NUMERIC),
|
||||
("Array(DateTime)", GenericDataType.TEMPORAL),
|
||||
("Array(Enum8('a' = 1))", GenericDataType.STRING),
|
||||
# Wrappers around the element type don't change the generic type.
|
||||
("Array(Nullable(Int64))", GenericDataType.NUMERIC),
|
||||
("Array(LowCardinality(String))", GenericDataType.STRING),
|
||||
# Non-array / nested-array types have no array element type.
|
||||
("String", None),
|
||||
("Map(String, Array(String))", None),
|
||||
],
|
||||
)
|
||||
def test_multivalue_get_array_element_type(
|
||||
native_type: str, expected: GenericDataType | None
|
||||
) -> None:
|
||||
from superset.db_engine_specs.clickhouse import ( # noqa: N813
|
||||
ClickHouseEngineSpec as spec,
|
||||
)
|
||||
|
||||
assert spec.get_array_element_type(native_type) == expected
|
||||
|
||||
|
||||
def test_multivalue_array_explode_sql() -> None:
|
||||
"""array_explode compiles to ``arrayJoin(col)`` (element expansion)."""
|
||||
from sqlalchemy import column
|
||||
|
||||
from superset.db_engine_specs.clickhouse import ( # noqa: N813
|
||||
ClickHouseEngineSpec as spec,
|
||||
)
|
||||
|
||||
expr = spec.array_explode(column("scores"))
|
||||
assert _compile(expr) == "arrayJoin(scores)"
|
||||
|
||||
|
||||
def test_multivalue_contains_any_numeric_coercion_sql() -> None:
|
||||
"""Numeric-array element values must render as numbers, not quoted strings."""
|
||||
from sqlalchemy import column
|
||||
|
||||
from superset.db_engine_specs.clickhouse import ( # noqa: N813
|
||||
ClickHouseEngineSpec as spec,
|
||||
)
|
||||
|
||||
# Simulate values already coerced to numbers (as helpers.py does via the
|
||||
# element type) and confirm the emitted array literal is numeric.
|
||||
expr = spec.array_contains_any(column("scores"), [5, 6])
|
||||
assert _compile(expr) == "hasAny(scores, array(5, 6))"
|
||||
|
||||
@@ -31,6 +31,7 @@ from superset.db_engine_specs.postgres import (
|
||||
_check_not_redshift,
|
||||
PostgresEngineSpec as spec, # noqa: N813
|
||||
)
|
||||
from superset.errors import SupersetErrorType
|
||||
from superset.exceptions import SupersetSecurityException
|
||||
from superset.sql.parse import Table
|
||||
from superset.utils.core import GenericDataType
|
||||
@@ -501,3 +502,238 @@ def test_get_schema_names_excludes_only_actual_system_schemas(
|
||||
"pgstats",
|
||||
"information_schema",
|
||||
}
|
||||
|
||||
|
||||
def _basic_parameters(**overrides: Any) -> dict[str, Any]:
|
||||
parameters: dict[str, Any] = {
|
||||
"username": "user",
|
||||
"password": "pwd",
|
||||
"host": "localhost",
|
||||
"port": 5432,
|
||||
"database": "db",
|
||||
"query": {},
|
||||
}
|
||||
parameters.update(overrides)
|
||||
return parameters
|
||||
|
||||
|
||||
def test_build_sqlalchemy_uri_defaults_missing_port_to_5432() -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): ``build_sqlalchemy_uri`` defaults a missing
|
||||
``port`` key to the class's own declared default (5432) instead of
|
||||
raising a ``KeyError``, so the dynamic form can connect without a port.
|
||||
"""
|
||||
parameters = _basic_parameters()
|
||||
del parameters["port"]
|
||||
|
||||
uri = spec.build_sqlalchemy_uri(parameters) # type: ignore[arg-type]
|
||||
|
||||
assert make_url(uri).port == 5432
|
||||
assert spec.metadata["default_port"] == 5432
|
||||
|
||||
|
||||
def test_build_sqlalchemy_uri_defaults_blank_port_to_5432() -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): ``build_sqlalchemy_uri`` defaults a blank
|
||||
(``None``) ``port`` value to 5432 rather than emitting ``port=None``.
|
||||
"""
|
||||
parameters = _basic_parameters(port=None)
|
||||
|
||||
uri = spec.build_sqlalchemy_uri(parameters) # type: ignore[arg-type]
|
||||
|
||||
assert make_url(uri).port == 5432
|
||||
|
||||
|
||||
def test_build_sqlalchemy_uri_respects_explicit_port() -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): an explicitly provided port is still honored
|
||||
and not overridden by the default.
|
||||
"""
|
||||
parameters = _basic_parameters(port=5433)
|
||||
|
||||
uri = spec.build_sqlalchemy_uri(parameters) # type: ignore[arg-type]
|
||||
|
||||
assert make_url(uri).port == 5433
|
||||
|
||||
|
||||
def test_build_sqlalchemy_uri_preserves_explicit_port_zero() -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): an explicitly supplied port of ``0`` (a value
|
||||
the schema's ``Range(min=0, ...)`` validator accepts) must not be
|
||||
silently overwritten by the default port. A truthiness check like
|
||||
``port or default`` would incorrectly replace ``0`` with 5432.
|
||||
"""
|
||||
parameters = _basic_parameters(port=0)
|
||||
|
||||
uri = spec.build_sqlalchemy_uri(parameters) # type: ignore[arg-type]
|
||||
|
||||
assert make_url(uri).port == 0
|
||||
|
||||
|
||||
def test_build_sqlalchemy_uri_defaults_empty_string_port_to_5432() -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): ``build_sqlalchemy_uri`` may be called directly
|
||||
with raw, non-schema-loaded parameters (see
|
||||
``ValidateDatabaseParametersCommand``), where a cleared number input
|
||||
submits ``""`` rather than ``null``. That must default to 5432 rather
|
||||
than raising when SQLAlchemy tries to parse ``""`` as a port.
|
||||
"""
|
||||
parameters = _basic_parameters(port="")
|
||||
|
||||
uri = spec.build_sqlalchemy_uri(parameters) # type: ignore[arg-type]
|
||||
|
||||
assert make_url(uri).port == 5432
|
||||
|
||||
|
||||
def test_parameters_schema_blank_port_string_loads_as_none() -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): the Connect Database form's Port field is a
|
||||
number input; clearing it submits ``""`` (HTML input values are always
|
||||
strings), not ``null``. The schema must normalize that to ``None``
|
||||
instead of rejecting it with "Not a valid integer.", so the dynamic
|
||||
form's CONNECT flow (which loads through ``parameters_schema`` before
|
||||
calling ``build_sqlalchemy_uri``) succeeds with a blank port.
|
||||
"""
|
||||
loaded = spec.parameters_schema.load(_basic_parameters(port=""))
|
||||
|
||||
assert loaded["port"] is None
|
||||
|
||||
|
||||
def test_validate_parameters_blank_port_is_not_a_missing_parameter(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): a blank/missing ``port`` must not trigger
|
||||
``CONNECTION_MISSING_PARAMETERS_ERROR``, since ``build_sqlalchemy_uri``
|
||||
falls back to the default Postgres port.
|
||||
"""
|
||||
mocker.patch(
|
||||
"superset.db_engine_specs.postgres.is_hostname_valid", return_value=True
|
||||
)
|
||||
|
||||
properties = {"parameters": _basic_parameters(port=None)}
|
||||
errors = spec.validate_parameters(properties) # type: ignore[arg-type]
|
||||
|
||||
for error in errors:
|
||||
assert "port" not in (error.extra or {}).get("missing", [])
|
||||
assert error.error_type != SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR
|
||||
|
||||
|
||||
def test_validate_parameters_missing_host_still_errors(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): omitting ``host`` still reports it as missing;
|
||||
only ``port`` was made optional.
|
||||
"""
|
||||
properties = {"parameters": _basic_parameters(host="", port=None)}
|
||||
errors = spec.validate_parameters(properties) # type: ignore[arg-type]
|
||||
|
||||
assert len(errors) == 1
|
||||
assert errors[0].error_type == SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR
|
||||
assert (errors[0].extra or {})["missing"] == ["host"]
|
||||
|
||||
|
||||
def test_validate_parameters_missing_other_required_field_still_errors(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): omitting a still-required field (``database``)
|
||||
continues to be reported, even though ``port`` is blank too.
|
||||
"""
|
||||
mocker.patch(
|
||||
"superset.db_engine_specs.postgres.is_hostname_valid", return_value=True
|
||||
)
|
||||
|
||||
properties = {"parameters": _basic_parameters(database="", port=None)}
|
||||
errors = spec.validate_parameters(properties) # type: ignore[arg-type]
|
||||
|
||||
missing_errors = [
|
||||
error
|
||||
for error in errors
|
||||
if error.error_type == SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR
|
||||
]
|
||||
assert len(missing_errors) == 1
|
||||
assert (missing_errors[0].extra or {})["missing"] == ["database"]
|
||||
|
||||
|
||||
def test_validate_parameters_explicit_valid_port_checks_open(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): when a port IS supplied, format/range/open
|
||||
validation is preserved unchanged.
|
||||
"""
|
||||
mocker.patch(
|
||||
"superset.db_engine_specs.postgres.is_hostname_valid", return_value=True
|
||||
)
|
||||
is_port_open = mocker.patch(
|
||||
"superset.db_engine_specs.postgres.is_port_open", return_value=True
|
||||
)
|
||||
|
||||
properties = {"parameters": _basic_parameters(port=5432)}
|
||||
errors = spec.validate_parameters(properties) # type: ignore[arg-type]
|
||||
|
||||
assert errors == []
|
||||
is_port_open.assert_called_once_with("localhost", 5432)
|
||||
|
||||
|
||||
def test_validate_parameters_invalid_port_still_errors(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): an out-of-range port supplied by the user
|
||||
still produces ``CONNECTION_INVALID_PORT_ERROR``, exactly as before.
|
||||
"""
|
||||
mocker.patch(
|
||||
"superset.db_engine_specs.postgres.is_hostname_valid", return_value=True
|
||||
)
|
||||
|
||||
properties = {"parameters": _basic_parameters(port=70000)}
|
||||
errors = spec.validate_parameters(properties) # type: ignore[arg-type]
|
||||
|
||||
assert len(errors) == 1
|
||||
assert errors[0].error_type == SupersetErrorType.CONNECTION_INVALID_PORT_ERROR
|
||||
|
||||
|
||||
def test_validate_parameters_non_integer_port_matches_base_parity(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): a non-integer port must produce BOTH errors
|
||||
that ``BasicParametersMixin.validate_parameters`` produces -- the
|
||||
"Port must be a valid integer." error from the failed ``int()``
|
||||
conversion, AND the "must be an integer between 0 and 65535" range
|
||||
error, since the base method does not return early after the former
|
||||
and falls through to the range check (which is also False for a
|
||||
non-int value). A Postgres-specific override that stops after the
|
||||
first error would silently narrow this API's error-reporting contract.
|
||||
"""
|
||||
mocker.patch(
|
||||
"superset.db_engine_specs.postgres.is_hostname_valid", return_value=True
|
||||
)
|
||||
|
||||
properties = {"parameters": _basic_parameters(port="not-a-port")}
|
||||
errors = spec.validate_parameters(properties) # type: ignore[arg-type]
|
||||
|
||||
assert len(errors) == 2
|
||||
assert errors[0].message == "Port must be a valid integer."
|
||||
assert errors[0].error_type == SupersetErrorType.CONNECTION_INVALID_PORT_ERROR
|
||||
assert (
|
||||
errors[1].message
|
||||
== "The port must be an integer between 0 and 65535 (inclusive)."
|
||||
)
|
||||
assert errors[1].error_type == SupersetErrorType.CONNECTION_INVALID_PORT_ERROR
|
||||
|
||||
|
||||
def test_parameters_schema_port_is_not_required() -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): the JSON schema exposed to the frontend for the
|
||||
Connect Database dynamic form must not mark ``port`` as required, so the
|
||||
modal doesn't block client-side submission when the field is left blank.
|
||||
"""
|
||||
json_schema = spec.parameters_json_schema()
|
||||
|
||||
assert "port" not in json_schema.get("required", [])
|
||||
assert "host" in json_schema.get("required", [])
|
||||
assert "database" in json_schema.get("required", [])
|
||||
|
||||
@@ -1981,6 +1981,23 @@ def test_create_virtual_dataset_request_optional_fields() -> None:
|
||||
assert req.description == "A virtual dataset"
|
||||
|
||||
|
||||
def test_create_virtual_dataset_rejects_non_aggregate_saved_metric() -> None:
|
||||
from pydantic import ValidationError
|
||||
|
||||
with pytest.raises(ValidationError, match="saved metrics must aggregate rows"):
|
||||
CreateVirtualDatasetRequest(
|
||||
database_id=1,
|
||||
sql="SELECT needed_operators FROM staffing",
|
||||
dataset_name="Staffing",
|
||||
metrics=[
|
||||
{
|
||||
"metric_name": "needed_operators",
|
||||
"expression": "needed_operators",
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
# --- Tool logic tests ---
|
||||
|
||||
|
||||
@@ -2119,6 +2136,39 @@ async def test_create_virtual_dataset_create_failed(mcp_server: object) -> None:
|
||||
assert "Failed to create dataset" in data["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_virtual_dataset_sql_error_is_actionable(
|
||||
mcp_server: object,
|
||||
) -> None:
|
||||
"""Warehouse SQL errors are recoverable tool results, not adapter crashes."""
|
||||
from superset.exceptions import SupersetGenericDBErrorException
|
||||
|
||||
mock_command = MagicMock()
|
||||
mock_command.run.side_effect = SupersetGenericDBErrorException(
|
||||
"Invalid column name 'missing_value'"
|
||||
)
|
||||
|
||||
with patch(
|
||||
"superset.commands.dataset.create.CreateDatasetCommand",
|
||||
return_value=mock_command,
|
||||
):
|
||||
async with Client(mcp_server) as client:
|
||||
request = CreateVirtualDatasetRequest(
|
||||
database_id=1,
|
||||
sql="SELECT missing_value FROM sample_events",
|
||||
dataset_name="Test",
|
||||
)
|
||||
result = await client.call_tool(
|
||||
"create_virtual_dataset", {"request": request.model_dump()}
|
||||
)
|
||||
data = json.loads(result.content[0].text)
|
||||
|
||||
assert data["id"] is None
|
||||
assert data["columns"] == []
|
||||
assert data["error"] is not None
|
||||
assert "Invalid column name" in data["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_virtual_dataset_permission_denied(mcp_server: object) -> None:
|
||||
"""SQL access denied surfaces as DatasetInvalidError with id=None."""
|
||||
@@ -2289,7 +2339,13 @@ async def test_create_virtual_dataset_update_failure_rollback(
|
||||
if exception_to_raise == "DatasetUpdateFailedError":
|
||||
mock_update_instance.run.side_effect = DatasetUpdateFailedError()
|
||||
else:
|
||||
mock_update_instance.run.side_effect = DatasetInvalidError()
|
||||
from superset.commands.dataset.exceptions import (
|
||||
DatasetColumnsExistsValidationError,
|
||||
)
|
||||
|
||||
invalid_error = DatasetInvalidError()
|
||||
invalid_error.append(DatasetColumnsExistsValidationError())
|
||||
mock_update_instance.run.side_effect = invalid_error
|
||||
mock_update_cls = MagicMock(return_value=mock_update_instance)
|
||||
|
||||
mock_delete_instance = MagicMock()
|
||||
@@ -2336,7 +2392,11 @@ async def test_create_virtual_dataset_update_failure_rollback(
|
||||
# Verify the error response
|
||||
data = json.loads(result.content[0].text)
|
||||
assert data["id"] is None
|
||||
assert "creation rolled back" in data["error"]
|
||||
if exception_to_raise == "DatasetInvalidError":
|
||||
assert "columns" in data["error"]
|
||||
assert "already exist" in data["error"]
|
||||
else:
|
||||
assert "creation rolled back" in data["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -66,7 +66,7 @@ def mock_auth():
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def allow_data_model_metadata():
|
||||
def allow_data_model_metadata(): # noqa: PT004
|
||||
"""Keep the standalone get_schema suite in the unrestricted default path."""
|
||||
with patch.object(
|
||||
get_schema_module,
|
||||
@@ -606,3 +606,40 @@ class TestGetSchemaPermissionMap:
|
||||
factories = set(get_schema_module._SCHEMA_CORE_FACTORIES.keys())
|
||||
perms = set(get_schema_module._MODEL_TYPE_CLASS_PERMISSION.keys())
|
||||
assert factories == perms
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resource_scope_is_enforced(self, app, mcp_server):
|
||||
"""RBAC access alone cannot bypass a scoped token's resource limit."""
|
||||
with (
|
||||
patch.dict(app.config, {"MCP_RBAC_ENABLED": True}),
|
||||
patch("superset.security_manager.can_access", return_value=True),
|
||||
patch.object(
|
||||
get_schema_module, "_token_scope_allows", return_value=False
|
||||
) as scope_allows,
|
||||
):
|
||||
async with Client(mcp_server) as client:
|
||||
with pytest.raises(ToolError, match="Permission denied"):
|
||||
await client.call_tool(
|
||||
"get_schema", {"request": {"model_type": "chart"}}
|
||||
)
|
||||
|
||||
scope_allows.assert_called_once_with("read", "Chart")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resource_scope_is_enforced_when_rbac_disabled(self, app, mcp_server):
|
||||
"""The RBAC feature flag does not disable credential scopes."""
|
||||
with (
|
||||
patch.dict(app.config, {"MCP_RBAC_ENABLED": False}),
|
||||
patch("superset.security_manager.can_access") as can_access,
|
||||
patch.object(
|
||||
get_schema_module, "_token_scope_allows", return_value=False
|
||||
) as scope_allows,
|
||||
):
|
||||
async with Client(mcp_server) as client:
|
||||
with pytest.raises(ToolError, match="Permission denied"):
|
||||
await client.call_tool(
|
||||
"get_schema", {"request": {"model_type": "chart"}}
|
||||
)
|
||||
|
||||
can_access.assert_not_called()
|
||||
scope_allows.assert_called_once_with("read", "Chart")
|
||||
|
||||
@@ -23,12 +23,14 @@ import pytest
|
||||
from flask import g
|
||||
|
||||
from superset.mcp_service.auth import (
|
||||
_required_resource_scope,
|
||||
check_tool_permission,
|
||||
CLASS_PERMISSION_ATTR,
|
||||
is_tool_visible_to_current_user,
|
||||
MCPPermissionDeniedError,
|
||||
METHOD_PERMISSION_ATTR,
|
||||
PERMISSION_PREFIX,
|
||||
RESOURCE_SCOPE_NAME,
|
||||
)
|
||||
|
||||
|
||||
@@ -108,6 +110,17 @@ def test_check_tool_permission_no_class_permission_allows(app_context) -> None:
|
||||
assert check_tool_permission(func) is True
|
||||
|
||||
|
||||
def test_scoped_token_constrains_permissionless_tool(app_context) -> None:
|
||||
"""Resource-only scopes do not grant permission-less tools."""
|
||||
g.user = MagicMock(username="admin")
|
||||
func = _make_tool_func()
|
||||
|
||||
with _patch_token_scopes(["superset:dashboard:read"]):
|
||||
assert check_tool_permission(func) is False
|
||||
with _patch_token_scopes(["superset:read"]):
|
||||
assert check_tool_permission(func) is True
|
||||
|
||||
|
||||
def test_check_tool_permission_no_user_denies(app_context) -> None:
|
||||
"""If no g.user, permission check should deny."""
|
||||
g.user = None
|
||||
@@ -170,6 +183,19 @@ def test_check_tool_permission_disabled_via_config(app_context, app) -> None:
|
||||
app.config["MCP_RBAC_ENABLED"] = True
|
||||
|
||||
|
||||
def test_disabled_rbac_still_enforces_token_scopes(app_context, app) -> None:
|
||||
"""Disabling user RBAC does not disable credential restrictions."""
|
||||
func = _make_tool_func(class_perm="Chart", method_perm="write")
|
||||
app.config["MCP_RBAC_ENABLED"] = False
|
||||
try:
|
||||
with _patch_token_scopes(["superset:dashboard:read"]):
|
||||
assert check_tool_permission(func) is False
|
||||
with _patch_token_scopes(["superset:chart:write"]):
|
||||
assert check_tool_permission(func) is True
|
||||
finally:
|
||||
app.config["MCP_RBAC_ENABLED"] = True
|
||||
|
||||
|
||||
# -- Permission constants --
|
||||
|
||||
|
||||
@@ -289,6 +315,19 @@ def test_visibility_public_tool_no_class_permission(app_context) -> None:
|
||||
assert is_tool_visible_to_current_user(tool) is True
|
||||
|
||||
|
||||
def test_visibility_hides_permissionless_tool_from_resource_scoped_token(
|
||||
app_context,
|
||||
) -> None:
|
||||
"""Permission-less tools require a flat scope in tools/list too."""
|
||||
g.user = MagicMock(username="viewer")
|
||||
tool = _make_mock_tool(fn=_make_tool_func())
|
||||
|
||||
with _patch_token_scopes(["superset:dashboard:read"]):
|
||||
assert is_tool_visible_to_current_user(tool) is False
|
||||
with _patch_token_scopes(["superset:read"]):
|
||||
assert is_tool_visible_to_current_user(tool) is True
|
||||
|
||||
|
||||
def test_visibility_allowed_tool(app_context) -> None:
|
||||
"""Tools where security_manager grants access are visible."""
|
||||
g.user = MagicMock(username="admin")
|
||||
@@ -431,6 +470,23 @@ def test_scope_falls_back_to_rbac_when_no_jwt_context(app_context) -> None:
|
||||
assert result is True
|
||||
|
||||
|
||||
def test_scope_context_error_fails_closed(app_context) -> None:
|
||||
"""An unexpected token lookup failure cannot erase token restrictions."""
|
||||
g.user = MagicMock(username="editor")
|
||||
func = _make_tool_func(class_perm="Chart", method_perm="read")
|
||||
|
||||
mock_sm = MagicMock()
|
||||
mock_sm.can_access = MagicMock(return_value=True)
|
||||
with (
|
||||
patch("superset.mcp_service.auth.security_manager", mock_sm),
|
||||
patch(
|
||||
"fastmcp.server.dependencies.get_access_token",
|
||||
side_effect=TypeError("invalid token context"),
|
||||
),
|
||||
):
|
||||
assert check_tool_permission(func) is False
|
||||
|
||||
|
||||
def test_scope_read_denied_when_token_lacks_read_scope(app_context) -> None:
|
||||
"""A read tool is denied when the token only carries an unrelated scope."""
|
||||
g.user = MagicMock(username="viewer")
|
||||
@@ -447,7 +503,9 @@ def test_scope_read_denied_when_token_lacks_read_scope(app_context) -> None:
|
||||
assert result is False
|
||||
|
||||
|
||||
def test_scope_denies_unmapped_method_for_scoped_token(app_context) -> None:
|
||||
def test_scope_denies_unmapped_method_for_scoped_token(
|
||||
app_context, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A scoped token presented for a method permission that is NOT in the
|
||||
scope map fails closed (denied), even when RBAC grants, so an unmapped
|
||||
custom permission cannot silently bypass scope enforcement."""
|
||||
@@ -463,6 +521,8 @@ def test_scope_denies_unmapped_method_for_scoped_token(app_context) -> None:
|
||||
result = check_tool_permission(func)
|
||||
|
||||
assert result is False
|
||||
assert "unmapped method permission 'some_custom_perm'" in caplog.text
|
||||
assert "required scope 'None'" not in caplog.text
|
||||
|
||||
|
||||
def test_scope_execute_sql_query_requires_write_scope(app_context) -> None:
|
||||
@@ -480,6 +540,103 @@ def test_scope_execute_sql_query_requires_write_scope(app_context) -> None:
|
||||
assert check_tool_permission(func) is True
|
||||
|
||||
|
||||
# -- Per-resource scopes (superset:<resource>:<action>) --
|
||||
|
||||
|
||||
def test_required_resource_scope_special_names() -> None:
|
||||
"""The explicit resource map handles names a naive lower() would break:
|
||||
'Row Level Security' (spaces) and 'ReportSchedule'/'SQLLab' (misnames)."""
|
||||
assert _required_resource_scope("Row Level Security", "read") == "superset:rls:read"
|
||||
assert _required_resource_scope("ReportSchedule", "write") == (
|
||||
"superset:report:write"
|
||||
)
|
||||
assert _required_resource_scope("SQLLab", "execute_sql_query") == (
|
||||
"superset:sqllab:write"
|
||||
)
|
||||
assert _required_resource_scope("Chart", "update") == "superset:chart:write"
|
||||
|
||||
|
||||
def test_required_resource_scope_unmapped_returns_none() -> None:
|
||||
"""An unmapped resource or method yields None (no per-resource scope),
|
||||
which callers must NOT treat as a grant."""
|
||||
assert _required_resource_scope("NotAResource", "read") is None
|
||||
assert _required_resource_scope("Chart", "not_a_method") is None
|
||||
|
||||
|
||||
def test_resource_scope_name_covers_all_tool_resource_classes() -> None:
|
||||
"""RESOURCE_SCOPE_NAME must cover every class_permission_name declared by
|
||||
MCP tools. If a new resource class is added, add it to the map."""
|
||||
assert set(RESOURCE_SCOPE_NAME.keys()) == {
|
||||
"Annotation",
|
||||
"Chart",
|
||||
"Dashboard",
|
||||
"Database",
|
||||
"Dataset",
|
||||
"Explore",
|
||||
"Query",
|
||||
"ReportSchedule",
|
||||
"Role",
|
||||
"Row Level Security",
|
||||
"SavedQuery",
|
||||
"SQLLab",
|
||||
"Tag",
|
||||
"Task",
|
||||
"Theme",
|
||||
"User",
|
||||
}
|
||||
|
||||
|
||||
def test_per_resource_scope_grants_matching_tool(app_context) -> None:
|
||||
"""A token scoped ONLY to superset:chart:write (no flat superset:write)
|
||||
still grants a Chart/write tool via the per-resource grant path."""
|
||||
g.user = MagicMock(username="editor")
|
||||
func = _make_tool_func(class_perm="Chart", method_perm="write")
|
||||
|
||||
mock_sm = MagicMock()
|
||||
mock_sm.can_access = MagicMock(return_value=True)
|
||||
with (
|
||||
patch("superset.mcp_service.auth.security_manager", mock_sm),
|
||||
_patch_token_scopes(["superset:chart:write"]),
|
||||
):
|
||||
result = check_tool_permission(func)
|
||||
|
||||
assert result is True
|
||||
|
||||
|
||||
def test_per_resource_scope_does_not_leak_across_resources(app_context) -> None:
|
||||
"""A token scoped to superset:chart:write does NOT grant a Dashboard/write
|
||||
tool (resource isolation)."""
|
||||
g.user = MagicMock(username="editor")
|
||||
func = _make_tool_func(class_perm="Dashboard", method_perm="write")
|
||||
|
||||
mock_sm = MagicMock()
|
||||
mock_sm.can_access = MagicMock(return_value=True)
|
||||
with (
|
||||
patch("superset.mcp_service.auth.security_manager", mock_sm),
|
||||
_patch_token_scopes(["superset:chart:write"]),
|
||||
):
|
||||
result = check_tool_permission(func)
|
||||
|
||||
assert result is False
|
||||
|
||||
|
||||
def test_per_resource_scope_enforces_action(app_context) -> None:
|
||||
"""A token scoped to superset:chart:read does NOT grant a Chart/write tool
|
||||
(action still enforced within the resource)."""
|
||||
g.user = MagicMock(username="editor")
|
||||
func = _make_tool_func(class_perm="Chart", method_perm="write")
|
||||
|
||||
mock_sm = MagicMock()
|
||||
mock_sm.can_access = MagicMock(return_value=True)
|
||||
with (
|
||||
patch("superset.mcp_service.auth.security_manager", mock_sm),
|
||||
_patch_token_scopes(["superset:chart:read"]),
|
||||
):
|
||||
result = check_tool_permission(func)
|
||||
|
||||
assert result is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# User/Role tools must request a permission FAB actually registers.
|
||||
#
|
||||
|
||||
@@ -233,13 +233,22 @@ async def test_api_key_passthrough_propagates_required_scopes() -> None:
|
||||
# -- Transport-layer DB validation (app configured) --
|
||||
|
||||
|
||||
def _make_app_with_api_key(username: str | None) -> MagicMock:
|
||||
"""Return a mock Flask app whose SecurityManager validates to ``username``."""
|
||||
def _make_app_with_api_key(
|
||||
username: str | None, scopes: str | None = None
|
||||
) -> MagicMock:
|
||||
"""Return a mock Flask app whose SecurityManager validates to ``username``.
|
||||
|
||||
``scopes`` is what ``get_api_key_scopes`` returns (FAB stores scopes as a
|
||||
comma-separated string, or None). It must be configured explicitly — an
|
||||
unconfigured MagicMock return value would raise on ``.split(",")`` inside
|
||||
the verifier's broad except-block and silently read as a rejected key.
|
||||
"""
|
||||
mock_user = MagicMock()
|
||||
mock_user.username = username
|
||||
|
||||
mock_sm = MagicMock()
|
||||
mock_sm.validate_api_key = MagicMock(return_value=mock_user if username else None)
|
||||
mock_sm.get_api_key_scopes = MagicMock(return_value=scopes)
|
||||
|
||||
mock_app = MagicMock()
|
||||
mock_app.app_context.return_value.__enter__ = MagicMock(return_value=None)
|
||||
@@ -264,6 +273,41 @@ async def test_transport_validation_valid_key_returns_access_token() -> None:
|
||||
assert result.claims.get(API_KEY_VALIDATED_USERNAME_CLAIM) == "alice"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_transport_validation_uses_keys_own_scopes() -> None:
|
||||
"""A key with its own ApiKey.scopes carries them on the AccessToken,
|
||||
parsed from FAB's comma-separated storage format."""
|
||||
mock_app = _make_app_with_api_key(
|
||||
"alice", scopes="superset:dashboard:read, superset:chart:read"
|
||||
)
|
||||
verifier = CompositeTokenVerifier(
|
||||
jwt_verifier=None, api_key_prefixes=["sst_"], app=mock_app
|
||||
)
|
||||
|
||||
result = await verifier.verify_token("sst_valid_key")
|
||||
|
||||
assert result is not None
|
||||
assert result.scopes == ["superset:dashboard:read", "superset:chart:read"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_transport_validation_no_key_scopes_remains_unscoped() -> None:
|
||||
"""A key without scopes remains unscoped despite global JWT requirements."""
|
||||
mock_app = _make_app_with_api_key("alice", scopes=None)
|
||||
jwt_verifier = MagicMock()
|
||||
jwt_verifier.required_scopes = ["superset:read"]
|
||||
jwt_verifier.verify_token = AsyncMock()
|
||||
|
||||
verifier = CompositeTokenVerifier(
|
||||
jwt_verifier=jwt_verifier, api_key_prefixes=["sst_"], app=mock_app
|
||||
)
|
||||
|
||||
result = await verifier.verify_token("sst_valid_key")
|
||||
|
||||
assert result is not None
|
||||
assert result.scopes == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_transport_validation_invalid_key_returns_none() -> None:
|
||||
"""An invalid API key is rejected at transport (returns None → HTTP 401)."""
|
||||
|
||||
Some files were not shown because too many files have changed in this diff Show More
Reference in New Issue
Block a user