Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Two Schemas for workflows/config.yaml, package validation, default package name #2364

Merged
merged 63 commits into from Oct 14, 2021
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
25adc4d
update ajv
fiskus Sep 27, 2021
6b89d60
update workflows JSON Schema
fiskus Sep 27, 2021
0e14ee9
fix workflows Schema and catalog specific Schema
fiskus Sep 28, 2021
773c5ef
apply new Schema validation for catalog workflows
fiskus Sep 28, 2021
e032908
fix Schema format
fiskus Sep 28, 2021
83cf642
this feature is not ready yet
fiskus Sep 28, 2021
49b4a4b
fix tests according to ajv update
fiskus Sep 29, 2021
7254f02
rewrite ajv options
fiskus Sep 29, 2021
914ba38
fix ajv errors format
fiskus Sep 29, 2021
b778edb
fix schema
fiskus Sep 29, 2021
43d94bd
add package_handle support
fiskus Sep 29, 2021
e5e5b24
add hook for getting package name template
fiskus Sep 29, 2021
0d9d5f3
minor changes
fiskus Sep 30, 2021
10017fe
use new approach for changing name on workflow change
fiskus Sep 30, 2021
60d3f80
add tests
fiskus Sep 30, 2021
1545cc7
add return type
fiskus Oct 4, 2021
907bf40
add object_schema property
fiskus Oct 4, 2021
7a83ab9
handle versioning too
fiskus Oct 4, 2021
3406757
use handle_pattern template
fiskus Oct 5, 2021
191f1f2
show pattern error
fiskus Oct 5, 2021
7c76725
add entries validator
fiskus Oct 5, 2021
956ad11
adjust UI
fiskus Oct 6, 2021
728f98c
show files error
fiskus Oct 6, 2021
3fdb01b
Merge branch 'master' of github.com:quiltdata/quilt into two-schemas
fiskus Oct 6, 2021
919b335
clear error message on files add
fiskus Oct 7, 2021
1d9372f
Update shared/schemas/workflows.yml.json
fiskus Oct 7, 2021
047393e
use object version
fiskus Oct 7, 2021
5607921
use regex format too
fiskus Oct 7, 2021
0b384c5
Merge branch 'two-schemas' of github.com:quiltdata/quilt into two-sch…
fiskus Oct 7, 2021
85ed54a
rename schemas
fiskus Oct 7, 2021
478c0a5
Merge branch 'master' of github.com:quiltdata/quilt into two-schemas
fiskus Oct 7, 2021
0b0805e
add previous version
fiskus Oct 7, 2021
fab09e5
fix uploading valid package
fiskus Oct 7, 2021
cea88dc
Remove extra empty line
fiskus Oct 7, 2021
a5c6e72
remove outdated code
fiskus Oct 7, 2021
9858999
add description
fiskus Oct 8, 2021
42825ce
Apply suggestions from code review
fiskus Oct 8, 2021
58af2e7
Merge branch 'two-schemas' of github.com:quiltdata/quilt into two-sch…
fiskus Oct 8, 2021
846d4fe
omit file extension
fiskus Oct 8, 2021
e5856c9
update example
fiskus Oct 8, 2021
804751b
add console.error
fiskus Oct 8, 2021
4df465c
hide console.logs
fiskus Oct 8, 2021
33a7624
use caret for version
fiskus Oct 8, 2021
9a52687
disable entries validation for package from direcotry
fiskus Oct 8, 2021
20eb08f
validate s3 files too
fiskus Oct 8, 2021
af5f139
Merge branch 'master' of github.com:quiltdata/quilt into two-schemas
fiskus Oct 8, 2021
dabd012
update version Schema
fiskus Oct 11, 2021
660dea7
use workflow value intsead of workflow getter
fiskus Oct 11, 2021
27955f1
Apply suggestions from code review
fiskus Oct 11, 2021
94f1268
add workflow calculation inline
fiskus Oct 11, 2021
0acd939
remove catalog if there is no catalog version in workflows/config.yaml
fiskus Oct 11, 2021
ffae8c5
use all files state to calc actual files
fiskus Oct 11, 2021
6087f56
fix schema
fiskus Oct 11, 2021
bc6f703
Merge branch 'master' of github.com:quiltdata/quilt into two-schemas
fiskus Oct 12, 2021
c6b1fee
Merge branch 'master' of github.com:quiltdata/quilt into two-schemas
fiskus Oct 14, 2021
607df74
fix mistypings
fiskus Oct 14, 2021
89fae6e
catch Ajv errors
fiskus Oct 14, 2021
18b61eb
dont rewrite existing name
fiskus Oct 14, 2021
1e64883
adjust unit-test
fiskus Oct 14, 2021
a5b71c7
divide empty string made on purpose and lack of value
fiskus Oct 14, 2021
fb51a25
fix unit test
fiskus Oct 14, 2021
1ea1d7f
exporting and dont use exported is confusing
fiskus Oct 14, 2021
cb212cf
Merge branch 'master' of github.com:quiltdata/quilt into two-schemas
fiskus Oct 14, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 9 additions & 3 deletions catalog/app/containers/Bucket/PackageCopyDialog.js
Expand Up @@ -81,7 +81,7 @@ function DialogForm({
validate: validateMetaInput,
workflowsConfig,
}) {
const nameValidator = PD.useNameValidator()
const nameValidator = PD.useNameValidator(selectedWorkflow)
const nameExistence = PD.useNameExistence(successor.slug)
const [nameWarning, setNameWarning] = React.useState('')
const [metaHeight, setMetaHeight] = React.useState(0)
Expand Down Expand Up @@ -142,14 +142,18 @@ function DialogForm({
const [editorElement, setEditorElement] = React.useState()

const onFormChange = React.useCallback(
async ({ values }) => {
async ({ modified, values }) => {
if (document.body.contains(editorElement)) {
setMetaHeight(editorElement.clientHeight)
}

if (modified.workflow && values.workflow !== selectedWorkflow) {
setWorkflow(values.workflow)
}

handleNameChange(values.name)
},
[editorElement, handleNameChange, setMetaHeight],
[editorElement, handleNameChange, selectedWorkflow, setMetaHeight, setWorkflow],
)

React.useEffect(() => {
Expand Down Expand Up @@ -212,6 +216,7 @@ function DialogForm({
<RF.Field
component={PD.PackageNameInput}
name="name"
workflow={selectedWorkflow || workflowsConfig}
validate={validators.composeAsync(
validators.required,
nameValidator.validate,
Expand All @@ -220,6 +225,7 @@ function DialogForm({
errors={{
required: 'Enter a package name',
invalid: 'Invalid package name',
pattern: `Name should match ${selectedWorkflow?.packageNamePattern}`,
}}
helperText={nameWarning}
initialValue={initialName}
Expand Down
Expand Up @@ -18,9 +18,9 @@ function SingleError({ error }) {

return (
<Lab.Alert severity="error">
{error.dataPath && (
{error.instancePath && (
<>
<code className={classes.code}>{error.dataPath}</code>
<code className={classes.code}>{error.instancePath}</code>
</>
)}
{error.message}
Expand All @@ -34,7 +34,7 @@ export default function ErrorHelper({ className, error }) {
return (
<div className={className}>
{Array.isArray(error) ? (
error.map((e) => <SingleError error={e} key={e.dataPath + e.message} />)
error.map((e) => <SingleError error={e} key={e.instancePath + e.message} />)
) : (
<SingleError error={error} />
)}
Expand Down
@@ -1,3 +1,5 @@
import type { ErrorObject } from 'ajv'
import cx from 'classnames'
import * as FF from 'final-form'
import * as FP from 'fp-ts'
import * as R from 'ramda'
Expand All @@ -18,6 +20,7 @@ import DialogLoading from './DialogLoading'
import DialogSuccess, { DialogSuccessRenderMessageProps } from './DialogSuccess'
import * as FI from './FilesInput'
import * as Layout from './Layout'
import MetaInputErrorHelper from './MetaInputErrorHelper'
import * as PD from './PackageDialog'
import { isS3File, S3File } from './S3FilePicker'
import { FormSkeleton, MetaInputSkeleton } from './Skeleton'
Expand Down Expand Up @@ -56,6 +59,14 @@ const useStyles = M.makeStyles((t) => ({
files: {
height: '100%',
},
filesWithError: {
height: `calc(90% - ${t.spacing()}px)`,
},
filesError: {
marginTop: t.spacing(),
maxHeight: t.spacing(9),
overflowY: 'auto',
},
form: {
height: '100%',
},
Expand Down Expand Up @@ -106,14 +117,18 @@ export function PackageCreationForm({
disableStateDisplay,
ui = {},
}: PackageCreationFormProps & PD.SchemaFetcherRenderProps) {
const nameValidator = PD.useNameValidator()
const nameValidator = PD.useNameValidator(selectedWorkflow)
const nameExistence = PD.useNameExistence(bucket)
const [nameWarning, setNameWarning] = React.useState<React.ReactNode>('')
const [metaHeight, setMetaHeight] = React.useState(0)
const classes = useStyles()
const dialogContentClasses = PD.useContentStyles({ metaHeight })
const validateWorkflow = PD.useWorkflowValidator(workflowsConfig)

const [entriesError, setEntriesError] = React.useState<(Error | ErrorObject)[] | null>(
null,
)

const [selectedBucket, selectBucket] = React.useState(sourceBuckets.getDefault)

const existingEntries = initial?.manifest?.entries ?? EMPTY_MANIFEST_ENTRIES
Expand All @@ -137,6 +152,7 @@ export function PackageCreationForm({
)

const createPackage = requests.useCreatePackage()
const validateEntries = PD.useEntriesValidator(selectedWorkflow)

const onSubmit = async ({
name,
Expand Down Expand Up @@ -167,6 +183,24 @@ export function PackageCreationForm({
return !e || e.hash !== file.hash.value
})

const entries = FP.function.pipe(
R.mergeLeft(files.added, files.existing),
nl0 marked this conversation as resolved.
Show resolved Hide resolved
R.omit(Object.keys(files.deleted)),
Object.entries,
R.map(([path, file]) => ({
logical_key: path,
size: file.size,
})),
)

const error = await validateEntries(entries)
if (error && error.length) {
setEntriesError(error)
return {
files: 'schema',
}
}

let uploadedEntries
try {
uploadedEntries = await uploads.upload({
Expand Down Expand Up @@ -230,7 +264,7 @@ export function PackageCreationForm({
// eslint-disable-next-line no-console
console.log('error creating manifest', e)
// TODO: handle specific cases?
const errorMessage = e instanceof Error ? e.message : null
const errorMessage = e instanceof Error ? (e as Error).message : null
return { [FF.FORM_ERROR]: errorMessage || PD.ERROR_MESSAGES.MANIFEST }
}
}
Expand Down Expand Up @@ -269,6 +303,7 @@ export function PackageCreationForm({
const onFormChange = React.useCallback(
async ({ dirtyFields, values }) => {
if (dirtyFields?.name) handleNameChange(values.name)
if (dirtyFields?.files) setEntriesError(null)
},
[handleNameChange],
)
Expand Down Expand Up @@ -328,7 +363,7 @@ export function PackageCreationForm({
<RF.FormSpy
subscription={{ modified: true, values: true }}
onChange={({ modified, values }) => {
if (modified!.workflow) {
if (modified!.workflow && values.workflow !== selectedWorkflow) {
setWorkflow(values.workflow)
}
}}
Expand All @@ -354,6 +389,7 @@ export function PackageCreationForm({

<RF.Field
component={PD.PackageNameInput}
workflow={selectedWorkflow || workflowsConfig}
initialValue={initial?.name}
name="name"
validate={validators.composeAsync(
Expand All @@ -364,6 +400,7 @@ export function PackageCreationForm({
errors={{
required: 'Enter a package name',
invalid: 'Invalid package name',
pattern: `Name should match ${selectedWorkflow?.packageNamePattern}`,
}}
helperText={nameWarning}
validating={nameValidator.processing}
Expand Down Expand Up @@ -400,14 +437,17 @@ export function PackageCreationForm({

<Layout.RightColumn>
<RF.Field
className={classes.files}
className={cx(classes.files, {
[classes.filesWithError]: !!entriesError,
})}
// @ts-expect-error
component={FI.FilesInput}
name="files"
validate={validateFiles as FF.FieldValidator<$TSFixMe>}
validateFields={['files']}
errors={{
nonEmpty: 'Add files to create a package',
schema: 'Files should match schema',
[FI.HASHING]: 'Please wait while we hash the files',
[FI.HASHING_ERROR]:
'Error hashing files, probably some of them are too large. Please try again or contact support.',
Expand All @@ -424,6 +464,11 @@ export function PackageCreationForm({
disableStateDisplay={disableStateDisplay}
ui={{ reset: ui.resetFiles }}
/>

<MetaInputErrorHelper
className={classes.filesError}
error={entriesError}
/>
</Layout.RightColumn>
</Layout.Container>

Expand Down
Expand Up @@ -55,7 +55,7 @@ describe('containers/Bucket/PackageDialog/PackageDialog', () => {

test("should return error when metadata isn't compliant with Schema", () => {
expect(PD.mkMetaValidator({ type: 'array' })({ any: 'thing' })).toMatchObject([
{ message: 'should be array' },
{ message: 'must be array' },
])
})

Expand Down
78 changes: 73 additions & 5 deletions catalog/app/containers/Bucket/PackageDialog/PackageDialog.tsx
@@ -1,16 +1,20 @@
import { basename } from 'path'

import cx from 'classnames'
import { FORM_ERROR } from 'final-form'
import mime from 'mime-types'
import * as R from 'ramda'
import * as React from 'react'
import { useDropzone } from 'react-dropzone'
import type * as RF from 'react-final-form'
import * as redux from 'react-redux'
import * as M from '@material-ui/core'
import { fade } from '@material-ui/core/styles'
import * as Lab from '@material-ui/lab'

import JsonEditor from 'components/JsonEditor'
import { parseJSON, stringifyJSON } from 'components/JsonEditor/utils'
import * as authSelectors from 'containers/Auth/selectors'
import * as Notifications from 'containers/Notifications'
import { useData } from 'utils/Data'
import Delay from 'utils/Delay'
Expand All @@ -19,6 +23,8 @@ import * as AWS from 'utils/AWS'
import * as Sentry from 'utils/Sentry'
import useDragging from 'utils/dragging'
import { JsonSchema, makeSchemaValidator } from 'utils/json-schema'
import * as packageHandleUtils from 'utils/packageHandle'
import * as s3paths from 'utils/s3paths'
import * as spreadsheets from 'utils/spreadsheets'
import { readableBytes } from 'utils/string'
import * as workflows from 'utils/workflows'
Expand Down Expand Up @@ -127,7 +133,7 @@ const validateName = (req: ApiRequest) =>
return undefined
}, 200)

export function useNameValidator() {
export function useNameValidator(workflow?: workflows.Workflow) {
const req: ApiRequest = APIConnector.use()
const [counter, setCounter] = React.useState(0)
const [processing, setProcessing] = React.useState(false)
Expand All @@ -137,6 +143,10 @@ export function useNameValidator() {

const validate = React.useCallback(
async (name: string) => {
if (workflow?.packageNamePattern?.test(name) === false) {
return 'pattern'
}

setProcessing(true)
try {
const error = await validator(name)
Expand All @@ -148,7 +158,7 @@ export function useNameValidator() {
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[counter, validator],
[counter, validator, workflow?.packageNamePattern],
)

return React.useMemo(() => ({ validate, processing, inc }), [validate, processing, inc])
Expand Down Expand Up @@ -237,35 +247,64 @@ export function Field({
interface PackageNameInputOwnProps {
errors: Record<string, React.ReactNode>
input: RF.FieldInputProps<string>
directory?: string
meta: RF.FieldMetaState<string>
validating: boolean
workflow: { packageName: packageHandleUtils.NameTemplates }
}

type PackageNameInputProps = PackageNameInputOwnProps &
Omit<Parameters<typeof Field>[0], keyof PackageNameInputOwnProps>

export function PackageNameInput({
errors,
input,
input: { value, onChange },
meta,
workflow,
directory,
validating,
...rest
}: PackageNameInputProps) {
const readyForValidation = (input.value && meta.modified) || meta.submitFailed
const readyForValidation = (value && meta.modified) || meta.submitFailed
const errorCode = readyForValidation && meta.error
const error = errorCode ? errors[errorCode] || errorCode : ''
const [modified, setModified] = React.useState(meta.modified)
const handleChange = React.useCallback(
(event) => {
setModified(true)
onChange(event)
},
[onChange, setModified],
)
const props = {
disabled: meta.submitting || meta.submitSucceeded,
error,
fullWidth: true,
label: 'Name',
margin: 'normal' as const,
onChange: handleChange,
placeholder: 'e.g. user/package',
// NOTE: react-form doesn't change `FormState.validating` on async validation when field loses focus
validating,
...input,
value,
...rest,
}
const username = redux.useSelector(authSelectors.username)
React.useEffect(() => {
if (modified) return

const packageName = getDefaultPackageName(workflow, {
username,
directory,
})
if (!packageName) return

onChange({
target: {
value: packageName,
},
})
}, [directory, workflow, modified, onChange, username])
return <Field {...props} />
}

Expand Down Expand Up @@ -775,6 +814,18 @@ export function getUsernamePrefix(username?: string | null) {
return validParts ? `${validParts.join('')}/` : ''
}

const getDefaultPackageName = (
workflow: { packageName: packageHandleUtils.NameTemplates },
{ directory, username }: { directory?: string; username: string },
) =>
directory
nl0 marked this conversation as resolved.
Show resolved Hide resolved
? packageHandleUtils.execTemplate(workflow?.packageName, 'files', {
directory: basename(directory),
nl0 marked this conversation as resolved.
Show resolved Hide resolved
})
: packageHandleUtils.execTemplate(workflow?.packageName, 'packages', {
username: s3paths.ensureNoSlash(getUsernamePrefix(username)),
})

const usePackageNameWarningStyles = M.makeStyles({
root: {
marginRight: '4px',
Expand Down Expand Up @@ -819,3 +870,20 @@ export function DialogWrapper({
)
return <M.Dialog {...props} />
}

export function useEntriesValidator(workflow?: workflows.Workflow) {
const s3 = AWS.S3.use()

return React.useCallback(
async (entries: $TSFixMe) => {
const schemaUrl = workflow?.entriesSchema
if (!schemaUrl) return undefined
const entriesSchema = await requests.objectSchema({ s3, schemaUrl })
// TODO: Show error if there is network error
if (!entriesSchema) return undefined

return makeSchemaValidator(entriesSchema)(entries)
},
[workflow, s3],
)
}