Plugin API lookup key (#7224)

* Lookup plugin by slug

- Adjust plugin API to use plugin key and not (variable) pk value

* Fix for plugin table in CUI (legacy interface)

* Fix API endpoint layout:

- Move special endpoints first
- Fix "metadata" endpoint
- Allow custom "lookup_field" attribute for MetadataView

* Add "active_plugins" count to RegistryStatusView

* Updates for PUI

- Plugin management now uses slug rather than pk

* Bump API version

* Remove unused code

* Adds index on 'key' field for PluginConfig model

* Fix URL structure

* Unit test updates

* Unit test updates

* More unit test fixes
This commit is contained in:
Oliver 2024-05-15 14:12:37 +10:00 committed by GitHub
parent 2265055785
commit f8ef12f7bc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 136 additions and 104 deletions

View File

@ -550,6 +550,10 @@ class MetadataView(RetrieveUpdateAPI):
"""Return the model type associated with this API instance."""
model = self.kwargs.get(self.MODEL_REF, None)
if 'lookup_field' in self.kwargs:
# Set custom lookup field (instead of default 'pk' value) if supplied
self.lookup_field = self.kwargs.pop('lookup_field')
if model is None:
raise ValidationError(
f"MetadataView called without '{self.MODEL_REF}' parameter"

View File

@ -1,11 +1,14 @@
"""InvenTree API version information."""
# InvenTree API version
INVENTREE_API_VERSION = 196
INVENTREE_API_VERSION = 197
"""Increment this API version number whenever there is a significant change to the API that any clients need to know about."""
INVENTREE_API_TEXT = """
v197 - 2024-05-14 : https://github.com/inventree/InvenTree/pull/7224
- Refactor the plugin API endpoints to use the plugin "key" for lookup, rather than the PK value
v196 - 2024-05-05 : https://github.com/inventree/InvenTree/pull/7160
- Adds "location" field to BuildOutputComplete API endpoint

View File

@ -620,7 +620,7 @@ class PluginSettingsApiTest(PluginMixin, InvenTreeAPITestCase):
# get data
url = reverse(
'api-plugin-setting-detail', kwargs={'plugin': 'sample', 'key': 'API_KEY'}
'api-plugin-setting-detail', kwargs={'key': 'sample', 'setting': 'API_KEY'}
)
response = self.get(url, expected_code=200)
@ -637,7 +637,7 @@ class PluginSettingsApiTest(PluginMixin, InvenTreeAPITestCase):
# Non-existent plugin
url = reverse(
'api-plugin-setting-detail',
kwargs={'plugin': 'doesnotexist', 'key': 'doesnotmatter'},
kwargs={'key': 'doesnotexist', 'setting': 'doesnotmatter'},
)
response = self.get(url, expected_code=404)
self.assertIn("Plugin 'doesnotexist' not installed", str(response.data))
@ -645,7 +645,7 @@ class PluginSettingsApiTest(PluginMixin, InvenTreeAPITestCase):
# Wrong key
url = reverse(
'api-plugin-setting-detail',
kwargs={'plugin': 'sample', 'key': 'doesnotexist'},
kwargs={'key': 'sample', 'setting': 'doesnotexist'},
)
response = self.get(url, expected_code=404)
self.assertIn(

View File

@ -155,6 +155,7 @@ class PluginDetail(RetrieveUpdateDestroyAPI):
queryset = PluginConfig.objects.all()
serializer_class = PluginSerializers.PluginConfigSerializer
lookup_field = 'key'
def delete(self, request, *args, **kwargs):
"""Handle DELETE request for a PluginConfig instance.
@ -200,6 +201,7 @@ class PluginUninstall(UpdateAPI):
queryset = PluginConfig.objects.all()
serializer_class = PluginSerializers.PluginUninstallSerializer
permission_classes = [IsSuperuser]
lookup_field = 'key'
def perform_update(self, serializer):
"""Uninstall the plugin."""
@ -219,6 +221,7 @@ class PluginActivate(UpdateAPI):
queryset = PluginConfig.objects.all()
serializer_class = PluginSerializers.PluginActivateSerializer
permission_classes = [IsSuperuser]
lookup_field = 'key'
def get_object(self):
"""Returns the object for the view."""
@ -320,10 +323,10 @@ class PluginAllSettingList(APIView):
@extend_schema(
responses={200: PluginSerializers.PluginSettingSerializer(many=True)}
)
def get(self, request, pk):
def get(self, request, key):
"""Get all settings for a plugin config."""
# look up the plugin
plugin = check_plugin(None, pk)
plugin = check_plugin(key, None)
settings = getattr(plugin, 'settings', {})
@ -352,21 +355,21 @@ class PluginSettingDetail(RetrieveUpdateAPI):
The URL provides the 'slug' of the plugin, and the 'key' of the setting.
Both the 'slug' and 'key' must be valid, else a 404 error is raised
"""
key = self.kwargs['key']
setting_key = self.kwargs['setting']
# Look up plugin
plugin = check_plugin(
plugin_slug=self.kwargs.get('plugin'), plugin_pk=self.kwargs.get('pk')
)
plugin = check_plugin(self.kwargs.pop('key', None), None)
settings = getattr(plugin, 'settings', {})
if key not in settings:
if setting_key not in settings:
raise NotFound(
detail=f"Plugin '{plugin.slug}' has no setting matching '{key}'"
detail=f"Plugin '{plugin.slug}' has no setting matching '{setting_key}'"
)
return PluginSetting.get_setting_object(key, plugin=plugin.plugin_config())
return PluginSetting.get_setting_object(
setting_key, plugin=plugin.plugin_config()
)
# Staff permission required
permission_classes = [GlobalSettingsPermissions]
@ -384,7 +387,7 @@ class RegistryStatusView(APIView):
@extend_schema(responses={200: PluginSerializers.PluginRegistryStatusSerializer()})
def get(self, request):
"""Show registry status information."""
"""Show plugin registry status information."""
error_list = []
for stage, errors in registry.errors.items():
@ -397,7 +400,8 @@ class RegistryStatusView(APIView):
})
result = PluginSerializers.PluginRegistryStatusSerializer({
'registry_errors': error_list
'registry_errors': error_list,
'active_plugins': PluginConfig.objects.filter(active=True).count(),
}).data
return Response(result)
@ -410,31 +414,34 @@ plugin_api_urls = [
path(
'plugins/',
include([
# Plugin settings URLs
# Plugin management
path('reload/', PluginReload.as_view(), name='api-plugin-reload'),
path('install/', PluginInstall.as_view(), name='api-plugin-install'),
# Registry status
path(
'status/',
RegistryStatusView.as_view(),
name='api-plugin-registry-status',
),
path(
'settings/',
include([
re_path(
r'^(?P<plugin>[-\w]+)/(?P<key>\w+)/',
PluginSettingDetail.as_view(),
name='api-plugin-setting-detail',
), # Used for admin interface
path(
'', PluginSettingList.as_view(), name='api-plugin-setting-list'
),
)
]),
),
# Detail views for a single PluginConfig item
# Lookup for individual plugins (based on 'key', not 'pk')
path(
'<int:pk>/',
'<str:key>/',
include([
path(
'settings/',
include([
re_path(
r'^(?P<key>\w+)/',
r'^(?P<setting>\w+)/',
PluginSettingDetail.as_view(),
name='api-plugin-setting-detail-pk',
name='api-plugin-setting-detail',
),
path(
'',
@ -443,6 +450,12 @@ plugin_api_urls = [
),
]),
),
path(
'metadata/',
MetadataView.as_view(),
{'model': PluginConfig, 'lookup_field': 'key'},
name='api-plugin-metadata',
),
path(
'activate/',
PluginActivate.as_view(),
@ -456,23 +469,6 @@ plugin_api_urls = [
path('', PluginDetail.as_view(), name='api-plugin-detail'),
]),
),
# Metadata
path(
'metadata/',
MetadataView.as_view(),
{'model': PluginConfig},
name='api-plugin-metadata',
),
# Plugin management
path('reload/', PluginReload.as_view(), name='api-plugin-reload'),
path('install/', PluginInstall.as_view(), name='api-plugin-install'),
# Registry status
path(
'status/',
RegistryStatusView.as_view(),
name='api-plugin-registry-status',
),
# Anything else
path('', PluginList.as_view(), name='api-plugin-list'),
]),
),

View File

@ -0,0 +1,18 @@
# Generated by Django 4.2.12 on 2024-05-14 22:40
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('plugin', '0008_pluginconfig_package_name'),
]
operations = [
migrations.AlterField(
model_name='pluginconfig',
name='key',
field=models.CharField(db_index=True, help_text='Key of plugin', max_length=255, unique=True, verbose_name='Key'),
),
]

View File

@ -31,7 +31,11 @@ class PluginConfig(InvenTree.models.MetadataMixin, models.Model):
verbose_name_plural = _('Plugin Configurations')
key = models.CharField(
unique=True, max_length=255, verbose_name=_('Key'), help_text=_('Key of plugin')
unique=True,
db_index=True,
max_length=255,
verbose_name=_('Key'),
help_text=_('Key of plugin'),
)
name = models.CharField(

View File

@ -261,4 +261,10 @@ class PluginRegistryErrorSerializer(serializers.Serializer):
class PluginRegistryStatusSerializer(serializers.Serializer):
"""Serializer for plugin registry status."""
class Meta:
"""Meta for serializer."""
fields = ['active_plugins', 'registry_errors']
active_plugins = serializers.IntegerField(read_only=True)
registry_errors = serializers.ListField(child=PluginRegistryErrorSerializer())

View File

@ -97,12 +97,10 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
assert plgs is not None
self.assertEqual(plgs.active, active)
url = reverse('api-plugin-detail-activate', kwargs={'key': test_plg.key})
# Should not work - not a superuser
response = self.client.post(
reverse('api-plugin-detail-activate', kwargs={'pk': test_plg.pk}),
{},
follow=True,
)
response = self.client.post(url, {}, follow=True)
self.assertEqual(response.status_code, 403)
# Make user superuser
@ -115,11 +113,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
# Activate plugin with detail url
assert_plugin_active(self, False)
response = self.client.patch(
reverse('api-plugin-detail-activate', kwargs={'pk': test_plg.pk}),
{},
follow=True,
)
response = self.client.patch(url, {}, follow=True)
self.assertEqual(response.status_code, 200)
assert_plugin_active(self, True)
@ -129,11 +123,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
# Activate plugin
assert_plugin_active(self, False)
response = self.client.patch(
reverse('api-plugin-detail-activate', kwargs={'pk': test_plg.pk}),
{},
follow=True,
)
response = self.client.patch(url, {}, follow=True)
self.assertEqual(response.status_code, 200)
assert_plugin_active(self, True)
@ -237,7 +227,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
cfg = PluginConfig.objects.filter(key='sample').first()
assert cfg is not None
url = reverse('api-plugin-detail-activate', kwargs={'pk': cfg.pk})
url = reverse('api-plugin-detail-activate', kwargs={'key': cfg.key})
self.client.patch(url, {}, expected_code=200)
# Valid plugin settings endpoints
@ -246,7 +236,8 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
for key in valid_settings:
response = self.get(
reverse(
'api-plugin-setting-detail', kwargs={'plugin': 'sample', 'key': key}
'api-plugin-setting-detail',
kwargs={'key': 'sample', 'setting': key},
)
)
@ -256,7 +247,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
response = self.get(
reverse(
'api-plugin-setting-detail',
kwargs={'plugin': 'sample', 'key': 'INVALID_SETTING'},
kwargs={'key': 'sample', 'setting': 'INVALID_SETTING'},
),
expected_code=404,
)
@ -265,7 +256,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
response = self.get(
reverse(
'api-plugin-setting-detail',
kwargs={'plugin': 'sample', 'key': 'PROTECTED_SETTING'},
kwargs={'key': 'sample', 'setting': 'PROTECTED_SETTING'},
),
expected_code=200,
)
@ -276,7 +267,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
response = self.patch(
reverse(
'api-plugin-setting-detail',
kwargs={'plugin': 'sample', 'key': 'NUMERICAL_SETTING'},
kwargs={'key': 'sample', 'setting': 'NUMERICAL_SETTING'},
),
{'value': 456},
expected_code=200,
@ -288,7 +279,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase):
response = self.get(
reverse(
'api-plugin-setting-detail',
kwargs={'plugin': 'sample', 'key': 'NUMERICAL_SETTING'},
kwargs={'key': 'sample', 'setting': 'NUMERICAL_SETTING'},
),
expected_code=200,
)

View File

@ -114,9 +114,9 @@ function loadPluginTable(table, options={}) {
// Check if custom plugins are enabled for this instance
if (options.custom && !row.is_builtin && row.is_installed) {
if (row.active) {
buttons += makeIconButton('fa-stop-circle icon-red', 'btn-plugin-disable', row.pk, '{% trans "Disable Plugin" %}');
buttons += makeIconButton('fa-stop-circle icon-red', 'btn-plugin-disable', row.key, '{% trans "Disable Plugin" %}');
} else {
buttons += makeIconButton('fa-play-circle icon-green', 'btn-plugin-enable', row.pk, '{% trans "Enable Plugin" %}');
buttons += makeIconButton('fa-play-circle icon-green', 'btn-plugin-enable', row.key, '{% trans "Enable Plugin" %}');
}
}

View File

@ -80,9 +80,9 @@ export function GlobalSettingList({ keys }: { keys: string[] }) {
return <SettingList settingsState={globalSettings} keys={keys} />;
}
export function PluginSettingList({ pluginPk }: { pluginPk: string }) {
export function PluginSettingList({ pluginKey }: { pluginKey: string }) {
const pluginSettingsStore = useRef(
createPluginSettingsState({ plugin: pluginPk })
createPluginSettingsState({ plugin: pluginKey })
).current;
const pluginSettings = useStore(pluginSettingsStore);

View File

@ -131,8 +131,8 @@ export enum ApiEndpoints {
plugin_registry_status = 'plugins/status/',
plugin_install = 'plugins/install/',
plugin_reload = 'plugins/reload/',
plugin_activate = 'plugins/:id/activate/',
plugin_uninstall = 'plugins/:id/uninstall/',
plugin_activate = 'plugins/:key/activate/',
plugin_uninstall = 'plugins/:key/uninstall/',
// Machine API endpoints
machine_types_list = 'machine/types/',

View File

@ -40,12 +40,12 @@ export function useInstance<T = any>({
const [instance, setInstance] = useState<T | undefined>(defaultValue);
const instanceQuery = useQuery<T>({
queryKey: ['instance', endpoint, pk, params],
queryKey: ['instance', endpoint, pk, params, pathParams],
queryFn: async () => {
if (hasPrimaryKey) {
if (pk == null || pk == undefined || pk.length == 0 || pk == '-1') {
setInstance(defaultValue);
return null;
return defaultValue;
}
}
@ -63,7 +63,7 @@ export function useInstance<T = any>({
return response.data;
default:
setInstance(defaultValue);
return null;
return defaultValue;
}
})
.catch((error) => {
@ -72,7 +72,7 @@ export function useInstance<T = any>({
if (throwError) throw error;
return null;
return defaultValue;
});
},
refetchOnMount: refetchOnMount,

View File

@ -81,10 +81,10 @@ export interface PluginI {
}
export function PluginDrawer({
id,
pluginKey,
refreshTable
}: {
id: string;
pluginKey: string;
refreshTable: () => void;
}) {
const {
@ -93,7 +93,8 @@ export function PluginDrawer({
instanceQuery: { isFetching, error }
} = useInstance<PluginI>({
endpoint: ApiEndpoints.plugin_list,
pk: id,
hasPrimaryKey: true,
pk: pluginKey,
throwError: true
});
@ -102,15 +103,15 @@ export function PluginDrawer({
refreshInstance();
}, [refreshTable, refreshInstance]);
if (isFetching) {
if (!pluginKey || isFetching) {
return <LoadingOverlay visible={true} />;
}
if (error) {
if (!plugin || error) {
return (
<Text>
{(error as any)?.response?.status === 404 ? (
<Trans>Plugin with id {id} not found</Trans>
<Trans>Plugin with key {pluginKey} not found</Trans>
) : (
<Trans>An error occurred while fetching plugin details</Trans>
)}
@ -124,7 +125,7 @@ export function PluginDrawer({
<Box></Box>
<Group gap={'xs'}>
{plugin && PluginIcon(plugin)}
{plugin && <PluginIcon plugin={plugin} />}
<Title order={4}>
{plugin?.meta?.human_name ?? plugin?.name ?? '-'}
</Title>
@ -140,7 +141,7 @@ export function PluginDrawer({
openEditApiForm({
title: t`Edit plugin`,
url: ApiEndpoints.plugin_list,
pk: id,
pathParams: { key: pluginKey },
fields: {
active: {}
},
@ -224,13 +225,13 @@ export function PluginDrawer({
</Stack>
</Card>
{plugin && plugin.active && (
{plugin && plugin?.active && (
<Card withBorder>
<Stack gap="md">
<Title order={4}>
<Trans>Plugin settings</Trans>
</Title>
<PluginSettingList pluginPk={id} />
<PluginSettingList pluginKey={pluginKey} />
</Stack>
</Card>
)}
@ -241,9 +242,9 @@ export function PluginDrawer({
/**
* Construct an indicator icon for a single plugin
*/
function PluginIcon(plugin: PluginI) {
if (plugin.is_installed) {
if (plugin.active) {
function PluginIcon({ plugin }: { plugin: PluginI }) {
if (plugin?.is_installed) {
if (plugin?.active) {
return (
<Tooltip label={t`Plugin is active`}>
<IconCircleCheck color="green" />
@ -287,11 +288,13 @@ export default function PluginListTable() {
title: t`Plugin`,
sortable: true,
render: function (record: any) {
// TODO: Add link to plugin detail page
// TODO: Add custom badges
if (!record) {
return;
}
return (
<Group justify="left">
<PluginIcon {...record} />
<PluginIcon plugin={record} />
<Text>{record.name}</Text>
</Group>
);
@ -331,7 +334,7 @@ export default function PluginListTable() {
);
const activatePlugin = useCallback(
(plugin_id: number, plugin_name: string, active: boolean) => {
(plugin_key: string, plugin_name: string, active: boolean) => {
modals.openConfirmModal({
title: (
<StylishText>
@ -366,7 +369,9 @@ export default function PluginListTable() {
confirm: t`Confirm`
},
onConfirm: () => {
let url = apiUrl(ApiEndpoints.plugin_activate, plugin_id);
let url = apiUrl(ApiEndpoints.plugin_activate, null, {
key: plugin_key
});
const id = 'plugin-activate';
@ -424,7 +429,7 @@ export default function PluginListTable() {
color: 'red',
icon: <IconCircleX />,
onClick: () => {
activatePlugin(record.pk, record.name, false);
activatePlugin(record.key, record.name, false);
}
});
} else {
@ -433,7 +438,7 @@ export default function PluginListTable() {
color: 'green',
icon: <IconCircleCheck />,
onClick: () => {
activatePlugin(record.pk, record.name, true);
activatePlugin(record.key, record.name, true);
}
});
}
@ -464,7 +469,7 @@ export default function PluginListTable() {
color: 'red',
icon: <IconCircleX />,
onClick: () => {
setSelectedPlugin(record.pk);
setSelectedPlugin(record.key);
uninstallPluginModal.open();
},
disabled: plugins_install_disabled || false
@ -478,7 +483,7 @@ export default function PluginListTable() {
color: 'red',
icon: <IconCircleX />,
onClick: () => {
setSelectedPlugin(record.pk);
setSelectedPlugin(record.key);
deletePluginModal.open();
}
});
@ -519,12 +524,12 @@ export default function PluginListTable() {
}
});
const [selectedPlugin, setSelectedPlugin] = useState<number>(-1);
const [selectedPlugin, setSelectedPlugin] = useState<string>('');
const uninstallPluginModal = useEditApiFormModal({
title: t`Uninstall Plugin`,
url: ApiEndpoints.plugin_uninstall,
pk: selectedPlugin,
pathParams: { key: selectedPlugin },
fetchInitialData: false,
timeout: 30000,
fields: {
@ -556,7 +561,7 @@ export default function PluginListTable() {
const deletePluginModal = useDeleteApiFormModal({
url: ApiEndpoints.plugin_list,
pk: selectedPlugin,
pathParams: { key: selectedPlugin },
title: t`Delete Plugin`,
onFormSuccess: table.refreshTable,
preFormWarning: t`Deleting this plugin configuration will remove all associated settings and data. Are you sure you want to delete this plugin?`
@ -618,9 +623,14 @@ export default function PluginListTable() {
<DetailDrawer
title={t`Plugin Detail`}
size={'50%'}
renderContent={(id) => {
if (!id) return false;
return <PluginDrawer id={id} refreshTable={table.refreshTable} />;
renderContent={(pluginKey) => {
if (!pluginKey) return;
return (
<PluginDrawer
pluginKey={pluginKey}
refreshTable={table.refreshTable}
/>
);
}}
/>
<InvenTreeTable
@ -630,7 +640,7 @@ export default function PluginListTable() {
props={{
enableDownload: false,
rowActions: rowActions,
onRowClick: (plugin) => navigate(`${plugin.pk}/`),
onRowClick: (plugin) => navigate(`${plugin.key}/`),
tableActions: tableActions,
tableFilters: [
{