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

Allow dollar in layer name #8241

Merged
merged 12 commits into from
Dec 10, 2024
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
### Changed
- Renamed "resolution" to "magnification" in more places within the codebase, including local variables. [#8168](https://github.com/scalableminds/webknossos/pull/8168)
- Reading image files on datastore filesystem is now done asynchronously. [#8126](https://github.com/scalableminds/webknossos/pull/8126)
- Layer names are now allowed to contain `$` as special characters. [#8241](https://github.com/scalableminds/webknossos/pull/8241)
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Make the changelog entry more precise.

The current entry could be misinterpreted to suggest that $ is allowed in all names. Consider revising it to explicitly state the distinction between layer names and dataset names.

-Layer names are now allowed to contain `$` as special characters. [#8241](https://github.com/scalableminds/webknossos/pull/8241)
+Layer names (but not dataset names) are now allowed to contain the `$` character. [#8241](https://github.com/scalableminds/webknossos/pull/8241)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Layer names are now allowed to contain `$` as special characters. [#8241](https://github.com/scalableminds/webknossos/pull/8241)
- Layer names (but not dataset names) are now allowed to contain the `$` character. [#8241](https://github.com/scalableminds/webknossos/pull/8241)

- Datasets can now be renamed and can have duplicate names. [#8075](https://github.com/scalableminds/webknossos/pull/8075)
- Improved error messages for starting jobs on datasets from other organizations. [#8181](https://github.com/scalableminds/webknossos/pull/8181)
- Terms of Service for Webknossos are now accepted at registration, not afterward. [#8193](https://github.com/scalableminds/webknossos/pull/8193)
Expand Down
11 changes: 11 additions & 0 deletions conf/evolutions/125-allow-dollar-in-layer-names.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
START TRANSACTION;

do $$ begin ASSERT (select schemaVersion from webknossos.releaseInformation) = 124, 'Previous schema version mismatch'; end; $$ LANGUAGE plpgsql;


ALTER TABLE webknossos.annotation_layers DROP CONSTRAINT IF EXISTS annotation_layers_name_check;
ALTER TABLE webknossos.annotation_layers ADD CONSTRAINT annotation_layers_name_check CHECK (name ~* '^[A-Za-z0-9\-_\.\$]+$');

UPDATE webknossos.releaseInformation SET schemaVersion = 125;

COMMIT TRANSACTION;
13 changes: 13 additions & 0 deletions conf/evolutions/reversions/125-allow-dollar-in-layer-names.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
START TRANSACTION;

-- This reversion might take a while because it needs to search in all annotation layer names for '$' and replace it with ''
do $$ begin ASSERT (select schemaVersion from webknossos.releaseInformation) = 125, 'Previous schema version mismatch'; end; $$ LANGUAGE plpgsql;

UPDATE webknossos.annotation_layers SET name = regexp_replace(name, '\$', '', 'g') WHERE name ~* '\$';
daniel-wer marked this conversation as resolved.
Show resolved Hide resolved

ALTER TABLE webknossos.annotation_layers DROP CONSTRAINT IF EXISTS annotation_layers_name_check;
ALTER TABLE webknossos.annotation_layers ADD CONSTRAINT annotation_layers_name_check CHECK (name ~* '^[A-Za-z0-9\-_\.]+$');

daniel-wer marked this conversation as resolved.
Show resolved Hide resolved
UPDATE webknossos.releaseInformation SET schemaVersion = 124;

COMMIT TRANSACTION;
19 changes: 13 additions & 6 deletions frontend/javascripts/admin/dataset/dataset_components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,10 @@ export function CardContainer({
);
}
}
export const layerNameRules = [
const sharedRules = [
{
min: 1,
},
// Note that these rules are also checked by the backend
{
pattern: /^[0-9a-zA-Z_.-]+$/,
message: "Only letters, digits and the following characters are allowed: . _ -",
},
{
validator: syncValidator(
(value: string | null) => !value || !value.startsWith("."),
Expand All @@ -52,13 +47,25 @@ export const layerNameRules = [
},
];

export const layerNameRules = [
...sharedRules,
{
pattern: /^[0-9a-zA-Z_.\-$.]+$/,
message: "Only letters, digits and the following characters are allowed: . _ - $",
},
];

export const getDatasetNameRules = (activeUser: APIUser | null | undefined) => [
{
required: true,
message: messages["dataset.import.required.name"],
},
{ min: 3, message: messages["dataset.name_length"] },
...layerNameRules,
{
pattern: /^[0-9a-zA-Z_.-]+$/,
message: "Only letters, digits and the following characters are allowed: . _ -",
},
{
validator: async () => {
if (!activeUser) throw new Error("Can't do operation if no user is logged in.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,9 @@ function SimpleLayerForm({
{
validator: syncValidator(
(value: string) =>
dataLayers.filter((someLayer: APIDataLayer) => someLayer.name === value)
.length <= 1,
form
.getFieldValue(["dataSource", "dataLayers"])
.filter((someLayer: APIDataLayer) => someLayer.name === value).length <= 1,
"Layer names must be unique.",
),
},
Expand Down
16 changes: 11 additions & 5 deletions frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -339,12 +339,18 @@ class DatasetSettingsView extends React.PureComponent<PropsWithFormAndRouter, St
this.state.activeDataSourceEditMode === "simple" ? "advanced" : "simple",
);

const afterForceUpdateCallback = () =>
const afterForceUpdateCallback = () => {
// Trigger validation manually, because fields may have been updated
form
.validateFields()
.then((formValues) => this.submit(formValues))
.catch((errorInfo) => this.handleValidationFailed(errorInfo));
// and defer the validation as it is done asynchronously by antd or so.
setTimeout(
() =>
form
.validateFields()
.then((formValues) => this.submit(formValues))
.catch((errorInfo) => this.handleValidationFailed(errorInfo)),
0,
);
};
daniel-wer marked this conversation as resolved.
Show resolved Hide resolved

// Need to force update of the SimpleAdvancedDataForm as removing a layer in the advanced tab does not update
// the form items in the simple tab (only the values are updated). The form items automatically update once
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ export function checkLayerNameForInvalidCharacters(readableLayerName: string): V
message: messages["tracing.volume_layer_name_starts_with_dot"],
};
}
const uriSafeCharactersRegex = /[0-9a-zA-Z-._]+/g;
const validLayerNameCharactersRegex = /[0-9a-zA-Z-._$]+/g;
// Removing all URISaveCharacters from readableLayerName. The leftover chars are all invalid.
const allInvalidChars = readableLayerName.replace(uriSafeCharactersRegex, "");
const allInvalidChars = readableLayerName.replace(validLayerNameCharactersRegex, "");
const allUniqueInvalidCharsAsSet = new Set(allInvalidChars);
const allUniqueInvalidCharsAsString = "".concat(...allUniqueInvalidCharsAsSet.values());
const isValid = allUniqueInvalidCharsAsString.length === 0;
Expand Down
4 changes: 2 additions & 2 deletions tools/postgres/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ CREATE TABLE webknossos.releaseInformation (
schemaVersion BIGINT NOT NULL
);

INSERT INTO webknossos.releaseInformation(schemaVersion) values(124);
INSERT INTO webknossos.releaseInformation(schemaVersion) values(125);
COMMIT TRANSACTION;


Expand Down Expand Up @@ -56,7 +56,7 @@ CREATE TABLE webknossos.annotation_layers(
_annotation CHAR(24) NOT NULL,
tracingId CHAR(36) NOT NULL UNIQUE,
typ webknossos.ANNOTATION_LAYER_TYPE NOT NULL,
name VARCHAR(256) NOT NULL CHECK (name ~* '^[A-Za-z0-9\-_\.]+$'),
name VARCHAR(256) NOT NULL CHECK (name ~* '^[A-Za-z0-9\-_\.\$]+$'),
statistics JSONB NOT NULL,
UNIQUE (name, _annotation),
PRIMARY KEY (_annotation, tracingId),
Expand Down