From 0e800a589dd576bdebe5184b17c961122b7a0d3f Mon Sep 17 00:00:00 2001 From: Jonathan Jin Date: Sun, 2 Jun 2024 12:07:59 +0200 Subject: [PATCH] fix: singularize resource names where appropriate (#197) Closes #196. Currently, `kele-resource` uses the plural form of the resource name, e.g. `pods`, everywhere. This was done primarily out of convenience but obviously is grammatically inaccurate in some places, e.g. "get a single pods." Here we introduce a mechanism for looking up the canonical singular form of any given resource name from the discovery cache and apply it to the `kele-resource` suffix labels. --- docs/references/changelog.md | 2 + kele.el | 56 ++++++++++++++++++- .../fake-group/v1/serverresources.json | 2 +- .../fake-other-group/v1/serverresources.json | 2 +- tests/unit/test-kele.el | 19 +++++++ 5 files changed, 77 insertions(+), 4 deletions(-) diff --git a/docs/references/changelog.md b/docs/references/changelog.md index 70bc6c4..1875330 100644 --- a/docs/references/changelog.md +++ b/docs/references/changelog.md @@ -24,6 +24,8 @@ versioning][semver]. - Fixed a bug where kubeconfig cluster entries with uppercase letters in the server address erroneously cause resource kind completion to silently fail and show no kinds present in cluster +- Fixed a bug in `kele-resource` where the improper singular/plural form of the + resource name is used, e.g. "Get a single pods" instead of "Get a single pod" ### Changed diff --git a/kele.el b/kele.el index 2249e2b..496152d 100644 --- a/kele.el +++ b/kele.el @@ -342,6 +342,48 @@ If CONTEXT is nil, use the current context." (-map (-partial #'alist-get 'groupVersion)) (-sort (lambda (a _) (equal a "v1"))))) +(defun kele--resource-list-has-resource (name resource-list) + "Return non-nil when RESOURCE-LIST has resource named NAME. + +RESOURCE-LIST expected to be an alist mirroring the +APIResourceList schema." + (->> (alist-get 'resources resource-list) + (-any (lambda (resource) + (equal (alist-get 'name resource) name))))) + +(cl-defmethod kele--get-singular-for-plural ((cache kele--discovery-cache) + type + &key context) + "Look up the singular name for a given resource TYPE in CACHE. + +TYPE is expected to be the plural name of the resource. + +If CONTEXT is nil, use the current context." + (let* ((ctx (or context (kele-current-context-name))) + (resource-lists (kele--get-resource-lists-for-context cache ctx)) + + ;; TODO: + ;; - Accept group-version optional kwarg + ;; - Error if len(filtered-resource-lists) > 1 AND group-version not specified + ;; - Filter for group-version + (filtered-resource-lists (-filter (-partial #'kele--resource-list-has-resource type) resource-lists))) + + (when (> (length filtered-resource-lists) 1) + (warn "More than one group-version found for resource name `%s'; assuming `%s'" + type + (alist-get 'groupVersion (car filtered-resource-lists)))) + + (let-alist (car filtered-resource-lists) + (let* ((resource (-first (lambda (resource) + (string-equal + (alist-get 'name resource) + type)) + .resources)) + (lower (alist-get 'singularName resource))) + (if (or (not lower) (string-equal lower "")) + (downcase (alist-get 'kind resource)) + lower))))) + (cl-defmethod kele--resource-namespaced-p ((cache kele--discovery-cache) group-version type @@ -1698,7 +1740,13 @@ instead of \"pod.\"" :verb 'delete :resource .kind :context .context) - (format "Delete a single %s" (propertize .kind 'face 'warning)) + (format "Delete a single %s" (propertize + (kele--get-singular-for-plural + kele--global-discovery-cache + .kind + :context .context) + 'face + 'warning)) (format "Don't have permission to delete %s" .kind)))) (interactive (-let* ((kind (kele--get-kind-arg)) @@ -1748,7 +1796,11 @@ instead of \"pod.\"" :verb 'get :resource .kind :context .context) - (format "Get a single %s" (propertize .kind 'face 'warning)) + (format "Get a single %s" + (propertize + (kele--get-singular-for-plural kele--global-discovery-cache .kind :context .context) + 'face + 'warning)) (format "Don't have permission to get %s" .kind)))) (interactive (-let* ((kind (kele--get-kind-arg)) diff --git a/tests/testdata/cache/discovery/123.456.789.0_9999/fake-group/v1/serverresources.json b/tests/testdata/cache/discovery/123.456.789.0_9999/fake-group/v1/serverresources.json index 8658263..5de15cc 100644 --- a/tests/testdata/cache/discovery/123.456.789.0_9999/fake-group/v1/serverresources.json +++ b/tests/testdata/cache/discovery/123.456.789.0_9999/fake-group/v1/serverresources.json @@ -4,7 +4,7 @@ "kind": "AmbiguousThingFoo", "name": "ambiguousthings", "namespaced": true, - "singularName": "", + "singularName": "ambiguousthing", "verbs": [ "get" ] diff --git a/tests/testdata/cache/discovery/123.456.789.0_9999/fake-other-group/v1/serverresources.json b/tests/testdata/cache/discovery/123.456.789.0_9999/fake-other-group/v1/serverresources.json index 16d27a8..241679a 100644 --- a/tests/testdata/cache/discovery/123.456.789.0_9999/fake-other-group/v1/serverresources.json +++ b/tests/testdata/cache/discovery/123.456.789.0_9999/fake-other-group/v1/serverresources.json @@ -4,7 +4,7 @@ "kind": "AmbiguousThingBar", "name": "ambiguousthings", "namespaced": true, - "singularName": "", + "singularName": "ambiguousthing", "verbs": [ "get" ] diff --git a/tests/unit/test-kele.el b/tests/unit/test-kele.el index e52b228..563d1b5 100644 --- a/tests/unit/test-kele.el +++ b/tests/unit/test-kele.el @@ -336,6 +336,25 @@ (it "sets the timer" (expect (oref cache timer) :to-equal :timer))) + (describe "kele--get-singular-for-plural" + (before-each + (setq kele-cache-dir (f-expand "./tests/testdata/cache")) + (async-wait (kele--cache-update kele--global-discovery-cache))) + + (it "fetches singularName" + (expect + (kele--get-singular-for-plural kele--global-discovery-cache + "ambiguousthings") + :to-equal + "ambiguousthing")) + + (describe "when singularName not present" + (it "returns lowercased .metadata.name" + (expect (kele--get-singular-for-plural kele--global-discovery-cache + "componentstatuses") + :to-equal + "componentstatus")))) + (describe "kele--get-groupversions-for-type" (before-each (setq kele-cache-dir (f-expand "./tests/testdata/cache"))