From 1c34951e1e97d27fd1dd3be480c9d925fe08624d Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Mon, 7 Feb 2022 17:24:31 +0100 Subject: [PATCH 1/3] Refactor `FixStandaloneSetup` to use original Fix file path if available. --- .../main/java/org/metafacture/metafix/FixMethod.java | 11 ++--------- .../org/metafacture/metafix/FixStandaloneSetup.java | 10 +++++----- .../main/java/org/metafacture/metafix/Metafix.java | 11 +++++------ 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/metafix/src/main/java/org/metafacture/metafix/FixMethod.java b/metafix/src/main/java/org/metafacture/metafix/FixMethod.java index 2671f8ff..04ad0c91 100644 --- a/metafix/src/main/java/org/metafacture/metafix/FixMethod.java +++ b/metafix/src/main/java/org/metafacture/metafix/FixMethod.java @@ -22,7 +22,6 @@ import org.metafacture.metamorph.api.Maps; import org.metafacture.metamorph.maps.FileMap; -import java.io.FileNotFoundException; import java.nio.file.Paths; import java.util.Arrays; import java.util.Collections; @@ -68,14 +67,8 @@ public void apply(final Metafix metafix, final Record record, final List } final RecordTransformer recordTransformer = metafix.getRecordTransformer(); - recordTransformer.setRecord(recordTransformer.transformRecord(INCLUDE_FIX.computeIfAbsent(includePath, k -> { - try { - return FixStandaloneSetup.parseFix(Metafix.fixReader(k)); - } - catch (final FileNotFoundException e) { - throw new MetafactureException(e); - } - }))); + recordTransformer.setRecord(recordTransformer.transformRecord( + INCLUDE_FIX.computeIfAbsent(includePath, FixStandaloneSetup::parseFix))); } }, nothing { diff --git a/metafix/src/main/java/org/metafacture/metafix/FixStandaloneSetup.java b/metafix/src/main/java/org/metafacture/metafix/FixStandaloneSetup.java index 46df72ce..4ff0bf3a 100644 --- a/metafix/src/main/java/org/metafacture/metafix/FixStandaloneSetup.java +++ b/metafix/src/main/java/org/metafacture/metafix/FixStandaloneSetup.java @@ -27,17 +27,17 @@ public static void main(final String[] args) { throw new IllegalArgumentException(String.format("Usage: %s ", FixStandaloneSetup.class.getName())); } - public static Fix parseFix(final Reader fixDef) { - final String path; + public static Fix parseFix(final String path) { + return (Fix) XtextValidator.getValidatedResource(path, new FixStandaloneSetup()).getContents().get(0); + } + public static Fix parseFix(final Reader fixDef) { try { - path = absPathToTempFile(fixDef, ".fix"); + return parseFix(absPathToTempFile(fixDef, ".fix")); } catch (final IOException e) { throw new MetafactureException(e); } - - return (Fix) XtextValidator.getValidatedResource(path, new FixStandaloneSetup()).getContents().get(0); } public static String absPathToTempFile(final Reader fixDef, final String suffix) throws IOException { diff --git a/metafix/src/main/java/org/metafacture/metafix/Metafix.java b/metafix/src/main/java/org/metafacture/metafix/Metafix.java index 28de5b3d..0311af7f 100644 --- a/metafix/src/main/java/org/metafacture/metafix/Metafix.java +++ b/metafix/src/main/java/org/metafacture/metafix/Metafix.java @@ -33,7 +33,6 @@ import java.io.Closeable; import java.io.FileNotFoundException; -import java.io.FileReader; import java.io.IOException; import java.io.Reader; import java.io.StringReader; @@ -103,10 +102,14 @@ public Metafix(final String fixDef) throws FileNotFoundException { } public Metafix(final String fixDef, final Map vars) throws FileNotFoundException { - this(fixReader(fixDef), vars); + this(vars); if (isFixFile(fixDef)) { fixFile = fixDef; + fix = FixStandaloneSetup.parseFix(fixDef); + } + else { + fix = FixStandaloneSetup.parseFix(new StringReader(fixDef)); } } @@ -119,10 +122,6 @@ public Metafix(final Reader fixDef, final Map vars) { fix = FixStandaloneSetup.parseFix(fixDef); } - /*package-private*/ static Reader fixReader(final String fixDef) throws FileNotFoundException { - return isFixFile(fixDef) ? new FileReader(fixDef) : new StringReader(fixDef); - } - /*package-private*/ static boolean isFixFile(final String fixDef) { return fixDef.endsWith(FIX_EXTENSION); } From a9d3bb991b8a88b65002d5c5ad78508d0430ab77 Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Mon, 7 Feb 2022 17:06:45 +0100 Subject: [PATCH 2/3] Throw exception for invalid Fix files. (#79) --- .../metafix/FixParseException.java | 31 +++++++++++++++++++ .../metafix/validation/XtextValidator.java | 17 ++++++++-- .../org/metafacture/metafix/MetafixTest.java | 9 ++++++ .../org/metafacture/metafix/fixes/invalid.fix | 3 ++ 4 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 metafix/src/main/java/org/metafacture/metafix/FixParseException.java create mode 100644 metafix/src/test/resources/org/metafacture/metafix/fixes/invalid.fix diff --git a/metafix/src/main/java/org/metafacture/metafix/FixParseException.java b/metafix/src/main/java/org/metafacture/metafix/FixParseException.java new file mode 100644 index 00000000..366a58ff --- /dev/null +++ b/metafix/src/main/java/org/metafacture/metafix/FixParseException.java @@ -0,0 +1,31 @@ +/* + * Copyright 2022 hbz NRW + * + * Licensed 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. + */ + +package org.metafacture.metafix; + +import org.metafacture.framework.MetafactureException; + +public class FixParseException extends MetafactureException { + + public FixParseException(final String message) { + super(message); + } + + public FixParseException(final String message, final Throwable cause) { + super(message, cause); + } + +} diff --git a/metafix/src/main/java/org/metafacture/metafix/validation/XtextValidator.java b/metafix/src/main/java/org/metafacture/metafix/validation/XtextValidator.java index a8de336b..5427598e 100644 --- a/metafix/src/main/java/org/metafacture/metafix/validation/XtextValidator.java +++ b/metafix/src/main/java/org/metafacture/metafix/validation/XtextValidator.java @@ -1,5 +1,7 @@ package org.metafacture.metafix.validation; +import org.metafacture.metafix.FixParseException; + import org.eclipse.emf.common.util.URI; import org.eclipse.xtext.ISetup; import org.eclipse.xtext.XtextStandaloneSetup; @@ -31,7 +33,7 @@ private static boolean validate(final XtextResource resource, final ISetup setup if (count > 0) { LOG.warn("The {} file '{}' has {} issue{}:", - setup.getClass().getSimpleName(), resource.getURI().toFileString(), count, count > 1 ? "s" : ""); + resourceType(setup), resource.getURI().toFileString(), count, count > 1 ? "s" : ""); issues.forEach(i -> LOG.warn("- {}: {} ({}:{})", i.getSeverity(), i.getMessage(), i.getLineNumber(), i.getColumn())); @@ -63,8 +65,17 @@ private static XtextResource getResource(final String path, final ISetup setup) public static XtextResource getValidatedResource(final String path, final ISetup setup) { final XtextResource resource = getResource(path, setup); - validate(resource, setup); - return resource; + + if (validate(resource, setup)) { + return resource; + } + else { + throw new FixParseException("Invalid " + resourceType(setup) + " resource: " + path); + } + } + + private static String resourceType(final ISetup setup) { + return setup.getClass().getSimpleName(); } public static void main(final String[] args) { diff --git a/metafix/src/test/java/org/metafacture/metafix/MetafixTest.java b/metafix/src/test/java/org/metafacture/metafix/MetafixTest.java index 9a0fdda5..5ddbe182 100644 --- a/metafix/src/test/java/org/metafacture/metafix/MetafixTest.java +++ b/metafix/src/test/java/org/metafacture/metafix/MetafixTest.java @@ -157,4 +157,13 @@ public void shouldGetDefaultValueForUnknownKey() { Assertions.assertEquals(VALUE, metafix.getValue(MAP_NAME, KEY)); } + @Test + // See https://github.com/metafacture/metafacture-fix/issues/79 + public void shouldThrowExceptionForInvalidFixFile() { + final String fixFile = "src/test/resources/org/metafacture/metafix/fixes/invalid.fix"; + MetafixTestHelpers.assertThrows(FixParseException.class, "Invalid FixStandaloneSetup resource: " + fixFile, () -> new Metafix(fixFile)); + + // TODO: Test logging statements + } + } diff --git a/metafix/src/test/resources/org/metafacture/metafix/fixes/invalid.fix b/metafix/src/test/resources/org/metafacture/metafix/fixes/invalid.fix new file mode 100644 index 00000000..0214cfb4 --- /dev/null +++ b/metafix/src/test/resources/org/metafacture/metafix/fixes/invalid.fix @@ -0,0 +1,3 @@ +# -----------license---------- +copy_field("metadata.mods:mods.mods:accessCondition.xlink:href","license.id") +lookup("license.id", ,"data/maps/duepublico-licenses.tsv","sep_char":"\t") From 29f416ef6435674761ccb0f7bff9447c9ee75487 Mon Sep 17 00:00:00 2001 From: Jens Wille Date: Tue, 8 Feb 2022 09:09:35 +0100 Subject: [PATCH 3/3] Apply suggestions from code review (#134) Co-authored-by: Fabian Steeg Co-authored-by: Fabian Steeg --- .../test/resources/org/metafacture/metafix/fixes/invalid.fix | 2 -- 1 file changed, 2 deletions(-) diff --git a/metafix/src/test/resources/org/metafacture/metafix/fixes/invalid.fix b/metafix/src/test/resources/org/metafacture/metafix/fixes/invalid.fix index 0214cfb4..75db1144 100644 --- a/metafix/src/test/resources/org/metafacture/metafix/fixes/invalid.fix +++ b/metafix/src/test/resources/org/metafacture/metafix/fixes/invalid.fix @@ -1,3 +1 @@ -# -----------license---------- -copy_field("metadata.mods:mods.mods:accessCondition.xlink:href","license.id") lookup("license.id", ,"data/maps/duepublico-licenses.tsv","sep_char":"\t")