Merge pull request #167 from gchq/dev/fix-delete-model

Dev/fix delete model

cypress/e2e/delete.cy.ts

@@ -0,0 +1,45 @@
+import convertNameToUrlFormat from '../utils/convertNameToUrlFormat'
+
+let modelUrl = ''
+
+describe('delete version', () => {
+  before(() => {
+    cy.log('Navigate to Upload page and json tab')
+    cy.visit('/upload')
+    cy.get('[data-test=uploadJsonTab]').click({ force: true })
+
+    cy.log('Selecting schema and inputting metadata')
+    cy.get('[data-test=selectSchemaInput]').trigger('mousedown', { force: true, button: 0 })
+    cy.fixture('schema_names.json').then((schemaNames) => {
+      cy.get(`[role=option]:contains(${schemaNames.model})`).click()
+    })
+
+    cy.fixture('minimal_metadata.json').then((modelMetadata) => {
+      const updatedMetadata = { ...modelMetadata }
+      updatedMetadata.buildOptions.uploadType = 'Model card only'
+      cy.get('[data-test=metadataTextarea]')
+        .clear()
+        .type(JSON.stringify(updatedMetadata), { parseSpecialCharSequences: false, delay: 0 })
+
+      cy.log('Submitting model')
+      cy.get('[data-test=warningCheckbox]').click()
+      cy.get('[data-test=submitButton]').click()
+      cy.url({ timeout: 15000 })
+        .should('contain', `/model/${convertNameToUrlFormat(updatedMetadata.highLevelDetails.name)}`)
+        .then((url) => {
+          modelUrl = url
+        })
+    })
+  })
+
+  it('Can delete a model version and all corresponding data', () => {
+    cy.visit(`${modelUrl}?tab=settings`)
+    cy.get('[data-test=modelSettingsPage]').contains('Danger Zone')
+    cy.log('Select delete version button')
+    cy.get('[data-test=deleteVersionButton]').click()
+    cy.log('Select confirm')
+    cy.get('[data-test=confirmButton]').click()
+    cy.visit(modelUrl)
+    cy.get('body').contains('Unable to find model')
+  })
+})

cypress/e2e/model.cy.ts

@@ -58,7 +58,7 @@ describe('Model with code and binary files', () => {
 
     cy.log('Checking model has been built')
     cy.get('[data-test=buildLogsTab]').click({ force: true })
-    cy.get('[data-test=terminalLog] > :last-child', { timeout: 300000 }).should(
+    cy.get('[data-test=terminalLog] > :last-child', { timeout: 350000 }).should(
       'contain',
       'Successfully completed build'
     )

data/api.ts

@@ -1,11 +1,11 @@
 export async function fetchEndpoint(url: string, method: string, data?: unknown) {
   return fetch(url, {
     method,
-    body: JSON.stringify(data),
     headers: {
       ...(!data && data !== 0 && { 'Content-Length': '0' }),
       'Content-Type': 'application/json',
     },
+    ...(data !== undefined && { body: JSON.stringify(data) }),
   })
 }
 
@@ -20,3 +20,7 @@ export async function postEndpoint(url: string, data: unknown) {
 export async function putEndpoint(url: string, data?: unknown) {
   return fetchEndpoint(url, 'PUT', data)
 }
+
+export async function deleteEndpoint(url: string) {
+  return fetchEndpoint(url, 'DELETE')
+}

package.json

@@ -66,6 +66,7 @@
     "mongodb": "^4.5.0",
     "mongodb-memory-server": "^8.5.2",
     "mongoose": "^5.13.14",
+    "mongoose-delete": "^0.5.4",
     "morgan": "^1.10.0",
     "multer": "^1.4.4",
     "nanoid": "^3.1.31",

pages/model/[uuid].tsx

@@ -28,7 +28,7 @@ import Tabs from '@mui/material/Tabs'
 import Typography from '@mui/material/Typography'
 import { useTheme } from '@mui/material/styles'
 import copy from 'copy-to-clipboard'
-import { postEndpoint, putEndpoint } from 'data/api'
+import { postEndpoint, putEndpoint, deleteEndpoint } from 'data/api'
 import { useGetModelDeployments, useGetModelVersion, useGetModelVersions } from 'data/model'
 import { useGetCurrentUser } from 'data/user'
 import { Types } from 'mongoose'
@@ -42,12 +42,13 @@ import ModelOverview from 'src/ModelOverview'
 import TerminalLog from 'src/TerminalLog'
 import Wrapper from 'src/Wrapper'
 import createComplianceFlow from 'utils/complianceFlow'
-import { getErrorMessage } from '@/utils/fetcher'
+import { getErrorMessage } from '../../utils/fetcher'
 import ApprovalsChip from '../../src/common/ApprovalsChip'
 import EmptyBlob from '../../src/common/EmptyBlob'
 import MultipleErrorWrapper from '../../src/errors/MultipleErrorWrapper'
 import { Deployment, User, Version, ModelUploadType, DateString } from '../../types/interfaces'
 import DisabledElementTooltip from '../../src/common/DisabledElementTooltip'
+import ConfirmationDialogue from '../../src/common/ConfirmationDialogue'
 import useNotification from '../../src/common/Snackbar'
 
 const ComplianceFlow = dynamic(() => import('../../src/ComplianceFlow'))
@@ -86,6 +87,8 @@ function Model() {
   const { versions, isVersionsLoading, isVersionsError } = useGetModelVersions(uuid)
   const { version, isVersionLoading, isVersionError, mutateVersion } = useGetModelVersion(uuid, selectedVersion, true)
   const { deployments, isDeploymentsLoading, isDeploymentsError } = useGetModelDeployments(uuid)
+  const [deleteConfirmOpen, setDeleteConfirmOpen] = useState(false)
+  const [deleteModelErrorMessage, setDeleteModelErrorMessage] = useState('')
 
   const hasUploadType = useMemo(() => version !== undefined && !!version.metadata.buildOptions?.uploadType, [version])
   const sendNotification = useNotification()
@@ -232,6 +235,25 @@ function Model() {
     }
   }
 
+  const handleDelete = () => {
+    setDeleteModelErrorMessage('')
+    setDeleteConfirmOpen(true)
+  }
+
+  const handleDeleteCancel = () => {
+    setDeleteConfirmOpen(false)
+  }
+
+  const handleDeleteConfirm = async () => {
+    const response = await deleteEndpoint(`/api/v1/version/${version._id}`)
+
+    if (response.ok) {
+      router.push('/')
+    } else {
+      setDeleteModelErrorMessage(await getErrorMessage(response))
+    }
+  }
+
   return (
     <Wrapper title={`Model: ${version.metadata.highLevelDetails.name}`} page='model'>
       {hasUploadType && version.metadata.buildOptions.uploadType === ModelUploadType.ModelCard && (
@@ -498,7 +520,7 @@ function Model() {
         )}
 
         {group === 'settings' && (
-          <>
+          <Box data-test='modelSettingsPage'>
             <Typography variant='h6' sx={{ mb: 1 }}>
               General
             </Typography>
@@ -510,14 +532,40 @@ function Model() {
             </Box>
 
             <Box sx={{ mb: 4 }} />
-
+            <ConfirmationDialogue
+              open={deleteConfirmOpen}
+              title='Delete version'
+              onConfirm={handleDeleteConfirm}
+              onCancel={handleDeleteCancel}
+              errorMessage={deleteModelErrorMessage}
+            />
             <Typography variant='h6' sx={{ mb: 1 }}>
               Danger Zone
             </Typography>
-            <Button variant='contained' color='error'>
-              Delete Model
-            </Button>
-          </>
+            <Stack direction='row' spacing={2}>
+              <DisabledElementTooltip
+                conditions={[
+                  currentUser.id !== version?.metadata?.contacts?.uploader
+                    ? 'You do not have permission to delete this version.'
+                    : '',
+                ]}
+                placement='bottom'
+              >
+                <Button
+                  variant='contained'
+                  disabled={currentUser.id !== version?.metadata?.contacts?.uploader}
+                  color='error'
+                  onClick={handleDelete}
+                  data-test='deleteVersionButton'
+                >
+                  Delete version
+                </Button>
+              </DisabledElementTooltip>
+              <Button variant='contained' color='error' disabled data-test='deleteModelButton'>
+                Delete model
+              </Button>
+            </Stack>
+          </Box>
         )}
       </Paper>
     </Wrapper>

server/index.ts

@@ -30,14 +30,22 @@ import { getSpecification } from './routes/v1/specification'
 import { getUiConfig } from './routes/v1/uiConfig'
 import { postUpload } from './routes/v1/upload'
 import { favouriteModel, getLoggedInUser, getUsers, postRegenerateToken, unfavouriteModel } from './routes/v1/users'
-import { getVersion, putVersion, resetVersionApprovals, updateLastViewed } from './routes/v1/version'
+import {
+  deleteVersion,
+  getVersion,
+  putVersion,
+  postResetVersionApprovals,
+  putUpdateLastViewed,
+} from './routes/v1/version'
 import { getApplicationLogs, getItemLogs } from './routes/v1/admin'
 import { connectToMongoose } from './utils/database'
 import logger, { expressErrorHandler, expressLogger } from './utils/logger'
 import { ensureBucketExists } from './utils/minio'
 import { getUser } from './utils/user'
 import { pullBuilderImage } from './utils/build/build'
 
+import { createRegistryClient, getImageDigest, deleteImageTag } from './utils/registry'
+
 const port = config.get('listen')
 const dev = process.env.NODE_ENV !== 'production'
 const app = next({
@@ -72,8 +80,9 @@ server.get('/api/v1/deployment/:uuid/version/:version/raw/:fileType', ...fetchRa
 
 server.get('/api/v1/version/:id', ...getVersion)
 server.put('/api/v1/version/:id', ...putVersion)
-server.post('/api/v1/version/:id/reset-approvals', ...resetVersionApprovals)
-server.put('/api/v1/version/:id/lastViewed/:role', ...updateLastViewed)
+server.delete('/api/v1/version/:id', ...deleteVersion)
+server.post('/api/v1/version/:id/reset-approvals', ...postResetVersionApprovals)
+server.put('/api/v1/version/:id/lastViewed/:role', ...putUpdateLastViewed)
 
 server.get('/api/v1/schemas', ...getSchemas)
 server.get('/api/v1/schema/default', ...getDefaultSchema)
@@ -111,6 +120,20 @@ export async function startServer() {
     ensureBucketExists(config.get('minio.registryBucket'))
   }
 
+  const registry = await createRegistryClient()
+  console.log(
+    await getImageDigest(registry, {
+      namespace: 'user',
+      model: 'yolo-v4-tiny-2x44vf',
+      version: 'v1.8',
+    })
+  )
+  // await deleteImageTag(registry, {
+  //   namespace: 'user',
+  //   model: 'yolo-v4-tiny-2x44vf',
+  //   version: 'v1.8',
+  // })
+
   // we don't actually need to wait for mongoose to connect before
   // we start serving connections
   connectToMongoose()

server/models/Deployment.ts

@@ -1,4 +1,5 @@
 import { Document, model, Schema, Types } from 'mongoose'
+import MongooseDelete from 'mongoose-delete'
 import logger from '../utils/logger'
 import { ModelDoc } from './Model'
 import { UserDoc } from './User'
@@ -55,6 +56,12 @@ const DeploymentSchema = new Schema<Deployment>(
   }
 )
 
+DeploymentSchema.plugin(MongooseDelete, {
+  overrideMethods: 'all',
+  deletedBy: true,
+  deletedByType: Schema.Types.ObjectId,
+})
+
 DeploymentSchema.methods.log = async function log(level: string, msg: string) {
   logger[level]({ deploymentId: this._id }, msg)
   // eslint-disable-next-line @typescript-eslint/no-use-before-define

server/models/Model.ts

@@ -1,4 +1,5 @@
 import { Document, model, Schema, Types } from 'mongoose'
+import MongooseDelete from 'mongoose-delete'
 import { UserDoc } from './User'
 import { VersionDoc } from './Version'
 
@@ -34,6 +35,8 @@ const ModelSchema = new Schema<Model>(
   }
 )
 
+ModelSchema.plugin(MongooseDelete, { overrideMethods: 'all', deletedBy: true, deletedByType: Schema.Types.ObjectId })
+
 const ModelModel = model<Model>('Model', ModelSchema)
 
 export async function createIndexes() {

server/models/Request.ts

@@ -1,4 +1,5 @@
 import { Document, model, Schema, Types } from 'mongoose'
+import MongooseDelete from 'mongoose-delete'
 import { DeploymentDoc } from './Deployment'
 import { approvalStateOptions, ApprovalStates } from '../../types/interfaces'
 import { UserDoc } from './User'
@@ -51,5 +52,7 @@ const RequestSchema = new Schema<Request>(
   }
 )
 
+RequestSchema.plugin(MongooseDelete, { overrideMethods: 'all', deletedBy: true, deletedByType: Schema.Types.ObjectId })
+
 const RequestModel = model<Request>('Request', RequestSchema)
 export default RequestModel

server/models/Version.ts

@@ -1,4 +1,5 @@
 import { Document, IndexOptions, model, Schema, Types } from 'mongoose'
+import MongooseDelete from 'mongoose-delete'
 import { LogStatement } from './Deployment'
 import { approvalStateOptions, ApprovalStates, DateString } from '../../types/interfaces'
 import { ModelDoc } from './Model'
@@ -56,6 +57,8 @@ const VersionSchema = new Schema<Version>(
   }
 )
 
+VersionSchema.plugin(MongooseDelete, { overrideMethods: 'all', deletedBy: true, deletedByType: Schema.Types.ObjectId })
+
 VersionSchema.index({ model: 1, version: 1 }, { unique: true } as unknown as IndexOptions)
 
 VersionSchema.methods.log = async function log(level: string, msg: string) {

server/routes/v1/deployment.ts

@@ -205,6 +205,12 @@ export const fetchRawModelFiles = [
       throw NotFound({ deploymentUuid: uuid }, `Unable to find deployment for uuid ${uuid}`)
     }
 
+    const versionDocument = await findVersionByName(req.user, deployment.model, version)
+
+    if (!versionDocument) {
+      throw NotFound({ deployment, version }, `Version ${version} not found for deployment ${deployment.uuid}.`)
+    }
+
     if (!req.user._id.equals(deployment.owner)) {
       throw Unauthorised(
         { deploymentOwner: deployment.owner },
@@ -223,14 +229,9 @@ export const fetchRawModelFiles = [
       throw NotFound({ fileType }, 'Unknown file type specified')
     }
 
-    const versionDocument = await findVersionByName(req.user, deployment.model, version)
     const bucketName: string = config.get('minio.uploadBucket')
     const client = new Minio.Client(config.get('minio'))
 
-    if (versionDocument === null) {
-      throw NotFound({ versionId: version }, `Unable to find version for id ${version}`)
-    }
-
     let filePath
 
     if (fileType === 'code') {
@@ -240,10 +241,8 @@ export const fetchRawModelFiles = [
       filePath = versionDocument.files.rawBinaryPath
     }
 
-    // Stat object to get size so browser can determine progress
     const { size } = await client.statObject(bucketName, filePath)
 
-    // res.set('Content-Disposition', contentDisposition(fileType, { type: 'inline' }))
     res.set('Content-disposition', `attachment; filename=${fileType}.zip`)
     res.set('Content-Type', 'application/zip')
     res.set('Cache-Control', 'private, max-age=604800, immutable')