Merge pull request #827 from fractal-analytics-platform/import-json-schema-validation

Fixed JSON schema validation during arguments import

Author: zonia3000

CHANGELOG.md

@@ -3,6 +3,9 @@
 # Unreleased
 
 * Added body request normalizer (\#825);
+* Fixed JSON schema validation during arguments import (\#827);
+* Updated slim-select version to support range selection (\#827);
+* Supported pre/post `pinned_package_versions` in task-collection (\#827);
 
 # 1.19.3
 

__tests__/v2/TaskCollection.test.js

@@ -147,11 +147,13 @@ describe('TaskCollection', () => {
 
 		const [key1, key2] = screen.getAllByRole('textbox', { name: 'Name' });
 		const [value1, value2] = screen.getAllByRole('textbox', { name: 'Version' });
+		const [pre1] = screen.getAllByRole('radio', { name: 'Pre' });
 
 		await fireEvent.input(key1, { target: { value: 'package1' } });
 		await fireEvent.input(value1, { target: { value: '1.2.3' } });
 		await fireEvent.input(key2, { target: { value: 'package2' } });
 		await fireEvent.input(value2, { target: { value: '0.0.8' } });
+		await fireEvent.click(pre1);
 
 		const packageInput = screen.getByRole('textbox', { name: 'Package' });
 		await fireEvent.input(packageInput, { target: { value: 'main-package' } });
@@ -165,8 +167,10 @@ describe('TaskCollection', () => {
 				// @ts-expect-error
 				body: expect.toBeFormDataWith({
 					package: 'main-package',
-					pinned_package_versions: JSON.stringify({
-						package1: '1.2.3',
+					pinned_package_versions_pre: JSON.stringify({
+						package1: '1.2.3'
+					}),
+					pinned_package_versions_post: JSON.stringify({
 						package2: '0.0.8'
 					})
 				})

components/package-lock.json

@@ -54,7 +54,7 @@
 		"color-hash": "^2.0.2",
 		"dompurify": "^3.2.3",
 		"marked": "^7.0.4",
-		"slim-select": "^2.8.1",
+		"slim-select": "^2.12.1",
 		"svelte-preprocess": "^6.0.3"
 	},
 	"svelte": "./dist/index.js",

components/package.json

@@ -376,7 +376,7 @@
 	</div>
 </div>
 {#if allRows.length === 0}
-	<p>
+	<p class="mt-4">
 		There are no available tasks. You can add new tasks on the
 		<a href="/v2/tasks/management">Tasks management</a> page.
 	</p>

components/src/lib/tasks/FilteredTasksTable.svelte

@@ -50,7 +50,7 @@
 		"jose": "^4.14.4",
 		"log4js": "^6.9.1",
 		"marked": "^7.0.4",
-		"slim-select": "^2.8.1"
+		"slim-select": "^2.12.1"
 	},
 	"files": [
 		"build"

package-lock.json

@@ -2,6 +2,7 @@
 	import { FormErrorHandler } from '$lib/common/errors';
 	import TaskGroupSelector from './TaskGroupSelector.svelte';
 	import { recentActivities } from '$lib/stores';
+	import { PropertyDescription } from 'fractal-components';
 
 	/**
 	 * @typedef {Object} Props
@@ -16,7 +17,7 @@
 	let package_version = $state('');
 	let python_version = $state('');
 	let package_extras = $state('');
-	/** @type {{key: string, value: string}[]} */
+	/** @type {Array<{key: string, value: string, type: 'pre' | 'post'}>} */
 	let pinnedPackageVersions = $state([]);
 	let privateTask = $state(false);
 	let selectedGroup = $state(null);
@@ -78,9 +79,13 @@
 			formData.append('package_version', package_version);
 		}
 
-		const ppv = getPinnedPackageVersionsMap();
-		if (ppv) {
-			formData.append('pinned_package_versions', JSON.stringify(ppv));
+		const ppvPre = getPinnedPackageVersionsMap('pre');
+		if (ppvPre) {
+			formData.append('pinned_package_versions_pre', JSON.stringify(ppvPre));
+		}
+		const ppvPost = getPinnedPackageVersionsMap('post');
+		if (ppvPost) {
+			formData.append('pinned_package_versions_post', JSON.stringify(ppvPost));
 		}
 
 		let url = `/api/v2/task/collect/pip?private=${privateTask}`;
@@ -115,13 +120,14 @@
 	}
 
 	/**
+	 * @param {'pre'|'post'} type
 	 * @returns {{[key: string]: string}|undefined}
 	 */
-	function getPinnedPackageVersionsMap() {
+	function getPinnedPackageVersionsMap(type) {
 		/** @type {{[key: string]: string}} */
 		const map = {};
 		for (const ppv of pinnedPackageVersions) {
-			if (ppv.key && ppv.value) {
+			if (ppv.key && ppv.value && ppv.type === type) {
 				map[ppv.key] = ppv.value;
 			}
 		}
@@ -132,7 +138,7 @@
 	}
 
 	function addPackageVersion() {
-		pinnedPackageVersions = [...pinnedPackageVersions, { key: '', value: '' }];
+		pinnedPackageVersions = [...pinnedPackageVersions, { key: '', value: '', type: 'post' }];
 	}
 
 	/**
@@ -275,24 +281,51 @@
 		{/if}
 		{#each pinnedPackageVersions as ppv, i (i)}
 			<div class="row">
-				<div class="col-xl-6 col-lg-8 col-md-12 mb-2">
+				<div class="col-xl-8 col-lg-10 col-md-12 mb-2">
 					<div class="input-group">
 						<label class="input-group-text" for="ppv_key_{i}">Name</label>
 						<input
 							type="text"
-							class="form-control"
+							class="form-control ppv-input"
 							id="ppv_key_{i}"
 							bind:value={ppv.key}
 							required
 						/>
 						<label class="input-group-text" for="ppv_value_{i}">Version</label>
 						<input
 							type="text"
-							class="form-control"
+							class="form-control ppv-input"
 							id="ppv_value_{i}"
 							bind:value={ppv.value}
 							required
 						/>
+						<span class="input-group-text">
+							<div class="form-check form-check-inline">
+								<input
+									class="form-check-input"
+									type="radio"
+									name="ppv-type-{i}"
+									id="ppv-pre-{i}"
+									value="pre"
+									bind:group={ppv.type}
+								/>
+								<label class="form-check-label" for="ppv-pre-{i}">Pre</label>
+							</div>
+							<div class="form-check form-check-inline me-1">
+								<input
+									class="form-check-input"
+									type="radio"
+									name="ppv-type-{i}"
+									id="ppv-post-{i}"
+									value="post"
+									bind:group={ppv.type}
+								/>
+								<label class="form-check-label" for="ppv-post-{i}">Post</label>
+							</div>
+							<PropertyDescription
+								description="Whether the pinned dependency should be installed before or after the main package"
+							/>
+						</span>
 						<button
 							class="btn btn-outline-secondary"
 							type="button"
@@ -347,3 +380,9 @@
 		</div>
 	</form>
 </div>
+
+<style>
+	.ppv-input {
+		min-width: 100px;
+	}
+</style>

package.json

@@ -1,6 +1,14 @@
 <script>
 	import { downloadBlob } from '$lib/common/component_utilities';
 	import Modal from '$lib/components/common/Modal.svelte';
+	import {
+		getPropertiesToIgnore,
+		JsonSchemaDataError,
+		SchemaValidator,
+		stripNullAndEmptyObjectsAndArrays
+	} from 'fractal-components';
+	import { adaptJsonSchema } from 'fractal-components/jschema/jschema_adapter';
+	import JsonSchemaValidationErrors from './JsonSchemaValidationErrors.svelte';
 
 	/**
 	 * @typedef {Object} Props
@@ -20,17 +28,21 @@
 	/** @type {HTMLInputElement|undefined} */
 	let importArgsFileInput = $state(undefined);
 	let importArgsError = $state('');
+	/** @type {import('ajv').ErrorObject[] | undefined} */
+	let validationErrors = $state();
 
 	function onImportArgsModalOpen() {
 		importArgsFiles = null;
 		if (importArgsFileInput) {
 			importArgsFileInput.value = '';
 		}
 		importArgsError = '';
+		validationErrors = undefined;
 	}
 
 	async function importArgs() {
 		importArgsError = '';
+		validationErrors = undefined;
 		if (importArgsFiles === null || importArgsFiles.length === 0) {
 			return;
 		}
@@ -43,11 +55,53 @@
 			importArgsError = "File doesn't contain valid JSON";
 			return;
 		}
+		try {
+			if (json.args_parallel) {
+				validateArgumentsForImport(
+					workflowTask.task.args_schema_parallel,
+					workflowTask.task.args_schema_version,
+					json.args_parallel
+				);
+			}
+			if (json.args_non_parallel) {
+				validateArgumentsForImport(
+					workflowTask.task.args_schema_non_parallel,
+					workflowTask.task.args_schema_version,
+					json.args_non_parallel
+				);
+			}
+		} catch (err) {
+			if (err instanceof Error) {
+				importArgsError = err.message;
+			}
+			return;
+		}
 		importArgsModal?.confirmAndHide(async function () {
 			await onImport(json);
 		});
 	}
 
+	/**
+	 * @param {object} schema
+	 * @param {'pydantic_v1'|'pydantic_v2'} schemaVersion
+	 * @param {object} data
+	 */
+	function validateArgumentsForImport(schema, schemaVersion, data) {
+		if (!schema) {
+			return;
+		}
+		const jsonSchema = adaptJsonSchema(schema, getPropertiesToIgnore(false));
+		const validator = new SchemaValidator(schemaVersion);
+		const isSchemaValid = validator.loadSchema(jsonSchema);
+		if (!isSchemaValid) {
+			throw new Error('Invalid JSON Schema');
+		}
+		if (!validator.isValid(stripNullAndEmptyObjectsAndArrays(data))) {
+			validationErrors = validator.getErrors() || undefined;
+			throw new JsonSchemaDataError(validator.getErrors());
+		}
+	}
+
 	function exportArgs() {
 		const args = {
 			args_parallel: workflowTask.args_parallel,
@@ -120,6 +174,11 @@
 				<div id="errorAlert-importArgumentsModal"></div>
 			</div>
 		</div>
+		<div class="row mt-0">
+			<div class="col">
+				<JsonSchemaValidationErrors title="Invalid arguments:" {validationErrors} />
+			</div>
+		</div>
 	{/snippet}
 	{#snippet footer()}
 		<button class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button>

src/lib/components/v2/tasks/TaskCollection.svelte

@@ -0,0 +1,49 @@
+<script>
+	/**
+	 * @typedef {Object} Props
+	 * @property {import('ajv').ErrorObject[] | undefined} validationErrors
+	 * @property {string} title
+	 */
+
+	/** @type {Props} */
+	let { validationErrors, title } = $props();
+</script>
+
+{#if validationErrors}
+	<div class="alert alert-danger">
+		<p>{title}</p>
+		<ul id="validation-errors">
+			{#each validationErrors as error, index (index)}
+				<li>
+					{#if error.instancePath !== ''}
+						{error.instancePath}:
+					{/if}
+					{#if error.keyword === 'additionalProperties'}
+						must NOT have additional property '{error.params.additionalProperty}'
+					{:else}
+						{error.message}
+					{/if}
+					<small
+						data-bs-toggle="collapse"
+						data-bs-target="#collapse-{index}"
+						aria-expanded="true"
+						aria-controls="collapse-{index}"
+						class="text-primary"
+						role="button"
+					>
+						more
+					</small>
+					<div
+						id="collapse-{index}"
+						class="accordion-collapse collapse"
+						data-bs-parent="#validation-errors"
+					>
+						<div class="accordion-body">
+							<pre class="alert alert-warning mt-1">{JSON.stringify(error, null, 2)}</pre>
+						</div>
+					</div>
+				</li>
+			{/each}
+		</ul>
+	</div>
+{/if}