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.

Sign up for GitHub

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

Jump to bottom

Package name validation #1998

Merged
merged 25 commits into from
Jan 15, 2021
Merged

Package name validation #1998

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7a95e2e
use NameInput and CommitInput
fiskus Jan 11, 2021
3537771
use helperText prop
fiskus Jan 11, 2021
ecc7b19
more straight edge between 3 levels of abstraction
fiskus Jan 11, 2021
40ffbf8
show warning when package already exists
fiskus Jan 11, 2021
2343072
use specific components
fiskus Jan 12, 2021
6c9a2f9
persistent package name
fiskus Jan 12, 2021
0a0eb9e
tune up warning messages
fiskus Jan 12, 2021
09316a5
remove unused validator
fiskus Jan 12, 2021
a6ebd87
workaround for react-form bug on losing focus
fiskus Jan 12, 2021
dfcb895
cosmetic change
fiskus Jan 12, 2021
78a5cf4
tweak text
fiskus Jan 12, 2021
93d0843
fix async/await bug
fiskus Jan 12, 2021
752c41b
Merge branch 'master' of github.com:quiltdata/quilt into package-name…
fiskus Jan 13, 2021
e668c35
skip name validation when name is not modified
fiskus Jan 13, 2021
00f54e9
onChanged -> onChange
fiskus Jan 13, 2021
d34c442
revert workaround for infinite spinner
fiskus Jan 13, 2021
6d09e95
manually manage validating state
fiskus Jan 13, 2021
1715499
remove flickering
fiskus Jan 13, 2021
102a540
Merge branch 'master' of github.com:quiltdata/quilt into package-name…
fiskus Jan 14, 2021
f00cd4b
check if package is present
fiskus Jan 14, 2021
687e8a1
reset state onExit
fiskus Jan 14, 2021
4018020
Consistency
fiskus Jan 14, 2021
726cd8f
check of package revisions list
fiskus Jan 14, 2021
f77aac5
use listOfObjects directly
fiskus Jan 14, 2021
998ea32
merge with master
fiskus Jan 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 30 additions & 10 deletions catalog/app/containers/Bucket/PackageCopyDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ function DialogForm({
workflowsConfig,
}) {
const nameValidator = PD.useNameValidator()
const nameExistence = PD.useNameExistence(successor.slug)

const initialMeta = React.useMemo(
() => ({
Expand Down Expand Up @@ -133,6 +134,30 @@ function DialogForm({
}
}

const defaultNameWarning = ' ' // Reserve space for warning
const [nameWarning, setNameWarning] = React.useState('')

const onFormChange = React.useCallback(
async ({ values }) => {
const { name } = values
const fullName = `${successor.slug}/${name}`

const nameExists = await nameExistence.validate(name)
if (nameExists) {
setNameWarning(`Package "${fullName}" exists. Submitting will revise it`)
return
}

if (name) {
setNameWarning(`Package "${fullName}" will be created`)
return
}

setNameWarning(defaultNameWarning)
},
[successor, nameExistence],
)

return (
<RF.Form onSubmit={onSubmit}>
{({
Expand All @@ -149,11 +174,11 @@ function DialogForm({
<DialogTitle bucket={successor.slug} />
<M.DialogContent style={{ paddingTop: 0 }}>
<form onSubmit={handleSubmit}>
<RF.FormSpy subscription={{ values: true }} onChange={onFormChange} />

<RF.Field
component={PD.Field}
component={PD.PackageNameInput}
name="name"
label="Name"
placeholder="e.g. user/package"
validate={validators.composeAsync(
validators.required,
nameValidator.validate,
Expand All @@ -163,23 +188,18 @@ function DialogForm({
required: 'Enter a package name',
invalid: 'Invalid package name',
}}
margin="normal"
fullWidth
helperText={nameWarning}
initialValue={initialName}
/>

<RF.Field
component={PD.Field}
component={PD.CommitMessageInput}
name="commitMessage"
label="Commit message"
placeholder="Enter a commit message"
validate={validators.required}
validateFields={['commitMessage']}
errors={{
required: 'Enter a commit message',
}}
fullWidth
margin="normal"
/>

<PD.SchemaFetcher
Expand Down
40 changes: 30 additions & 10 deletions catalog/app/containers/Bucket/PackageCreateDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,12 +349,14 @@ function PackageCreateDialog({
const [uploads, setUploads] = React.useState({})
const [success, setSuccess] = React.useState(null)
const nameValidator = PD.useNameValidator()
const nameExistence = PD.useNameExistence(bucket)

const reset = (form) => () => {
form.restart()
setSuccess(null)
setUploads({})
nameValidator.inc()
nameExistence.inc()
}

const handleClose = ({ submitting = false } = {}) => () => {
Expand Down Expand Up @@ -462,6 +464,25 @@ function PackageCreateDialog({
}
}

const defaultNameWarning = ' ' // Reserve space for warning
const [nameWarning, setNameWarning] = React.useState(defaultNameWarning)

const onFormChange = React.useCallback(
async ({ modified, values }) => {
if (!modified.name) return

const { name } = values

setNameWarning(defaultNameWarning)

const nameExists = await nameExistence.validate(name)
if (nameExists) {
setNameWarning(`Package "${name}" exists. Submitting will revise it`)
}
},
[nameExistence],
)

return (
<RF.Form onSubmit={uploadPackage}>
{({
Expand Down Expand Up @@ -533,11 +554,14 @@ function PackageCreateDialog({
<M.DialogTitle>Create package</M.DialogTitle>
<M.DialogContent style={{ paddingTop: 0 }}>
<form onSubmit={handleSubmit}>
<RF.FormSpy
subscription={{ modified: true, values: true }}
onChange={onFormChange}
/>

<RF.Field
component={PD.Field}
component={PD.PackageNameInput}
name="name"
label="Name"
placeholder="e.g. user/package"
validate={validators.composeAsync(
validators.required,
nameValidator.validate,
Expand All @@ -547,22 +571,18 @@ function PackageCreateDialog({
required: 'Enter a package name',
invalid: 'Invalid package name',
}}
margin="normal"
fullWidth
helperText={nameWarning}
validating={nameValidator.processing}
/>

<RF.Field
component={PD.Field}
component={PD.CommitMessageInput}
name="msg"
label="Commit message"
placeholder="Enter a commit message"
validate={validators.required}
validateFields={['msg']}
errors={{
required: 'Enter a commit message',
}}
fullWidth
margin="normal"
/>

<RF.Field
Expand Down
99 changes: 82 additions & 17 deletions catalog/app/containers/Bucket/PackageDialog/PackageDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,25 +93,61 @@ const readFile = (file) =>
reader.readAsText(file)
})

const validateName = (req) =>
cacheDebounce(async (name) => {
if (name) {
const res = await req({
endpoint: '/package_name_valid',
method: 'POST',
body: { name },
})
if (!res.valid) return 'invalid'
}
return undefined
}, 200)

export function useNameValidator() {
const req = APIConnector.use()
const [counter, setCounter] = React.useState(0)
const [processing, setProcessing] = React.useState(false)
const inc = React.useCallback(() => setCounter(R.inc), [setCounter])

const validator = React.useMemo(() => validateName(req), [req])

const validate = React.useCallback(
async (name) => {
setProcessing(true)
const error = await validator(name)
setProcessing(false)
return error
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[counter, validator],
)

return React.useMemo(() => ({ validate, processing, inc }), [validate, processing, inc])
}

export function useNameExistence(bucket) {
const [counter, setCounter] = React.useState(0)
const inc = React.useCallback(() => setCounter(R.inc), [setCounter])

const s3 = AWS.S3.use()

// eslint-disable-next-line react-hooks/exhaustive-deps
const validate = React.useCallback(
cacheDebounce(async (name) => {
if (name) {
const res = await req({
endpoint: '/package_name_valid',
method: 'POST',
body: { name },
const list = await requests.bucketListing({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should check for package existence here, not for data existence (either by querying the package index or listing the .quilt/named_packages/$handle dir)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know anything about this, please, provide more details

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see the contents of .quilt/named_packages dir: it has all the packages registered in the bucket and has the $handle/$pointer_file structure, so to check if the package exists in the bucket you should list objects under .quilt/named_packages/$handle/ prefix -- if there are any, the package exists, otherwise it does not.

Copy link
Member Author

@fiskus fiskus Jan 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added method requests.ensurePackageIsPresent, where i check for .quilt/named_packages/package_name/latest.

headObject(".quilt/named_packages/package_name") (just directory, without package revision) suddenly doesn't work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added method requests.ensurePackageIsPresent, where i check for .quilt/named_packages/package_name/latest.

i think listing revisions is more reliable, bc the latest tag may be absent for some reason

headObject(".quilt/named_packages/package_name") (just directory, without package revision) suddenly doesn't work

that's expected, bc head request only works for objects, and there's no such object, there's only prefix, so you should use listObjectsV2 for that prefix (you can limit number of keys to 1 to just see if there's anything under that prefix, no matter what)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed requests.ensurePackageIsPresent. Now it uses request.bucketListing (which uses listObjectsV2). I just check if any file under .named_packages/package_name

s3,
bucket,
path: name,
})
if (!res.valid) return 'invalid'
if (list.dirs.includes(`${name}/`)) return 'exists'
}
return undefined
}, 200),
[req, counter],
[bucket, counter, s3],
)

return React.useMemo(() => ({ validate, inc }), [validate, inc])
Expand Down Expand Up @@ -154,23 +190,52 @@ export const getMetaValue = (value) =>
)
: undefined

export function Field({ input, meta, errors, label, ...rest }) {
const error = meta.submitFailed && meta.error
const validating = meta.submitFailed && meta.validating
export function Field({ error, helperText, validating, warning, ...rest }) {
const props = {
InputLabelProps: { shrink: true },
InputProps: {
endAdornment: validating && <M.CircularProgress size={20} />,
},
error: !!error,
label: (
<>
{error ? errors[error] || error : label}
{validating && <M.CircularProgress size={13} style={{ marginLeft: 8 }} />}
</>
),
helperText: error || helperText,
...rest,
}
return <M.TextField {...props} />
}

export function PackageNameInput({ errors, input, meta, validating, ...rest }) {
nl0 marked this conversation as resolved.
Show resolved Hide resolved
const errorCode = (input.value || meta.submitFailed) && meta.error
const error = errorCode ? errors[errorCode] || errorCode : ''
const props = {
disabled: meta.submitting || meta.submitSucceeded,
InputLabelProps: { shrink: true },
error,
fullWidth: true,
label: 'Name',
margin: 'normal',
placeholder: 'e.g. user/package',
// NOTE: react-form doesn't change `FormState.validating` on async validation when field loses focus
validating,
...input,
...rest,
}
return <M.TextField {...props} />
return <Field {...props} />
}

export function CommitMessageInput({ errors, input, meta, ...rest }) {
const errorCode = meta.submitFailed && meta.error
const error = errorCode ? errors[errorCode] || errorCode : ''
const props = {
disabled: meta.submitting || meta.submitSucceeded,
error,
fullWidth: true,
label: 'Commit message',
margin: 'normal',
placeholder: 'Enter a commit message',
validating: meta.submitFailed && meta.validating,
...input,
...rest,
}
return <Field {...props} />
}

const useWorkflowInputStyles = M.makeStyles((t) => ({
Expand Down
42 changes: 32 additions & 10 deletions catalog/app/containers/Bucket/PackageUpdateDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,7 @@ function DialogForm({
const req = APIConnector.use()
const [uploads, setUploads] = React.useState({})
const nameValidator = PD.useNameValidator()
const nameExistence = PD.useNameExistence(bucket)

const initialMeta = React.useMemo(
() => ({
Expand Down Expand Up @@ -998,6 +999,29 @@ function DialogForm({
}
}

const defaultNameWarning = ' ' // Reserve space for warning
const [nameWarning, setNameWarning] = React.useState('')

const onFormChange = React.useCallback(
async ({ modified, values }) => {
if (!modified.name) return

const { name } = values

setNameWarning(defaultNameWarning)

if (initialName === name) return

const nameExists = await nameExistence.validate(name)
if (nameExists) {
setNameWarning('Package with this name exists already')
} else {
setNameWarning('New package will be created')
}
},
[initialName, nameExistence],
)

return (
<RF.Form onSubmit={onSubmitWrapped}>
{({
Expand All @@ -1014,11 +1038,14 @@ function DialogForm({
<M.DialogTitle>Push package revision</M.DialogTitle>
<M.DialogContent style={{ paddingTop: 0 }}>
<form onSubmit={handleSubmit}>
<RF.FormSpy
subscription={{ modified: true, values: true }}
onChange={onFormChange}
/>

<RF.Field
component={PD.Field}
component={PD.PackageNameInput}
name="name"
label="Name"
placeholder="e.g. user/package"
validate={validators.composeAsync(
validators.required,
nameValidator.validate,
Expand All @@ -1028,23 +1055,18 @@ function DialogForm({
required: 'Enter a package name',
invalid: 'Invalid package name',
}}
margin="normal"
fullWidth
nl0 marked this conversation as resolved.
Show resolved Hide resolved
helperText={nameWarning}
initialValue={initialName}
/>

<RF.Field
component={PD.Field}
component={PD.CommitMessageInput}
name="msg"
label="Commit message"
placeholder="Enter a commit message"
validate={validators.required}
validateFields={['msg']}
errors={{
required: 'Enter a commit message',
}}
fullWidth
margin="normal"
/>

<RF.Field
Expand Down