From 1339d8cbc7d57cc218004646161198ac4fc00951 Mon Sep 17 00:00:00 2001 From: mathieuRA Date: Fri, 28 Feb 2025 17:33:34 +0100 Subject: [PATCH] feedback --- @xen-orchestra/rest-api/.USAGE.md | 1 + @xen-orchestra/rest-api/README.md | 1 + .../rest-api/src/rest-api/rest-api.mts | 8 ++++---- .../rest-api/src/vms/vm.controller.mts | 18 +++++++++++------- packages/xo-server/src/xapi-stats.mjs | 8 +++++++- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/@xen-orchestra/rest-api/.USAGE.md b/@xen-orchestra/rest-api/.USAGE.md index 3278f62031a..4c17233b7d9 100644 --- a/@xen-orchestra/rest-api/.USAGE.md +++ b/@xen-orchestra/rest-api/.USAGE.md @@ -10,6 +10,7 @@ The REST API is based on the `TSOA` framework and therefore we use decorators a @Routes('foo') @Security('*') @Response(401) +@Tags('vms') @provide(Foo) class Foo extends Controller {} ``` diff --git a/@xen-orchestra/rest-api/README.md b/@xen-orchestra/rest-api/README.md index f2c74377071..1eaa5fea5c7 100644 --- a/@xen-orchestra/rest-api/README.md +++ b/@xen-orchestra/rest-api/README.md @@ -28,6 +28,7 @@ The REST API is based on the `TSOA` framework and therefore we use decorators a @Routes('foo') @Security('*') @Response(401) +@Tags('vms') @provide(Foo) class Foo extends Controller {} ``` diff --git a/@xen-orchestra/rest-api/src/rest-api/rest-api.mts b/@xen-orchestra/rest-api/src/rest-api/rest-api.mts index f78453f3972..9c99e3ad456 100644 --- a/@xen-orchestra/rest-api/src/rest-api/rest-api.mts +++ b/@xen-orchestra/rest-api/src/rest-api/rest-api.mts @@ -9,6 +9,10 @@ export class RestApi { this.#xoApp = xoApp } + get xoApp() { + return this.#xoApp + } + authenticateUser(...args: Parameters) { return this.#xoApp.authenticateUser(...args) } @@ -21,10 +25,6 @@ export class RestApi { return this.#xoApp.getObjectsByType(type, opts) } - getXapiVmStats(...args: Parameters) { - return this.#xoApp.getXapiVmStats(...args) - } - runWithApiContext(...args: Parameters) { return this.#xoApp.runWithApiContext(...args) } diff --git a/@xen-orchestra/rest-api/src/vms/vm.controller.mts b/@xen-orchestra/rest-api/src/vms/vm.controller.mts index c432c3bff92..f8fa00933b0 100644 --- a/@xen-orchestra/rest-api/src/vms/vm.controller.mts +++ b/@xen-orchestra/rest-api/src/vms/vm.controller.mts @@ -1,7 +1,7 @@ -import { Example, Get, Path, Query, Request, Response, Route, Security } from 'tsoa' +import { Example, Get, Path, Query, Request, Response, Route, Security, Tags } from 'tsoa' import { Request as ExRequest } from 'express' import { inject } from 'inversify' -import { invalidParameters } from 'xo-common/api-errors.js' +import { incorrectState, invalidParameters } from 'xo-common/api-errors.js' import { provide } from 'inversify-binding-decorators' import type { XapiStatsGranularity, XapiVmStats, XoVm } from '@vates/types' @@ -13,6 +13,7 @@ import { XapiXoController } from '../abstract-classes/xapi-xo-controller.mjs' @Route('vms') @Security('*') @Response(401, 'Authentication required') +@Tags('vms') // the `provide` decorator is mandatory on class that injects/receives dependencies. // It automatically bind the class to the IOC container that handles dependency injection @provide(VmController) @@ -59,14 +60,17 @@ export class VmController extends XapiXoController { @Example(vmStatsExample) @Get('{id}/stats') @Response(404, 'VM not found') - @Response(422, 'VM is halted or host could not be found') - @Response(422, 'Invalid granularity') + @Response(422, 'Invalid granularity, VM is halted or host could not be found') async getVmStats(@Path() id: string, @Query() granularity?: XapiStatsGranularity): Promise { try { - return await this.restApi.getXapiVmStats(id as XoVm['id'], granularity) + return await this.restApi.xoApp.getXapiVmStats(id as XoVm['id'], granularity) } catch (error) { - if (error instanceof Error && error.message === `VM ${id} is halted or host could not be found.`) { - /* throw */ invalidParameters(error.message, error) + if ( + incorrectState.is(error, { + property: 'resident_on', + }) + ) { + /* throw */ invalidParameters(`VM ${id} is halted or host could not be found.`, error) } throw error } diff --git a/packages/xo-server/src/xapi-stats.mjs b/packages/xo-server/src/xapi-stats.mjs index fb731e302c4..fff3f154338 100644 --- a/packages/xo-server/src/xapi-stats.mjs +++ b/packages/xo-server/src/xapi-stats.mjs @@ -9,6 +9,7 @@ import sum from 'lodash/sum.js' import uniq from 'lodash/uniq.js' import zipWith from 'lodash/zipWith.js' import { BaseError } from 'make-error' +import { incorrectState } from 'xo-common/api-errors.js' import { parseDateTime } from '@xen-orchestra/xapi' export class FaultyGranularity extends BaseError {} @@ -388,7 +389,12 @@ export default class XapiStats { const vm = xapi.getObject(vmId) const host = vm.$resident_on if (!host) { - throw new Error(`VM ${vmId} is halted or host could not be found.`) + /* throw */ incorrectState({ + actual: host, + expected: '', + object: vm, + property: 'resident_on', + }) } return this._getAndUpdateStats(xapi, {