This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8644-00cf017d8f5e70a8070e24dbee0176ee0f1b45f6 in repository https://gitbox.apache.org/repos/asf/texera.git
commit b7993fbafac472d0ef1b87d86169978c42343ac4 Author: Suryaa Charan <[email protected]> AuthorDate: Sat Oct 3 00:36:16 2026 +0000 feat: add a delimiter picker with regex validation (#8644) ### What changes were proposed in this PR? Delimiter properties were plain text boxes. A stored tab looked empty, typing `\t` stopped at the backslash, and Unnest String accepted invalid or degenerate regexes without a word. This replaces them with a **delimiter picker**: common delimiters by name, plus a **Custom…** entry. Each operator picks a mode through a schema hint (`@JsonSchemaInject(json = UIWidget.UIWidgetCharDelimiter / UIWidgetRegexDelimiter)`). ``` CSV scans (char) Comma Tab Semicolon Pipe Space | Custom… (1 char; typed \t -> tab) Unnest String (regex) Comma Tab Semicolon Pipe (\|) Whitespace (\s+) New line | Custom regex… (validated) ``` | Area | Change | |---|---| | Frontend | New formly type `delimiter` (`frontend/src/app/common/formly/delimiter/`). Works in the canvas property panel and the Form View. Delimiter errors show immediately, including for invalid values loaded from a saved workflow. | | Regex check (browser) | Must compile, must not match the empty string (`\|`, `\s*`), must not match every character (`.`). The last is **exact**: it tests every BMP character other than line breaks, stopping at the first miss, so `[a0-]` or `[^,]` pass. Patterns using Java-only syntax (inline flags, possessive quantifiers, `\p{..}`, `\x{..}`, `\Q..\E`, `\e`, `\h`, `\z`, …) are left to the backend, because JavaScript can't parse them or reads them differently. | | Regex check (backend) | `UnnestStringOpDesc` makes the same checks with Java's engine during schema propagation, so a bad pattern is a **compile error** instead of a runtime `PatternSyntaxException` or an empty result. | | CSV scans | `CSVFileScan`, `ParallelCSVFileScan`, `CSVOldFileScan` use the char-mode widget. `maxLength: 1` is kept. | **Saved workflows:** the stored value is still the delimiter itself, so existing workflows load unchanged and a stored tab shows as "Tab". **One intentional behavior change:** a saved Unnest String delimiter that matches the empty string or every character (e.g. `|`, `.`) now fails to compile. Previously it ran and silently split every character apart, or output only line breaks. #### Before / after <!-- drag comparison/C1-csv-stored-tab.png here --> <!-- drag comparison/C2-csv-typing-tab.png here --> <!-- drag comparison/C3-csv-choices.png here --> <!-- drag comparison/C4-csv-legacy-value.png here --> <!-- drag comparison/C5-unnest-choices.png here --> <!-- drag comparison/C6-unnest-pipe.png here --> <!-- drag comparison/C7-unnest-dot.png here --> <!-- drag comparison/C8-unnest-invalid.png here --> <!-- drag comparison/C9-unnest-compile.png here --> <img width="1124" height="425" alt="C1-csv-stored-tab" src="https://github.com/user-attachments/assets/ab9ba44d-9598-4675-be5f-89f08325a576"; /> <img width="1124" height="425" alt="C2-csv-typing-tab" src="https://github.com/user-attachments/assets/29aac797-7da7-4b52-a300-cd64e22c3810"; /> <img width="1124" height="1030" alt="C3-csv-choices" src="https://github.com/user-attachments/assets/61fb0b71-3588-476c-9003-64cd2f0b9996"; /> <img width="1124" height="574" alt="C4-csv-legacy-value" src="https://github.com/user-attachments/assets/098d5824-22a1-4e9f-b96c-5c42ed750fe5"; /> <img width="1124" height="1018" alt="C5-unnest-choices" src="https://github.com/user-attachments/assets/babde8bb-faf3-4e0a-b46b-04da73496cdd"; /> <img width="1124" height="607" alt="C6-unnest-pipe" src="https://github.com/user-attachments/assets/1d7bf753-a114-40b4-bdec-62676530a443"; /> <img width="1124" height="651" alt="C7-unnest-dot" src="https://github.com/user-attachments/assets/e6157de3-5ff1-4eb1-80df-de1d9dd12da9"; /> <img width="1124" height="562" alt="C8-unnest-invalid" src="https://github.com/user-attachments/assets/d370d8af-86a4-4aaf-ae75-f1a722af5335"; /> <img width="1884" height="750" alt="C9-unnest-compile" src="https://github.com/user-attachments/assets/7de580fc-927a-4bdf-a5ba-43298c8fd48d"; /> ### Any related issues, documentation, discussions? Closes #8638 ### How was this PR tested? **Automated.** Tests were written first and seen failing, then made to pass. | Suite | Result | |---|---| | Frontend, full (`ng test`) | 232 files, 6,376 passed | | Delimiter / formly specs (included above) | presets for both modes, custom-regex validation (valid, invalid, empty-match, every-character, Java-only syntax), edge cases, and a rendered test that feeds the backend's exact schema through formly-jsonschema | | `WorkflowOperator/test`, full | 2,602 passed | | New backend specs | each CSV preset read by all three CSV readers; Unnest String split with every regex preset and custom patterns; compile-time rejection with Java semantics | | Lint / format / types / build | eslint, prettier-eslint, `tsc`, `ng build`, scalafmt: clean | **Manual, in the browser.** Local stack (`bin/local-dev.sh up`), Chrome driven by Playwright, 1600×1000: 1. Open a workflow with CSV File Scan and Unnest String, click each operator, and open the Delimiter picker. 2. Pick each preset; choose Custom… and type `#`, `\t`, `ab`. 3. For Unnest String, choose Custom regex… and type `[,;]\s*`, `(`, `[a-`, `a{2,1}`, `|`, `\s*`, `.`, `\.`, `,++`, then clear the field. 4. Type `(?i)(` and `(?s).`: the browser leaves them to the backend, and the Result Panel shows the compile error. All 33 scripted checks pass. The before/after screenshots above come from this run and from the same flow on the base commit. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: suryaacharan <[email protected]> Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]> --- .../operator/metadata/annotations/UIWidget.java | 7 + .../source/scan/csv/CSVScanSourceOpDesc.scala | 3 +- .../scan/csv/ParallelCSVScanSourceOpDesc.scala | 3 +- .../scan/csvOld/CSVOldScanSourceOpDesc.scala | 3 +- .../operator/unneststring/UnnestStringOpDesc.scala | 72 +++++- .../source/scan/csv/CSVScanSourceOpDescSpec.scala | 90 ++++++++ .../unneststring/UnnestStringOpDescSpec.scala | 109 ++++++++- .../unneststring/UnnestStringOpExecSpec.scala | 94 ++++++++ .../formly/delimiter/delimiter-presets.spec.ts | 243 +++++++++++++++++++++ .../common/formly/delimiter/delimiter-presets.ts | 139 ++++++++++++ .../formly/delimiter/delimiter.rendered.spec.ts | 209 ++++++++++++++++++ .../common/formly/delimiter/delimiter.type.html | 47 ++++ .../common/formly/delimiter/delimiter.type.scss | 17 +- .../common/formly/delimiter/delimiter.type.spec.ts | 227 +++++++++++++++++++ .../app/common/formly/delimiter/delimiter.type.ts | 135 ++++++++++++ .../src/app/common/formly/formly-config.spec.ts | 21 ++ frontend/src/app/common/formly/formly-config.ts | 28 +++ 17 files changed, 1433 insertions(+), 14 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/annotations/UIWidget.java b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/annotations/UIWidget.java index 04bc6a05e6..ad8e00f440 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/annotations/UIWidget.java +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/annotations/UIWidget.java @@ -23,6 +23,13 @@ public class UIWidget { public static final String UIWidgetTextArea = "{ \"widget\": {\n \"formlyConfig\": {\n \"type\": \"textarea\",\n \"templateOptions\": {\n \"autosize\": true,\n \"autosizeMinRows\": 3\n }\n }\n }\n }"; + // A delimiter picker: common delimiters by name (so a tab is chosen, not typed), plus a + // free-form entry for anything else. "char" is one literal character; "regex" is a pattern + // the picker checks as it is typed. + public static final String UIWidgetCharDelimiter = "{ \"maxLength\": 1, \"widget\": {\n \"formlyConfig\": {\n \"type\": \"delimiter\",\n \"props\": {\n \"delimiterMode\": \"char\"\n }\n }\n }\n }"; + + public static final String UIWidgetRegexDelimiter = "{ \"widget\": {\n \"formlyConfig\": {\n \"type\": \"delimiter\",\n \"props\": {\n \"delimiterMode\": \"regex\"\n }\n }\n }\n }"; + public static final String UIWidgetPassword = "{ \"widget\": {\n \"formlyConfig\": {\n \"templateOptions\": {\n \"type\": \"password\"\n }\n }\n }\n }"; } diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csv/CSVScanSourceOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csv/CSVScanSourceOpDesc.scala index 38a3d18736..1ebbd10040 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csv/CSVScanSourceOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csv/CSVScanSourceOpDesc.scala @@ -28,6 +28,7 @@ import org.apache.texera.amber.core.tuple.AttributeTypeUtils.inferSchemaFromRows import org.apache.texera.amber.core.tuple.{AttributeType, Schema} import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity} import org.apache.texera.amber.core.workflow.{PhysicalOp, SchemaPropagationFunc} +import org.apache.texera.amber.operator.metadata.annotations.UIWidget import org.apache.texera.amber.operator.source.scan.ScanSourceOpDesc import org.apache.texera.amber.operator.source.scan.csv.CSVScanSourceOpExec import org.apache.texera.amber.util.JSONUtils.objectMapper @@ -43,7 +44,7 @@ class CSVScanSourceOpDesc extends ScanSourceOpDesc { @JsonSchemaTitle("Delimiter") @JsonPropertyDescription("single character separating the fields on each line") @JsonInclude(JsonInclude.Include.NON_ABSENT) - @JsonSchemaInject(json = """{ "maxLength": 1 }""") + @JsonSchemaInject(json = UIWidget.UIWidgetCharDelimiter) var customDelimiter: Option[String] = None @JsonProperty(defaultValue = "true") diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csv/ParallelCSVScanSourceOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csv/ParallelCSVScanSourceOpDesc.scala index dda5cd3eec..4e6da0f5c4 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csv/ParallelCSVScanSourceOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csv/ParallelCSVScanSourceOpDesc.scala @@ -29,6 +29,7 @@ import org.apache.texera.amber.core.tuple.AttributeTypeUtils.inferSchemaFromRows import org.apache.texera.amber.core.tuple.{Attribute, AttributeType, Schema} import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity} import org.apache.texera.amber.core.workflow.{PhysicalOp, SchemaPropagationFunc} +import org.apache.texera.amber.operator.metadata.annotations.UIWidget import org.apache.texera.amber.operator.source.scan.ScanSourceOpDesc import org.apache.texera.amber.util.JSONUtils.objectMapper @@ -42,7 +43,7 @@ class ParallelCSVScanSourceOpDesc extends ScanSourceOpDesc { @JsonSchemaTitle("Delimiter") @JsonPropertyDescription("single character separating the fields on each line") @JsonDeserialize(contentAs = classOf[java.lang.String]) - @JsonSchemaInject(json = """{ "maxLength": 1 }""") + @JsonSchemaInject(json = UIWidget.UIWidgetCharDelimiter) var customDelimiter: Option[String] = None @JsonProperty(defaultValue = "true") diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csvOld/CSVOldScanSourceOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csvOld/CSVOldScanSourceOpDesc.scala index b8ec56e4df..5eb0b1303c 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csvOld/CSVOldScanSourceOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/csvOld/CSVOldScanSourceOpDesc.scala @@ -28,6 +28,7 @@ import org.apache.texera.amber.core.tuple.AttributeTypeUtils.inferSchemaFromRows import org.apache.texera.amber.core.tuple.{Attribute, AttributeType, Schema} import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity} import org.apache.texera.amber.core.workflow.{PhysicalOp, SchemaPropagationFunc} +import org.apache.texera.amber.operator.metadata.annotations.UIWidget import org.apache.texera.amber.operator.source.scan.ScanSourceOpDesc import org.apache.texera.amber.util.JSONUtils.objectMapper @@ -40,7 +41,7 @@ class CSVOldScanSourceOpDesc extends ScanSourceOpDesc { @JsonProperty(defaultValue = ",") @JsonSchemaTitle("Delimiter") @JsonPropertyDescription("single character separating the fields on each line") - @JsonSchemaInject(json = """{ "maxLength": 1 }""") + @JsonSchemaInject(json = UIWidget.UIWidgetCharDelimiter) var customDelimiter: Option[String] = Some(",") @JsonProperty(defaultValue = "true") diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpDesc.scala index fa2e5c2151..cb7f9f21b2 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpDesc.scala @@ -20,6 +20,7 @@ package org.apache.texera.amber.operator.unneststring import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription} +import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaInject import org.apache.texera.amber.core.executor.OpExecWithClassName import org.apache.texera.amber.core.tuple.{AttributeType, Schema} import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity} @@ -33,13 +34,20 @@ import org.apache.texera.amber.core.workflow.{ import org.apache.texera.amber.operator.{StandaloneCodeGenerator, StandaloneHelpers} import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.pyStringLiteral import org.apache.texera.amber.operator.flatmap.FlatMapOpDesc -import org.apache.texera.amber.operator.metadata.annotations.{AutofillAttributeName, SampleColumn} +import org.apache.texera.amber.operator.metadata.annotations.{ + AutofillAttributeName, + SampleColumn, + UIWidget +} import org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, OperatorInfo} import org.apache.texera.amber.util.JSONUtils.objectMapper +import java.util.regex.{Pattern, PatternSyntaxException} + class UnnestStringOpDesc extends FlatMapOpDesc with StandaloneCodeGenerator { @JsonProperty(value = "Delimiter", required = true, defaultValue = ",") - @JsonPropertyDescription("string that separates the data") + @JsonPropertyDescription("regular expression that separates the data") + @JsonSchemaInject(json = UIWidget.UIWidgetRegexDelimiter) var delimiter: String = _ @JsonProperty(value = "Attribute", required = true) @@ -80,6 +88,7 @@ class UnnestStringOpDesc extends FlatMapOpDesc with StandaloneCodeGenerator { .withOutputPorts(operatorInfo.outputPorts) .withPropagateSchema( SchemaPropagationFunc(inputSchemas => { + validateDelimiter() val outputSchema = Option(resultAttribute) .filter(_.trim.nonEmpty) .map(attr => inputSchemas.values.head.add(attr, AttributeType.STRING)) @@ -89,6 +98,41 @@ class UnnestStringOpDesc extends FlatMapOpDesc with StandaloneCodeGenerator { ) } + // Checked here, with Java's engine, so a bad pattern surfaces on the operator as a readable + // message while editing instead of as an exception or an empty result once the workflow + // runs. The picker checks the same things in the browser, but defers to this for syntax + // JavaScript can't parse or reads differently (inline flags, \p{..}). + private def validateDelimiter(): Unit = { + val source = Option(delimiter).getOrElse("") + if (source.isEmpty) { + throw new RuntimeException("Delimiter cannot be empty") + } + val pattern = + try Pattern.compile(source) + catch { + case e: PatternSyntaxException => + throw new RuntimeException( + s"Delimiter is not a valid regular expression: ${e.getDescription}" + + s" near index ${e.getIndex}" + ) + } + if (pattern.matcher("").find()) { + throw new RuntimeException( + "Delimiter matches an empty string, so it would split between every character" + ) + } + if (UnnestStringOpDesc.matchesEveryCharacter(pattern)) { + val literal = + if (source.length == 1) s""". To split on a literal "$source", use \\$source""" else "" + // A value's line breaks survive a pattern that leaves them out, so say what is left. + val whatIsLeft = + if (UnnestStringOpDesc.matchesLineBreaks(pattern)) + "matches every character, so nothing would be left" + else "matches every character except line breaks, so only line breaks would be left" + throw new RuntimeException(s"Delimiter $whatIsLeft$literal") + } + } + override def generateStandaloneCode(): String = generateStandaloneCode(Map.empty) override def generateStandaloneCode(inputSchemas: Map[PortIdentity, Schema]): String = { @@ -117,3 +161,27 @@ class UnnestStringOpDesc extends FlatMapOpDesc with StandaloneCodeGenerator { override def standaloneHelpers(): Seq[String] = Seq(StandaloneHelpers.AttributeCasts) } + +object UnnestStringOpDesc { + // Line terminators, which `.` leaves out. Mirrors LINE_BREAKS in the frontend's + // delimiter-presets.ts. + private val LineBreaks: Seq[Int] = Seq(0x0a, 0x0d, 0x85, 0x2028, 0x2029) + + private def matches(pattern: Pattern, code: Int): Boolean = + pattern.matcher(String.valueOf(code.toChar)).find() + + /** + * Whether the pattern matches every character of the Basic Multilingual Plane other than line + * breaks (and lone surrogate halves, which are not characters on their own), each tested on + * its own. Exact rather than a sample, and cheap: a pattern that leaves any character behind + * stops at the first one it misses. Mirrors matchesEveryCharacter in the frontend's + * delimiter-presets.ts. + */ + private def matchesEveryCharacter(pattern: Pattern): Boolean = + (0 to 0xffff).iterator + .filterNot(code => LineBreaks.contains(code) || code.toChar.isSurrogate) + .forall(matches(pattern, _)) + + private def matchesLineBreaks(pattern: Pattern): Boolean = + LineBreaks.forall(matches(pattern, _)) +} diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/scan/csv/CSVScanSourceOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/scan/csv/CSVScanSourceOpDescSpec.scala index fe426552a9..f5122381f2 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/scan/csv/CSVScanSourceOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/scan/csv/CSVScanSourceOpDescSpec.scala @@ -288,6 +288,17 @@ class CSVScanSourceOpDescSpec extends AnyFlatSpec with BeforeAndAfter { } } + it should "render the delimiter as the single-character delimiter picker" in { + delimiterOwners.foreach { + case (name, opDescClass) => + withClue(s"$name: ") { + val formlyConfig = delimiterSchema(opDescClass).path("widget").path("formlyConfig") + assert(formlyConfig.path("type").asText() == "delimiter") + assert(formlyConfig.path("props").path("delimiterMode").asText() == "char") + } + } + } + it should "validate an empty or one-character delimiter and refuse a longer one" in { delimiterOwners.foreach { case (name, opDescClass) => @@ -304,6 +315,85 @@ class CSVScanSourceOpDescSpec extends AnyFlatSpec with BeforeAndAfter { } } + // The presets the delimiter picker offers for a CSV scan, exactly as it stores them. + private val pickerPresets: List[(String, String)] = List( + "comma" -> ",", + "tab" -> "\t", + "semicolon" -> ";", + "pipe" -> "|", + "space" -> " " + ) + + private def writeCsv(delimiter: String, body: String): String = { + val tmpFile = Files.createTempFile("delimited-", ".csv") + tmpFile.toFile.deleteOnExit() + Files.write(tmpFile, body.replace(",", delimiter).getBytes(StandardCharsets.UTF_8)) + tmpFile.toString + } + + private def allReaders(delimiter: String): List[ScanSourceOpDesc] = { + val csv = new CSVScanSourceOpDesc() + csv.customDelimiter = Some(delimiter) + val parallelCsv = new ParallelCSVScanSourceOpDesc() + parallelCsv.customDelimiter = Some(delimiter) + val oldCsv = new CSVOldScanSourceOpDesc() + oldCsv.customDelimiter = Some(delimiter) + List(csv, parallelCsv, oldCsv) + } + + it should "read a file separated by each delimiter the picker offers" in { + pickerPresets.foreach { + case (name, delimiter) => + val path = writeCsv(delimiter, "id,name,age\n1,Alice,30\n2,Bob,25\n") + allReaders(delimiter).foreach { reader => + withClue(s"$name, ${reader.getClass.getSimpleName}: ") { + assert(columnNames(reader, path) == List("id", "name", "age")) + } + } + } + } + + it should "accept each preset against the schema's one-character bound" in { + delimiterOwners.foreach { + case (owner, opDescClass) => + pickerPresets.foreach { + case (name, delimiter) => + withClue(s"$owner, $name: ") { + assert(schemaValidates(delimiterSchema(opDescClass), delimiter)) + } + } + } + } + + it should "read a custom non-ASCII delimiter" in { + val path = writeCsv("§", "id,name,age\n1,Alice,30\n") + allReaders("§").foreach { reader => + withClue(s"${reader.getClass.getSimpleName}: ") { + assert(columnNames(reader, path) == List("id", "name", "age")) + } + } + } + + it should "keep a quoted field whole when it contains the delimiter" in { + val tmpFile = Files.createTempFile("quoted-", ".csv") + tmpFile.toFile.deleteOnExit() + Files.write(tmpFile, "id;name\n1;\"Smith; J\"\n".getBytes(StandardCharsets.UTF_8)) + allReaders(";").foreach { reader => + withClue(s"${reader.getClass.getSimpleName}: ") { + assert(columnNames(reader, tmpFile.toString) == List("id", "name")) + } + } + } + + it should "fall back to a comma when the delimiter is cleared" in { + val path = writeCsv(",", "id,name,age\n1,Alice,30\n") + allReaders("").foreach { reader => + withClue(s"${reader.getClass.getSimpleName}: ") { + assert(columnNames(reader, path) == List("id", "name", "age")) + } + } + } + it should "read a multi-character delimiter as its first character" in { // What the constraint gives up: a saved workflow holding a longer delimiter now // shows as invalid in the property editor. Its run is unchanged, which is what diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpDescSpec.scala index dc290e9947..74fde249a4 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpDescSpec.scala @@ -24,7 +24,7 @@ import org.apache.texera.amber.core.tuple.{Attribute, AttributeType, Schema} import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity} import org.apache.texera.amber.core.workflow.PortIdentity import org.apache.texera.amber.operator.LogicalOp -import org.apache.texera.amber.operator.metadata.OperatorGroupConstants +import org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, OperatorMetadataGenerator} import org.apache.texera.amber.util.JSONUtils.objectMapper import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -83,6 +83,113 @@ class UnnestStringOpDescSpec extends AnyFlatSpec with Matchers { } } + it should "report a delimiter that is not a valid regular expression" in { + val op = newDesc("(", "tags", "tag") + val physical = op.getPhysicalOp(workflowId, executionId) + val input = Schema().add(new Attribute("tags", AttributeType.STRING)) + val ex = intercept[RuntimeException] { + physical.propagateSchema.func(Map(op.operatorInfo.inputPorts.head.id -> input)) + } + ex.getMessage should startWith("Delimiter is not a valid regular expression") + } + + it should "name the problem for each common regex mistake" in { + val input = Schema().add(new Attribute("tags", AttributeType.STRING)) + // unclosed group, unclosed class, nothing to repeat, reversed range, trailing escape + List("(", "[", "*", "a{2,1}", "\\").foreach { bad => + withClue(s"pattern '$bad': ") { + val op = newDesc(bad, "tags", "tag") + val physical = op.getPhysicalOp(workflowId, executionId) + val ex = intercept[RuntimeException] { + physical.propagateSchema.func(Map(op.operatorInfo.inputPorts.head.id -> input)) + } + ex.getMessage should startWith("Delimiter is not a valid regular expression") + ex.getMessage should include("near index") + } + } + } + + private def compileError(delimiter: String): Option[String] = { + val op = newDesc(delimiter, "tags", "tag") + val input = Schema().add(new Attribute("tags", AttributeType.STRING)) + try { + op.getPhysicalOp(workflowId, executionId) + .propagateSchema + .func(Map(op.operatorInfo.inputPorts.head.id -> input)) + None + } catch { case e: RuntimeException => Some(e.getMessage) } + } + + // Checked with Java's engine, so patterns the browser can't parse (inline flags) or reads + // differently (\p{..}) are judged by what the operator actually runs. + it should "reject a pattern that matches the empty string, which splits every character apart" in { + List("|", "\\s*", "(?i)", "(?s).*", "\\z", "\\Q\\E").foreach { pattern => + withClue(s"pattern '$pattern': ") { + compileError(pattern).getOrElse("") should include("matches an empty string") + } + } + } + + it should "reject a pattern that matches every character, line breaks included" in { + List("(?s).", "[\\s\\S]").foreach { pattern => + withClue(s"pattern '$pattern': ") { + compileError(pattern).getOrElse("") should include( + "matches every character, so nothing would be left" + ) + } + } + } + + // Line breaks are what's left (see UnnestStringOpExecSpec), so the message says so rather + // than claiming nothing is. + it should "reject a pattern that matches every character but line breaks, naming what's left" in { + List(".", "[^\\n]", ".+").foreach { pattern => + withClue(s"pattern '$pattern': ") { + compileError(pattern).getOrElse("") should include( + "matches every character except line breaks, so only line breaks would be left" + ) + } + } + } + + it should "suggest the escape for a lone metacharacter that matches everything" in { + compileError(".").getOrElse("") should include("\\.") + } + + it should "reject an empty delimiter" in { + compileError("").getOrElse("") should include("cannot be empty") + } + + it should "accept useful patterns that use Java-only syntax" in { + List("\\p{L}+", "(?i)and", ",++", "\\h*,\\h*", "\\Q|\\E", "\\s+").foreach { pattern => + withClue(s"pattern '$pattern': ") { + compileError(pattern) shouldBe None + } + } + } + + it should "accept every preset the delimiter picker offers" in { + val input = Schema().add(new Attribute("tags", AttributeType.STRING)) + List(",", "\\t", "\t", ";", "\\|", "\\s+", "\\n").foreach { preset => + withClue(s"preset '$preset': ") { + val op = newDesc(preset, "tags", "tag") + val physical = op.getPhysicalOp(workflowId, executionId) + physical.propagateSchema.func(Map(op.operatorInfo.inputPorts.head.id -> input)) + } + } + } + + "UnnestStringOpDesc json schema" should "render the delimiter as the regex delimiter picker" in { + val formlyConfig = OperatorMetadataGenerator + .generateOperatorJsonSchema(classOf[UnnestStringOpDesc]) + .path("properties") + .path("Delimiter") + .path("widget") + .path("formlyConfig") + formlyConfig.path("type").asText() shouldBe "delimiter" + formlyConfig.path("props").path("delimiterMode").asText() shouldBe "regex" + } + // A hole widens an integer column to float, so the split would see "6.0" where // the engine splits "6". A real DOUBLE holding 6.0 looks the same and keeps its // point, so only the declared type can decide. diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpExecSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpExecSpec.scala index 146d9c780e..57a97e86e2 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpExecSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/unneststring/UnnestStringOpExecSpec.scala @@ -147,4 +147,98 @@ class UnnestStringOpExecSpec extends AnyFlatSpec with BeforeAndAfter { assertThrows[java.util.NoSuchElementException](processedTuple.next().getField("split")) opExec.close() } + + // Runs the operator over one value and returns the pieces it emits. + private def unnest(delimiter: String, value: String): List[String] = { + opDesc.attribute = "field1" + opDesc.delimiter = delimiter + val exec = new UnnestStringOpExec(objectMapper.writeValueAsString(opDesc)) + // Built directly, not propagated: schema propagation now rejects the degenerate patterns + // (`.`, `|`) whose runtime behavior the tests below pin. + val schema = tupleSchema.add(new Attribute("split", AttributeType.STRING)) + val input = Tuple + .builder(tupleSchema) + .add(new Attribute("field1", AttributeType.STRING), value) + .add(new Attribute("field2", AttributeType.INTEGER), 1) + .add(new Attribute("field3", AttributeType.STRING), "a") + .build() + exec.open() + val pieces = exec + .processTuple(input, 0) + .map(_.asInstanceOf[SchemaEnforceable].enforceSchema(schema).getField[String]("split")) + .toList + exec.close() + pieces + } + + // The presets the delimiter picker offers in regex mode, exactly as it stores them. + "UnnestStringOpExec with the picker's presets" should "split on a comma" in { + assert(unnest(",", "a,b,c") == List("a", "b", "c")) + } + + it should "split on a tab written as the regex escape" in { + assert(unnest("\\t", "a\tb\tc") == List("a", "b", "c")) + } + + it should "split on a tab stored as the literal character by an older workflow" in { + assert(unnest("\t", "a\tb\tc") == List("a", "b", "c")) + } + + it should "split on a semicolon" in { + assert(unnest(";", "a;b;c") == List("a", "b", "c")) + } + + it should "split on an escaped pipe" in { + assert(unnest("\\|", "a|b|c") == List("a", "b", "c")) + } + + it should "split on any run of whitespace" in { + assert(unnest("\\s+", "a b\t c\nd") == List("a", "b", "c", "d")) + } + + it should "split on a new line written as the regex escape" in { + assert(unnest("\\n", "a\nb\nc") == List("a", "b", "c")) + } + + "UnnestStringOpExec with a custom regex" should "split on a character class" in { + assert(unnest("[,;]", "a,b;c") == List("a", "b", "c")) + } + + it should "absorb the spaces around a comma" in { + assert(unnest("\\s*,\\s*", "a , b, c") == List("a", "b", "c")) + } + + it should "split on a multi-character delimiter" in { + assert(unnest("::", "a::b::c") == List("a", "b", "c")) + } + + it should "split on an escaped dot and keep the text between dots" in { + assert(unnest("\\.", "a.b.c") == List("a", "b", "c")) + } + + it should "drop everything for an unescaped dot, which matches every character" in { + // Why the operator rejects it at compile time: every piece is empty. + assert(unnest(".", "a.b.c") == List()) + } + + it should "leave only the line breaks of a multi-line value for an unescaped dot" in { + assert(unnest(".", "ab\ncd\r\ne") == List("\n", "\r\n")) + } + + it should "leave only the line breaks of a multi-line value for anything but a line feed" in { + assert(unnest("[^\\n]", "ab\ncd") == List("\n")) + } + + it should "split every character apart for a bare pipe, the empty alternation" in { + // The operator rejects this pattern at compile time; this pins what it would have done. + assert(unnest("|", "abc") == List("a", "b", "c")) + } + + it should "split on a non-ASCII delimiter" in { + assert(unnest("•", "a•b•c") == List("a", "b", "c")) + } + + it should "accept Java-only syntax such as a possessive quantifier" in { + assert(unnest(",++", "a,,b,c") == List("a", "b", "c")) + } } diff --git a/frontend/src/app/common/formly/delimiter/delimiter-presets.spec.ts b/frontend/src/app/common/formly/delimiter/delimiter-presets.spec.ts new file mode 100644 index 0000000000..8ee505ef91 --- /dev/null +++ b/frontend/src/app/common/formly/delimiter/delimiter-presets.spec.ts @@ -0,0 +1,243 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { delimiterError, delimiterPresets, matchDelimiterPreset, unescapeCharDelimiter } from "./delimiter-presets"; + +describe("char delimiter presets", () => { + // name shown -> the exact character a CSV scan's reader receives + const expected: [string, string][] = [ + ["Comma", ","], + ["Tab", "\t"], + ["Semicolon", ";"], + ["Pipe", "|"], + ["Space", " "], + ]; + + it("offers exactly the common CSV delimiters, in order", () => { + expect(delimiterPresets("char").map(p => p.value)).toEqual(expected.map(([, value]) => value)); + }); + + it.each(expected)("stores %s as the single character itself", (name, value) => { + const preset = matchDelimiterPreset(value, "char")!; + expect(preset.label).toContain(name); + expect(preset.value).toHaveLength(1); + }); + + it("shows the tab escape in its label, since the character itself is invisible", () => { + expect(matchDelimiterPreset("\t", "char")!.label).toContain("\\t"); + }); + + it.each([["#"], ["~"], ["§"], ["a"], [",;"], ["\\t"]])("treats %j as a custom value", value => { + expect(matchDelimiterPreset(value, "char")).toBeUndefined(); + }); + + it.each([ + ["\\t", "\t"], + ["\\\\", "\\"], + ])("reads the typed escape %j as the character it names", (typed, char) => { + expect(unescapeCharDelimiter(typed)).toBe(char); + }); + + it("leaves any other typed text as it is", () => { + expect(unescapeCharDelimiter("\\")).toBe("\\"); + expect(unescapeCharDelimiter("#")).toBe("#"); + expect(unescapeCharDelimiter("\\x")).toBe("\\x"); + }); + + it("never reads a char delimiter as a pattern", () => { + for (const value of ["(", "[", "*", "|", "\\", "."]) { + expect(delimiterError(value, "char")).toBeUndefined(); + } + }); +}); + +describe("regex delimiter presets", () => { + // name shown -> stored pattern -> a value it must split into a, b, c + const expected: [string, string, string][] = [ + ["Comma", ",", "a,b,c"], + ["Tab", "\\t", "a\tb\tc"], + ["Semicolon", ";", "a;b;c"], + ["Pipe", "\\|", "a|b|c"], + ["Whitespace", "\\s+", "a b\t c"], + ["New line", "\\n", "a\nb\nc"], + ]; + + it("offers exactly the common delimiters, in order", () => { + expect(delimiterPresets("regex").map(p => p.value)).toEqual(expected.map(([, value]) => value)); + }); + + it.each(expected)("stores %s as a pattern that splits on it", (name, pattern, sample) => { + const preset = matchDelimiterPreset(pattern, "regex")!; + expect(preset.label).toContain(name); + expect(delimiterError(pattern, "regex")).toBeUndefined(); + expect(sample.split(new RegExp(pattern))).toEqual(["a", "b", "c"]); + }); + + it.each([ + ["a literal tab", "\t", "\\t"], + ["a literal new line", "\n", "\\n"], + ])("names %s an older workflow stored as its preset", (_, stored, preset) => { + expect(matchDelimiterPreset(stored, "regex")!.value).toBe(preset); + }); + + it("does not show a bare pipe as the Pipe preset, since the two split differently", () => { + expect(matchDelimiterPreset("|", "regex")).toBeUndefined(); + }); + + it("treats a space as a custom pattern rather than as Whitespace", () => { + expect(matchDelimiterPreset(" ", "regex")).toBeUndefined(); + }); +}); + +describe("custom regex validation", () => { + it.each([ + ["a character class", "[,;]", "a,b;c"], + ["a comma with optional spaces", "\\s*,\\s*", "a , b,c"], + ["a multi-character delimiter", "::", "a::b::c"], + ["an escaped dot", "\\.", "a.b.c"], + ["an alternation", "(?:,|;)", "a,b;c"], + ["a run of digits", "\\d+", "a1b22c"], + ["a non-ASCII character", "•", "a•b•c"], + ["a space", " ", "a b c"], + ])("accepts %s", (_, pattern, sample) => { + expect(delimiterError(pattern, "regex")).toBeUndefined(); + expect(sample.split(new RegExp(pattern))).toEqual(["a", "b", "c"]); + }); + + it.each([ + ["an unclosed group", "("], + ["an unmatched closing parenthesis", ")"], + ["an unclosed character class", "["], + ["a quantifier with nothing to repeat", "*"], + ["a leading plus", "+"], + ["a reversed repetition range", "a{2,1}"], + ["a trailing backslash", "\\"], + ["an unfinished named group", "(?<"], + ["a reversed character range", "[z-a]"], + ])("rejects %s and says it is not a valid regular expression", (_, pattern) => { + expect(delimiterError(pattern, "regex")).toMatch(/Invalid regular expression/); + }); + + it.each([ + ["a bare pipe", "|"], + ["an optional run of whitespace", "\\s*"], + ["a starred character", "a*"], + ["an empty group", "(?:)"], + ["a start anchor", "^"], + ["an end anchor", "$"], + ["a match-anything pattern", ".*"], + ])("rejects %s, which matches an empty string and would split every character apart", (_, pattern) => { + expect(delimiterError(pattern, "regex")).toMatch(/matches an empty string/); + }); + + it("accepts a lookahead, which only matches before the delimiter and so keeps it on the next piece", () => { + expect(delimiterError("(?=,)", "regex")).toBeUndefined(); + expect("a,b".split(/(?=,)/)).toEqual(["a", ",b"]); + }); + + it("leaves an empty or unset value to the required check", () => { + expect(delimiterError("", "regex")).toBeUndefined(); + expect(delimiterError(null, "regex")).toBeUndefined(); + expect(delimiterError(undefined, "regex")).toBeUndefined(); + }); + + it("ignores a value that is not text", () => { + expect(delimiterError(44, "regex")).toBeUndefined(); + expect(matchDelimiterPreset(44, "regex")).toBeUndefined(); + }); + + // The browser's engine is not the operator's: Java accepts these, JavaScript throws on them. They are + // left to the backend, which compiles the pattern with Java (UnnestStringOpDescSpec) when the workflow + // compiles, rather than shown as a false error. + it.each([ + ["a possessive quantifier", ",++"], + ["a possessive star", "\\s*+,"], + ["an atomic group", "(?>,|;)"], + ["an inline case-insensitive flag", "(?i)and"], + ["an inline comments flag", "(?x) , "], + ])("leaves %s, which only Java accepts, to the operator", (_, pattern) => { + expect(delimiterError(pattern, "regex")).toBeUndefined(); + }); + + // JavaScript (without the `u` flag) parses these, but reads them as different patterns: `\\p{L}` as the + // literal text "p{L}", `\\h` as "h". Checking them in the browser gives wrong answers either way, so + // they are left to the operator, which checks them with Java's engine when the workflow compiles. + it.each([ + ["a negated Unicode class, which JavaScript misreads as matching every character", "[^\\p{L}]"], + ["a Unicode letter class", "\\p{L}+"], + ["a POSIX class", "[^\\p{Alnum}]+"], + ["a Java character class", "\\p{javaWhitespace}+"], + ["a quoted literal", "\\Q|\\E"], + ["horizontal whitespace", "\\h*,\\h*"], + ["an end-of-input anchor", "\\z"], + ["a start-of-input anchor", "\\A,"], + ["a code-point escape", "\\x{1F600}"], + ["a negated code-point escape, which JavaScript misreads as matching every character", "[^\\x{2C}]"], + ["an escape-character escape, which JavaScript misreads as the letter e", "[^\\e]"], + ["a bell-character escape", "\\a"], + ["a named character", "\\N{COMMA}"], + ["vertical whitespace", "\\v+"], + ["an octal escape", "\\011"], + ])("leaves %s to the operator", (_, pattern) => { + expect(delimiterError(pattern, "regex")).toBeUndefined(); + }); + + it("still checks an escaped backslash followed by p, which is not a Java class", () => { + expect(delimiterError("\\\\p|", "regex")).toMatch(/matches an empty string/); + }); + + it("still rejects a broken pattern that happens to contain Java-only syntax", () => { + expect(delimiterError("(?i)(", "regex")).toBeUndefined(); + expect(delimiterError("(", "regex")).toMatch(/Invalid regular expression/); + }); + + it.each([ + ["an unescaped dot", "."], + ["anything but a line feed", "[^\\n]"], + ["an unescaped dot, repeated", ".+"], + ])("rejects %s, which matches every character except line breaks, leaving only those", (_, pattern) => { + expect(delimiterError(pattern, "regex")).toMatch( + /matches every character except line breaks, so only line breaks would be left/ + ); + }); + + it("rejects a pattern matching every character, line breaks included, as leaving nothing", () => { + const error = delimiterError("[\\s\\S]", "regex"); + expect(error).toMatch(/matches every character, so nothing would be left/); + expect(error).not.toContain("line breaks"); + }); + + // Every character is tested, not a sample, so a pattern leaving any character behind is a working + // delimiter, however unusual. + it.each([ + ["a class of many common characters, which still leaves the rest", '[aZé中09 \\t,;:|/._\\-"]', "a~b"], + ["a class of exactly the old probe characters", "[a0-]", "Hello, a-0 x"], + ["word characters and hyphens", "[\\w-]", "Hello, a-0 x"], + ["a run of lowercase letters, digits and hyphens", "[a-z0-9-]+", "Hello, a-0 x"], + ["everything but a comma", "[^,]", "a,b"], + ["any non-space character", "\\S", "a b"], + ])("accepts %s, which leaves some text", (_, pattern, sample) => { + expect(delimiterError(pattern, "regex")).toBeUndefined(); + expect(sample.split(new RegExp(pattern)).some(piece => piece.length > 0)).toBe(true); + }); + + it("suggests the escape when a lone dot was probably meant literally", () => { + expect(delimiterError(".", "regex")).toContain("\\."); + }); +}); diff --git a/frontend/src/app/common/formly/delimiter/delimiter-presets.ts b/frontend/src/app/common/formly/delimiter/delimiter-presets.ts new file mode 100644 index 0000000000..01076efa5c --- /dev/null +++ b/frontend/src/app/common/formly/delimiter/delimiter-presets.ts @@ -0,0 +1,139 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * What a delimiter property holds: one literal character (the CSV scans, whose readers take a + * single Char) or a regular expression (Unnest String, which splits with `delimiter.r`). + */ +export type DelimiterMode = "char" | "regex"; + +export interface DelimiterPreset { + label: string; + value: string; + /** Other stored values that mean the same delimiter, so an older workflow still shows it by name. */ + aliases?: readonly string[]; +} + +/** The select option for "type your own". Only ever the select's value, never the stored one. */ +export const CUSTOM_DELIMITER = "__custom__"; + +const CHAR_PRESETS: readonly DelimiterPreset[] = [ + { label: "Comma ( , )", value: "," }, + { label: "Tab ( \\t )", value: "\t" }, + { label: "Semicolon ( ; )", value: ";" }, + { label: "Pipe ( | )", value: "|" }, + { label: "Space", value: " " }, +]; + +// Regex values are written as patterns: a bare `|` is an empty alternation that splits between +// every character, so the pipe preset escapes it. +const REGEX_PRESETS: readonly DelimiterPreset[] = [ + { label: "Comma ( , )", value: "," }, + { label: "Tab ( \\t )", value: "\\t", aliases: ["\t"] }, + { label: "Semicolon ( ; )", value: ";" }, + { label: "Pipe ( \\| )", value: "\\|" }, + { label: "Whitespace ( \\s+ )", value: "\\s+" }, + { label: "New line ( \\n )", value: "\\n", aliases: ["\n"] }, +]; + +export function delimiterPresets(mode: DelimiterMode): readonly DelimiterPreset[] { + return mode === "regex" ? REGEX_PRESETS : CHAR_PRESETS; +} + +/** The preset a stored value is, or undefined when it is a custom value (or empty). */ +export function matchDelimiterPreset(value: unknown, mode: DelimiterMode): DelimiterPreset | undefined { + if (typeof value !== "string" || value.length === 0) { + return undefined; + } + return delimiterPresets(mode).find(preset => preset.value === value || preset.aliases?.includes(value)); +} + +// Escapes someone may type into a single-character box, meaning the character they name. +const CHAR_ESCAPES: Readonly<Record<string, string>> = { "\\t": "\t", "\\\\": "\\" }; + +/** The character a typed escape such as `\t` names, or the typed text unchanged. */ +export function unescapeCharDelimiter(typed: string): string { + return CHAR_ESCAPES[typed] ?? typed; +} + +// Java syntax the browser can't judge. JavaScript throws on possessive quantifiers, atomic groups and +// inline flags, and (without the `u` flag) misreads Java's escapes: `\p{L}` becomes the literal text +// "p{L}", `\x{2C}` becomes "x{2C}", `\e`/`\a`/`\h` become letters, `\Q…\E` and the `\A`/`\z` anchors +// too. An escape only counts when an odd number of backslashes precedes it, so `\\p` (a literal +// backslash, then p) is still checked. +const JAVA_ONLY_SYNTAX = /[*+?}]\+|\(\?>|\(\?[a-zA-Z-]+[):]|(?<!\\)(?:\\\\)*\\(?:[pPxN]\{|[QhHvVzAZGRXea0])/; + +// Line terminators, which `.` leaves out. Mirrors UnnestStringOpDesc.LineBreaks. +const LINE_BREAKS = [0x0a, 0x0d, 0x85, 0x2028, 0x2029]; + +// Line breaks are tested separately; lone surrogate halves are not characters on their own. +function isLineBreakOrSurrogate(code: number): boolean { + return LINE_BREAKS.includes(code) || (code >= 0xd800 && code <= 0xdfff); +} + +/** + * Whether the pattern matches every character of the Basic Multilingual Plane other than line breaks, + * each tested on its own. This is exact rather than a sample, and cheap: a pattern that leaves any + * character behind stops at the first one it misses, and only a real match-everything pattern tests + * all ~63,000. Mirrored by UnnestStringOpDesc.matchesEveryCharacter. + */ +function matchesEveryCharacter(pattern: RegExp): boolean { + for (let code = 0; code <= 0xffff; code++) { + if (!isLineBreakOrSurrogate(code) && !pattern.test(String.fromCharCode(code))) { + return false; + } + } + return true; +} + +/** + * Why a delimiter cannot be used, or undefined when it can. Emptiness is left to `required` and to + * the operator's own default; a char delimiter's length is left to the schema's `maxLength`. + * + * The pattern is checked with the browser's regex engine, while the operator runs it with Java's. A + * pattern using Java-specific syntax (see JAVA_ONLY_SYNTAX) is not checked here at all, since the + * browser would judge a different pattern. The operator makes the same checks with Java semantics when + * the workflow compiles (UnnestStringOpDesc.validateDelimiter), so nothing is left unchecked. + */ +export function delimiterError(value: unknown, mode: DelimiterMode): string | undefined { + if (mode !== "regex" || typeof value !== "string" || value.length === 0) { + return undefined; + } + if (JAVA_ONLY_SYNTAX.test(value)) { + return undefined; + } + let pattern: RegExp; + try { + pattern = new RegExp(value); + } catch (e) { + return e instanceof Error ? e.message : "Invalid regular expression"; + } + if (pattern.test("")) { + return "This pattern matches an empty string, so it would split between every character"; + } + if (matchesEveryCharacter(pattern)) { + const literal = value.length === 1 ? ` To split on a literal "${value}", use \\${value}` : ""; + // A value's line breaks survive a pattern that leaves them out, so say what is left. + if (LINE_BREAKS.every(code => pattern.test(String.fromCharCode(code)))) { + return `This pattern matches every character, so nothing would be left.${literal}`; + } + return `This pattern matches every character except line breaks, so only line breaks would be left.${literal}`; + } + return undefined; +} diff --git a/frontend/src/app/common/formly/delimiter/delimiter.rendered.spec.ts b/frontend/src/app/common/formly/delimiter/delimiter.rendered.spec.ts new file mode 100644 index 0000000000..7e162c0418 --- /dev/null +++ b/frontend/src/app/common/formly/delimiter/delimiter.rendered.spec.ts @@ -0,0 +1,209 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Component } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormGroup, ReactiveFormsModule } from "@angular/forms"; +import { FormlyFieldConfig, FormlyModule } from "@ngx-formly/core"; +import { FormlyJsonschema } from "@ngx-formly/core/json-schema"; +import { FormlyNgZorroAntdModule } from "@ngx-formly/ng-zorro-antd"; +import { TEXERA_FORMLY_CONFIG } from "../formly-config"; +import { CUSTOM_DELIMITER } from "./delimiter-presets"; +import { DelimiterTypeComponent } from "./delimiter.type"; + +// The property schemas the backend generates (UIWidget.UIWidgetCharDelimiter / UIWidgetRegexDelimiter), +// so this renders the widget the way the operator panel does: schema -> formly-jsonschema -> Texera config. +const CSV_SCHEMA = { + type: "object", + properties: { + customDelimiter: { + type: "string", + title: "Delimiter", + default: ",", + maxLength: 1, + widget: { formlyConfig: { type: "delimiter", props: { delimiterMode: "char" } } }, + }, + }, +}; + +const UNNEST_SCHEMA = { + type: "object", + required: ["Delimiter"], + properties: { + Delimiter: { + type: "string", + default: ",", + widget: { formlyConfig: { type: "delimiter", props: { delimiterMode: "regex" } } }, + }, + }, +}; + +@Component({ + template: + "<form [formGroup]='form'><formly-form [form]='form' [fields]='fields' [model]='model'></formly-form></form>", + imports: [ReactiveFormsModule, FormlyModule], +}) +class HostComponent { + form = new FormGroup({}); + fields: FormlyFieldConfig[] = []; + model: Record<string, unknown> = {}; +} + +describe("delimiter widget rendered from the operator schema", () => { + let fixture: ComponentFixture<HostComponent>; + let host: HostComponent; + + async function render(schema: object, model: Record<string, unknown>): Promise<void> { + // No validation overrides: the panel only forces errors visible on a field with no validators, and + // formly-jsonschema gives every field one, so whether an error shows is up to the widget itself. + host.fields = [TestBed.inject(FormlyJsonschema).toFieldConfig(schema as never)]; + host.model = model; + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + } + + const widget = (): DelimiterTypeComponent => + fixture.debugElement.query(el => el.componentInstance instanceof DelimiterTypeComponent).componentInstance; + const customBox = (): HTMLInputElement | null => fixture.nativeElement.querySelector("input.delimiter-custom"); + const errorText = (): string => (fixture.nativeElement.querySelector("nz-form-control")?.textContent ?? "").trim(); + + async function typeCustom(text: string): Promise<void> { + widget().onSelect(CUSTOM_DELIMITER); + fixture.detectChanges(); + const box = customBox()!; + box.value = text; + box.dispatchEvent(new Event("input")); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + } + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [HostComponent, FormlyModule.forRoot(TEXERA_FORMLY_CONFIG), FormlyNgZorroAntdModule], + }).compileComponents(); + fixture = TestBed.createComponent(HostComponent); + host = fixture.componentInstance; + }); + + it("renders a CSV scan's delimiter as the picker, not a text box", async () => { + await render(CSV_SCHEMA, { customDelimiter: "\t" }); + + expect(widget().mode).toBe("char"); + expect(widget().selected).toBe("\t"); + expect(customBox()).toBeNull(); + }); + + it("writes a picked preset into the operator's properties", async () => { + await render(CSV_SCHEMA, { customDelimiter: "," }); + + widget().onSelect("|"); + fixture.detectChanges(); + + expect(host.form.value).toEqual({ customDelimiter: "|" }); + }); + + it("flags a stored multi-character CSV delimiter with the schema's length message", async () => { + await render(CSV_SCHEMA, { customDelimiter: ",;" }); + + expect(host.form.valid).toBe(false); + expect(errorText()).toContain("should NOT be longer than 1 characters"); + }); + + it("renders Unnest String's delimiter as the regex picker", async () => { + await render(UNNEST_SCHEMA, { Delimiter: "\\s+" }); + + expect(widget().mode).toBe("regex"); + expect(widget().selected).toBe("\\s+"); + }); + + it("shows why a custom regex does not compile, under the field", async () => { + await render(UNNEST_SCHEMA, { Delimiter: "," }); + + await typeCustom("[a-"); + + expect(host.form.valid).toBe(false); + expect(errorText()).toContain("Invalid regular expression"); + }); + + it("warns under the field that a bare pipe would split every character", async () => { + await render(UNNEST_SCHEMA, { Delimiter: "," }); + + await typeCustom("|"); + + expect(host.form.valid).toBe(false); + expect(errorText()).toContain("matches an empty string"); + }); + + it("warns under the field that an unescaped dot would leave nothing, and offers the escape", async () => { + await render(UNNEST_SCHEMA, { Delimiter: "," }); + + await typeCustom("."); + + expect(host.form.valid).toBe(false); + expect(errorText()).toContain("only line breaks would be left"); + expect(errorText()).toContain("\\."); + }); + + it("does not show a false error for Java-only syntax the operator accepts", async () => { + await render(UNNEST_SCHEMA, { Delimiter: "," }); + + await typeCustom(",++"); + + expect(host.form.valid).toBe(true); + }); + + it("lets \\t be typed despite the schema's one-character bound", async () => { + await render(CSV_SCHEMA, { customDelimiter: "," }); + await typeCustom("#"); + + expect(customBox()!.hasAttribute("maxlength")).toBe(false); + }); + + it("stores a typed \\t in a CSV scan as a real tab", async () => { + await render(CSV_SCHEMA, { customDelimiter: "," }); + + await typeCustom("\\t"); + + expect(host.form.value).toEqual({ customDelimiter: "\t" }); + expect(host.form.valid).toBe(true); + }); + + it("clears the error once the pattern is fixed", async () => { + await render(UNNEST_SCHEMA, { Delimiter: "," }); + await typeCustom("("); + expect(host.form.valid).toBe(false); + + await typeCustom("\\s*,\\s*"); + + expect(host.form.valid).toBe(true); + expect(host.form.value).toEqual({ Delimiter: "\\s*,\\s*" }); + expect(errorText()).not.toContain("Invalid"); + }); + + it("marks an emptied custom regex as required", async () => { + await render(UNNEST_SCHEMA, { Delimiter: "," }); + + await typeCustom(""); + + expect(host.form.valid).toBe(false); + expect(errorText()).toContain("This field is required"); + }); +}); diff --git a/frontend/src/app/common/formly/delimiter/delimiter.type.html b/frontend/src/app/common/formly/delimiter/delimiter.type.html new file mode 100644 index 0000000000..d20b8f74f7 --- /dev/null +++ b/frontend/src/app/common/formly/delimiter/delimiter.type.html @@ -0,0 +1,47 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +--> + +<div class="delimiter-input"> + <nz-select + class="delimiter-select" + nzPlaceHolder="Select a delimiter" + [attr.aria-label]="fieldLabel" + [ngModel]="selected" + (ngModelChange)="onSelect($event)" + [nzDisabled]="formControl.disabled"> + <nz-option + *ngFor="let preset of presets" + [nzValue]="preset.value" + [nzLabel]="preset.label"></nz-option> + <nz-option + [nzValue]="CUSTOM" + [nzLabel]="mode === 'regex' ? 'Custom regex…' : 'Custom…'"></nz-option> + </nz-select> + <input + *ngIf="selected === CUSTOM" + nz-input + class="delimiter-custom" + [value]="formControl.value ?? ''" + (input)="onCustomInput($event)" + [disabled]="formControl.disabled" + [id]="id" + (blur)="formControl.markAsTouched()" + [attr.aria-label]="'Custom ' + fieldLabel" + [placeholder]="customPlaceholder" /> +</div> diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/annotations/UIWidget.java b/frontend/src/app/common/formly/delimiter/delimiter.type.scss similarity index 57% copy from common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/annotations/UIWidget.java copy to frontend/src/app/common/formly/delimiter/delimiter.type.scss index 04bc6a05e6..c999abcce2 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/metadata/annotations/UIWidget.java +++ b/frontend/src/app/common/formly/delimiter/delimiter.type.scss @@ -1,4 +1,4 @@ -/* +/** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -17,12 +17,13 @@ * under the License. */ -package org.apache.texera.amber.operator.metadata.annotations; - -public class UIWidget { - - public static final String UIWidgetTextArea = "{ \"widget\": {\n \"formlyConfig\": {\n \"type\": \"textarea\",\n \"templateOptions\": {\n \"autosize\": true,\n \"autosizeMinRows\": 3\n }\n }\n }\n }"; - - public static final String UIWidgetPassword = "{ \"widget\": {\n \"formlyConfig\": {\n \"templateOptions\": {\n \"type\": \"password\"\n }\n }\n }\n }"; +.delimiter-input { + display: flex; + flex-direction: column; + gap: 6px; + width: 100%; +} +.delimiter-select { + width: 100%; } diff --git a/frontend/src/app/common/formly/delimiter/delimiter.type.spec.ts b/frontend/src/app/common/formly/delimiter/delimiter.type.spec.ts new file mode 100644 index 0000000000..24101dfabe --- /dev/null +++ b/frontend/src/app/common/formly/delimiter/delimiter.type.spec.ts @@ -0,0 +1,227 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormControl } from "@angular/forms"; +import { FieldTypeConfig } from "@ngx-formly/core"; +import { CUSTOM_DELIMITER, DelimiterMode, delimiterPresets } from "./delimiter-presets"; +import { DelimiterTypeComponent } from "./delimiter.type"; + +describe("DelimiterTypeComponent", () => { + let component: DelimiterTypeComponent; + let fixture: ComponentFixture<DelimiterTypeComponent>; + + function setField(initialValue: unknown, delimiterMode: DelimiterMode = "char"): FormControl { + const formControl = new FormControl(initialValue); + component.field = { formControl, props: { delimiterMode } } as unknown as FieldTypeConfig; + return formControl; + } + + const customBox = (): HTMLInputElement | null => fixture.nativeElement.querySelector("input.delimiter-custom"); + const shownSelection = (): string => + (fixture.nativeElement.querySelector(".ant-select-selection-item")?.textContent ?? "").trim(); + + function typeInto(box: HTMLInputElement, text: string): void { + box.value = text; + box.dispatchEvent(new Event("input")); + fixture.detectChanges(); + } + + beforeEach(async () => { + await TestBed.configureTestingModule({ imports: [DelimiterTypeComponent] }).compileComponents(); + fixture = TestBed.createComponent(DelimiterTypeComponent); + component = fixture.componentInstance; + }); + + describe.each(["char", "regex"] as DelimiterMode[])("in %s mode", mode => { + it.each(delimiterPresets(mode).map(p => [p.label, p.value]))( + "stores %s as its value when picked, with no text box", + async (label, value) => { + const formControl = setField(",", mode); + fixture.detectChanges(); + + component.onSelect(value); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + + expect(formControl.value).toBe(value); + expect(formControl.dirty).toBe(true); + expect(formControl.touched).toBe(true); + expect(component.selected).toBe(value); + expect(shownSelection()).toBe(label); + expect(customBox()).toBeNull(); + } + ); + + it("opens an empty text box when Custom is picked from a preset", () => { + const formControl = setField(",", mode); + + component.onSelect(CUSTOM_DELIMITER); + fixture.detectChanges(); + + expect(formControl.value).toBe(""); + expect(component.selected).toBe(CUSTOM_DELIMITER); + expect(customBox()).not.toBeNull(); + }); + + it("stays on Custom while the typed value happens to equal a preset", () => { + const formControl = setField(",", mode); + component.onSelect(CUSTOM_DELIMITER); + fixture.detectChanges(); + + typeInto(customBox()!, ";"); + + expect(formControl.value).toBe(";"); + expect(component.selected).toBe(CUSTOM_DELIMITER); + expect(customBox()).not.toBeNull(); + }); + + it("stays on Custom after the custom value is cleared", () => { + const formControl = setField(",", mode); + component.onSelect(CUSTOM_DELIMITER); + + formControl.setValue(""); + + expect(component.selected).toBe(CUSTOM_DELIMITER); + }); + + it("closes the text box when a preset is picked after Custom", () => { + const formControl = setField(",", mode); + component.onSelect(CUSTOM_DELIMITER); + fixture.detectChanges(); + + component.onSelect(";"); + fixture.detectChanges(); + + expect(formControl.value).toBe(";"); + expect(customBox()).toBeNull(); + }); + + it("selects nothing while the property is unset", () => { + setField(null, mode); + expect(component.selected).toBeNull(); + setField("", mode); + expect(component.selected).toBeNull(); + }); + + it("disables the picker with the form, e.g. on a read-only workflow", () => { + const formControl = setField(",", mode); + formControl.disable(); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector("nz-select").classList).toContain("ant-select-disabled"); + }); + }); + + it("shows a custom char delimiter with a single-character placeholder", () => { + setField("#"); + fixture.detectChanges(); + + expect(customBox()!.value).toBe("#"); + expect(customBox()!.placeholder).toBe("Any single character"); + }); + + it("turns a typed \\t into a tab and shows it as the Tab preset", () => { + const formControl = setField(","); + component.onSelect(CUSTOM_DELIMITER); + fixture.detectChanges(); + + typeInto(customBox()!, "\\t"); + + expect(formControl.value).toBe("\t"); + expect(component.selected).toBe("\t"); + expect(customBox()).toBeNull(); + }); + + it("lets a backslash be typed on its way to an escape, rather than capping it at one character", () => { + const formControl = setField(","); + component.onSelect(CUSTOM_DELIMITER); + fixture.detectChanges(); + + typeInto(customBox()!, "\\"); + expect(formControl.value).toBe("\\"); + expect(customBox()!.hasAttribute("maxlength")).toBe(false); + }); + + it("follows the value back to its preset when it changes from outside, e.g. undo or a co-editor", () => { + const formControl = setField(","); + fixture.detectChanges(); + component.onSelect(CUSTOM_DELIMITER); + fixture.detectChanges(); + typeInto(customBox()!, ";"); + + formControl.setValue(","); + fixture.detectChanges(); + formControl.setValue(";"); + fixture.detectChanges(); + + expect(component.selected).toBe(";"); + expect(customBox()).toBeNull(); + }); + + it("labels the picker and the text box for screen readers", () => { + component.field = { + formControl: new FormControl("#"), + props: { delimiterMode: "char", label: "Delimiter" }, + } as unknown as FieldTypeConfig; + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector("nz-select").getAttribute("aria-label")).toBe("Delimiter"); + expect(customBox()!.getAttribute("aria-label")).toBe("Custom Delimiter"); + }); + + it("lets a custom regex be any length, with an example in the placeholder", () => { + setField("[,;]\\s*", "regex"); + fixture.detectChanges(); + + expect(component.selected).toBe(CUSTOM_DELIMITER); + expect(customBox()!.value).toBe("[,;]\\s*"); + expect(customBox()!.hasAttribute("maxlength")).toBe(false); + expect(customBox()!.placeholder).toContain("Regular expression"); + }); + + it("keeps a custom value when Custom is picked again", () => { + const formControl = setField("#"); + + component.onSelect(CUSTOM_DELIMITER); + + expect(formControl.value).toBe("#"); + }); + + it("shows a stored tab as the Tab preset in both modes", () => { + setField("\t", "char"); + expect(component.selected).toBe("\t"); + setField("\t", "regex"); + expect(component.selected).toBe("\\t"); + }); + + it("opens an older workflow's over-long char delimiter on Custom so it can be fixed", () => { + setField(",;"); + fixture.detectChanges(); + + expect(component.selected).toBe(CUSTOM_DELIMITER); + expect(customBox()!.value).toBe(",;"); + }); + + it("falls back to char mode when the schema names no mode", () => { + component.field = { formControl: new FormControl(","), props: {} } as unknown as FieldTypeConfig; + expect(component.mode).toBe("char"); + }); +}); diff --git a/frontend/src/app/common/formly/delimiter/delimiter.type.ts b/frontend/src/app/common/formly/delimiter/delimiter.type.ts new file mode 100644 index 0000000000..1a0898faac --- /dev/null +++ b/frontend/src/app/common/formly/delimiter/delimiter.type.ts @@ -0,0 +1,135 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Component, OnInit } from "@angular/core"; +import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; +import { NgFor, NgIf } from "@angular/common"; +import { FormsModule } from "@angular/forms"; +import { FieldType, FieldTypeConfig } from "@ngx-formly/core"; +import { NzSelectModule } from "ng-zorro-antd/select"; +import { NzInputDirective } from "ng-zorro-antd/input"; +import { + CUSTOM_DELIMITER, + DelimiterMode, + DelimiterPreset, + delimiterPresets, + matchDelimiterPreset, + unescapeCharDelimiter, +} from "./delimiter-presets"; + +/** + * Delimiter picker: common delimiters are chosen by name, so nobody has to wonder whether a tab is + * typed as a tab key press or as `\t`; anything else is typed into a box that opens under "Custom". + * The stored value is unchanged from the plain text field this replaces -- the delimiter itself -- + * so existing workflows load as they were. `props.delimiterMode` ("char" or "regex") picks the + * presets and how a custom value is checked (see delimiterError, registered with this type). + * + * The Custom box is bound by hand rather than through formlyAttributes, which would copy the schema's + * `maxLength: 1` onto it as a `maxlength` attribute and block the second key of a typed `\t`. The + * length is still enforced by the schema's validator. + */ +@UntilDestroy() +@Component({ + selector: "texera-delimiter-type", + templateUrl: "./delimiter.type.html", + styleUrls: ["./delimiter.type.scss"], + imports: [NgFor, NgIf, FormsModule, NzSelectModule, NzInputDirective], +}) +export class DelimiterTypeComponent extends FieldType<FieldTypeConfig> implements OnInit { + readonly CUSTOM = CUSTOM_DELIMITER; + + // The value last written from the Custom box, so typing a value that happens to equal a preset does + // not snap the select back to that preset mid-edit. A value that differs came from elsewhere (undo, a + // co-editor, the agent), and the select follows it. Undefined while Custom is not in use. + private customValue: unknown = undefined; + + ngOnInit(): void { + // Any write this widget did not make itself ends the Custom edit, even when it lands back on the + // custom value (undo, then redo). + this.formControl.valueChanges.pipe(untilDestroyed(this)).subscribe(value => { + if (!this.isCustomValue(value)) { + this.customValue = undefined; + } + }); + } + + // Formly stores a cleared optional field as undefined rather than "", so every empty value is the + // same empty custom value. + private isCustomValue(value: unknown): boolean { + if (this.customValue === undefined) { + return false; + } + const isEmpty = (v: unknown) => v === undefined || v === null || v === ""; + return value === this.customValue || (isEmpty(value) && isEmpty(this.customValue)); + } + + get mode(): DelimiterMode { + return (this.props as { delimiterMode?: DelimiterMode } | undefined)?.delimiterMode === "regex" ? "regex" : "char"; + } + + get fieldLabel(): string { + return (this.props?.label as string | undefined) ?? "Delimiter"; + } + + get presets(): readonly DelimiterPreset[] { + return delimiterPresets(this.mode); + } + + /** The select's value: the matching preset, "Custom", or nothing while the property is unset. */ + get selected(): string | null { + const value = this.formControl?.value; + const inCustom = this.isCustomValue(value); + const preset = matchDelimiterPreset(value, this.mode); + if (!inCustom && preset) { + return preset.value; + } + if (inCustom || (typeof value === "string" && value.length > 0)) { + return CUSTOM_DELIMITER; + } + return null; + } + + get customPlaceholder(): string { + return this.mode === "regex" ? "Regular expression, e.g. [,;]\\s*" : "Any single character"; + } + + onSelect(choice: string): void { + if (choice === CUSTOM_DELIMITER) { + // Start from what is there when it is already custom; a preset's value would only be in the way. + if (matchDelimiterPreset(this.formControl.value, this.mode)) { + this.formControl.setValue(""); + } + this.customValue = this.formControl.value ?? ""; + } else { + this.customValue = undefined; + this.formControl.setValue(choice); + } + this.formControl.markAsDirty(); + this.formControl.markAsTouched(); + } + + onCustomInput(event: Event): void { + const typed = (event.target as HTMLInputElement).value; + const value = this.mode === "char" ? unescapeCharDelimiter(typed) : typed; + // A typed escape that names a preset (`\t`) is that preset, so the select shows it by name. + this.customValue = value !== typed && matchDelimiterPreset(value, this.mode) ? undefined : value; + this.formControl.setValue(value); + this.formControl.markAsDirty(); + } +} diff --git a/frontend/src/app/common/formly/formly-config.spec.ts b/frontend/src/app/common/formly/formly-config.spec.ts index 57e8932dca..1c7c1e4a31 100644 --- a/frontend/src/app/common/formly/formly-config.spec.ts +++ b/frontend/src/app/common/formly/formly-config.spec.ts @@ -17,9 +17,11 @@ * under the License. */ +import { FormControl } from "@angular/forms"; import { FormlyFieldConfig } from "@ngx-formly/core"; import { constValidationMessage, + delimiterValidationMessage, exclusiveMaximumValidationMessage, exclusiveMinimumValidationMessage, maxItemsValidationMessage, @@ -29,6 +31,7 @@ import { minlengthValidationMessage, minValidationMessage, multipleOfValidationMessage, + TEXERA_FORMLY_CONFIG, } from "./formly-config"; // the `err` argument is unused by every message builder, so any value is fine @@ -81,3 +84,21 @@ describe("formly validation messages", () => { expect(constValidationMessage(err, field({}))).toBe('should be equal to constant "undefined"'); }); }); + +describe("delimiter type", () => { + const delimiterType = TEXERA_FORMLY_CONFIG.types.find(type => type.name === "delimiter")!; + const validator = (delimiterType.defaultOptions as any).validators.delimiter; + const delimiterField = (value: string, delimiterMode: string): FormlyFieldConfig => + ({ props: { delimiterMode }, formControl: new FormControl(value) }) as unknown as FormlyFieldConfig; + + it("rejects a regex delimiter that does not compile, and says why", () => { + const f = delimiterField("(", "regex"); + expect(validator.expression(f.formControl, f)).toBe(false); + expect(delimiterValidationMessage(err, f)).toMatch(/Invalid regular expression/); + }); + + it("accepts the same text as a single-character delimiter", () => { + const f = delimiterField("(", "char"); + expect(validator.expression(f.formControl, f)).toBe(true); + }); +}); diff --git a/frontend/src/app/common/formly/formly-config.ts b/frontend/src/app/common/formly/formly-config.ts index 3d56e1f07c..d4aedb300b 100644 --- a/frontend/src/app/common/formly/formly-config.ts +++ b/frontend/src/app/common/formly/formly-config.ts @@ -22,6 +22,9 @@ import { ArrayTypeComponent } from "./array.type"; import { ObjectTypeComponent } from "./object.type"; import { MultiSchemaTypeComponent } from "./multischema.type"; import { FormlyFieldConfig } from "@ngx-formly/core"; +import { AbstractControl } from "@angular/forms"; +import { DelimiterTypeComponent } from "./delimiter/delimiter.type"; +import { DelimiterMode, delimiterError } from "./delimiter/delimiter-presets"; import { CodeareaCustomTemplateComponent } from "../../workspace/component/codearea-custom-template/codearea-custom-template.component"; import { PresetWrapperComponent } from "./preset-wrapper/preset-wrapper.component"; import { DatasetFileSelectorComponent } from "../../workspace/component/dataset-file-selector/dataset-file-selector.component"; @@ -94,6 +97,23 @@ export const TEXERA_FORMLY_CONFIG = { { name: "ui-udf-parameters", component: UiUdfParametersComponent, wrappers: ["form-field"] }, { name: "constrainedvalue", component: ConstrainedValueComponent, wrappers: ["form-field"] }, { name: "resourcevalue", component: ResourceValueSelectorComponent }, + { + name: "delimiter", + component: DelimiterTypeComponent, + wrappers: ["form-field"], + defaultOptions: { + // Shown without waiting for a touch: a delimiter loaded from a saved workflow can already be + // invalid (an old multi-character one), and nobody touches a field they do not know is wrong. + validation: { show: true }, + validators: { + delimiter: { + expression: (control: AbstractControl, field: FormlyFieldConfig) => + delimiterError(control.value, delimiterModeOf(field)) === undefined, + message: delimiterValidationMessage, + }, + }, + }, + }, ], wrappers: [ { name: "preset-wrapper", component: PresetWrapperComponent }, @@ -139,6 +159,14 @@ export function exclusiveMaximumValidationMessage(err: any, field: FormlyFieldCo return `should be < ${field.props?.exclusiveMaximum}`; } +function delimiterModeOf(field: FormlyFieldConfig): DelimiterMode { + return field.props?.["delimiterMode"] === "regex" ? "regex" : "char"; +} + +export function delimiterValidationMessage(err: any, field: FormlyFieldConfig) { + return delimiterError(field.formControl?.value, delimiterModeOf(field)) ?? ""; +} + export function constValidationMessage(err: any, field: FormlyFieldConfig) { return `should be equal to constant "${field.props?.const}"`; }
