From 0fc46c86cbee8189d0709856f50378fbd52b1af2 Mon Sep 17 00:00:00 2001 From: JanderJ1 Date: Wed, 25 Jan 2023 19:09:54 +0100 Subject: [PATCH 1/3] Work on foreign keys --- pom.xml | 4 +- .../frictionlessdata/datapackage/Package.java | 5 +- .../resource/AbstractResource.java | 48 +++++++++-- .../datapackage/resource/Resource.java | 15 +++- .../datapackage/ForeignKeysTest.java | 55 +++++++++++-- .../datapackage/TestUtil.java | 8 +- .../foreign-keys-extended-invalid1.json | 76 ++++++++++++++++++ .../foreign-keys-extended-invalid2.json | 76 ++++++++++++++++++ .../datapackages/foreign-keys-extended.json | 80 +++++++++++++++++++ .../datapackages/foreign-keys-invalid.json | 72 +++++++++++++++++ .../fixtures/datapackages/foreign-keys.json | 4 + 11 files changed, 422 insertions(+), 21 deletions(-) create mode 100644 src/test/resources/fixtures/datapackages/foreign-keys-extended-invalid1.json create mode 100644 src/test/resources/fixtures/datapackages/foreign-keys-extended-invalid2.json create mode 100644 src/test/resources/fixtures/datapackages/foreign-keys-extended.json create mode 100644 src/test/resources/fixtures/datapackages/foreign-keys-invalid.json diff --git a/pom.xml b/pom.xml index 411e873..8b5bbb6 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ 4.0.0 io.frictionlessdata datapackage-java - 0.6.11-SNAPSHOT + 0.6.12-SNAPSHOT jar https://github.com/frictionlessdata/datapackage-java/issues @@ -22,7 +22,7 @@ 8 ${java.version} ${java.version} - 0.6.11 + 0.6.12 1.3 5.9.1 2.0.5 diff --git a/src/main/java/io/frictionlessdata/datapackage/Package.java b/src/main/java/io/frictionlessdata/datapackage/Package.java index ee33d8d..3c5f970 100644 --- a/src/main/java/io/frictionlessdata/datapackage/Package.java +++ b/src/main/java/io/frictionlessdata/datapackage/Package.java @@ -535,7 +535,10 @@ public void write (File outputDir, boolean zipCompressed) throws Exception { for (Resource r : resourceList) { r.writeData(outFs.getPath(parentDirName )); - r.writeSchema(outFs.getPath(parentDirName)); + + if (null != r.getSchema()) { + r.writeSchema(outFs.getPath(parentDirName)); + } // write out dialect file only if not null or URL String dialectP = r.getPathForWritingDialect(); diff --git a/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java b/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java index 7f7a839..90da29f 100644 --- a/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java +++ b/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java @@ -11,6 +11,7 @@ import io.frictionlessdata.datapackage.exceptions.DataPackageException; import io.frictionlessdata.datapackage.exceptions.DataPackageValidationException; import io.frictionlessdata.tableschema.Table; +import io.frictionlessdata.tableschema.exception.ForeignKeyException; import io.frictionlessdata.tableschema.fk.ForeignKey; import io.frictionlessdata.tableschema.io.FileReference; import io.frictionlessdata.tableschema.io.URLFileReference; @@ -243,16 +244,52 @@ public List getTables() throws Exception { return tables; } - public void checkRelations() { + public void checkRelations(List> resources) throws Exception { if (null != schema) { for (ForeignKey fk : schema.getForeignKeys()) { fk.validate(); fk.getReference().validate(); } - for (ForeignKey fk : schema.getForeignKeys()) { - if (null != fk.getReference().getResource()) { - //Package pkg = new Package(fk.getReference().getDatapackage(), true); - // TODO fix this + Iterator> iterator = this.mappingIterator(false); + while (iterator.hasNext()) { + Map row = iterator.next(); + for (ForeignKey fk : schema.getForeignKeys()) { + if (null != fk.getReference().getResource()) { + String targetResourceName = fk.getReference().getResource(); + Resource targetResoure = resources + .stream() + .filter((r) -> r.getName().equals(targetResourceName)) + .findFirst() + .orElse(null); + if (null == targetResoure) { + throw new ForeignKeyException("Target resource " + targetResourceName + " for foreign key not found"); + } + Iterator> targetIterator = targetResoure.mappingIterator(false); + int cnt = 0; + boolean found = false; + while (targetIterator.hasNext()) { + Map targetRow = targetIterator.next(); + Object targetFields = fk.getReference().getFields(); + if (targetFields instanceof String) { + if (null == targetRow.get((String) targetFields)) { + throw new ForeignKeyException("Foreign key '" + targetFields + "' violation in row \"" + cnt + "\""); + } + Object targetVal = targetRow.get((String)targetFields); + Object val = row.get((String)fk.getFields()); + found = found | (targetVal.equals(val)); + if (found) + break; + } else if (targetFields instanceof String[]) { + for (String targetField : (String[])targetFields) { + + } + } + cnt++; + } + if (!found) { + throw new ForeignKeyException("Foreign key '" + fk.getFields() + "' violation."); + } + } } } } @@ -264,7 +301,6 @@ public void validate() { try { // will validate schema against data tables.forEach(Table::validate); - checkRelations(); } catch (Exception ex) { if (ex instanceof DataPackageValidationException) { errors.add((DataPackageValidationException) ex); diff --git a/src/main/java/io/frictionlessdata/datapackage/resource/Resource.java b/src/main/java/io/frictionlessdata/datapackage/resource/Resource.java index 01ecd6d..20b7b5c 100644 --- a/src/main/java/io/frictionlessdata/datapackage/resource/Resource.java +++ b/src/main/java/io/frictionlessdata/datapackage/resource/Resource.java @@ -38,16 +38,25 @@ */ public interface Resource { - String FORMAT_CSV = "csv"; - String FORMAT_JSON = "json"; + String FORMAT_CSV = TableDataSource.Format.FORMAT_CSV.getLabel(); + String FORMAT_JSON = TableDataSource.Format.FORMAT_JSON.getLabel();; /** * Return the {@link Table} objects underlying the Resource. + * * @return Table(s) * @throws Exception if reading the tables fails. */ List
getTables() throws Exception ; + /** + * Return a JSON representation of the Resource descriptor for use in a datapackage.json, + * i.e. not the JSON data + * + * See https://specs.frictionlessdata.io/data-resource/#descriptor + * + * @return JSON of the Resource description + */ String getJson(); /** @@ -386,7 +395,7 @@ public interface Resource { String getSerializationFormat(); - void checkRelations() throws Exception; + void checkRelations(List> resources) throws Exception; /** * Recreate a Resource object from a JSON descriptor, a base path to resolve relative file paths against diff --git a/src/test/java/io/frictionlessdata/datapackage/ForeignKeysTest.java b/src/test/java/io/frictionlessdata/datapackage/ForeignKeysTest.java index f4f9506..d9ba150 100644 --- a/src/test/java/io/frictionlessdata/datapackage/ForeignKeysTest.java +++ b/src/test/java/io/frictionlessdata/datapackage/ForeignKeysTest.java @@ -1,6 +1,8 @@ package io.frictionlessdata.datapackage; import io.frictionlessdata.datapackage.resource.Resource; +import io.frictionlessdata.tableschema.exception.ForeignKeyException; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -9,13 +11,56 @@ public class ForeignKeysTest { @Test - @DisplayName("Test that a schema can be defined via a URL") - // Test for https://github.com/frictionlessdata/specs/issues/645 - void testValidationURLAsSchemaReference() throws Exception{ + @DisplayName("Test that a schema can define foreign keys and our code resolves them") + // Test for https://github.com/frictionlessdata/datapackage-java/issues/4 + void testForeignKeyValidation() throws Exception{ Path resourcePath = TestUtil.getResourcePath("/fixtures/datapackages/foreign-keys.json"); Package pkg = new Package(resourcePath, true); - System.out.println(pkg); Resource teams = pkg.getResource("teams"); - teams.checkRelations(); + teams.checkRelations(pkg.getResources()); + } + + @Test + @DisplayName("Test that a schema can define foreign keys and our code resolves them -> must throw on invalid ref") + // Test for https://github.com/frictionlessdata/datapackage-java/issues/4 + void testForeignKeyValidationError() throws Exception{ + Path resourcePath = TestUtil.getResourcePath("/fixtures/datapackages/foreign-keys-invalid.json"); + Package pkg = new Package(resourcePath, true); + Resource teams = pkg.getResource("teams"); + ForeignKeyException fke = Assertions.assertThrows( + ForeignKeyException.class, () -> teams.checkRelations(pkg.getResources())); + Assertions.assertEquals("Foreign key 'city' violation.", fke.getMessage()); + } + + @Test + @DisplayName("Test that a schema can define compound foreign keys and our code resolves them") + // Test for https://github.com/frictionlessdata/datapackage-java/issues/4 + void testCompoundForeignKeyValidation() throws Exception{ + Path resourcePath = TestUtil.getResourcePath("/fixtures/datapackages/foreign-keys-extended.json"); + Package pkg = new Package(resourcePath, true); + Resource teams = pkg.getResource("teams"); + teams.checkRelations(pkg.getResources()); + } + + @Test + @DisplayName("Test that a schema with mismatched String and Array foreign key must throw") + void testCompoundForeignKeyValidationError() throws Exception{ + Path resourcePath = TestUtil.getResourcePath("/fixtures/datapackages/foreign-keys-extended-invalid1.json"); + ForeignKeyException fke = Assertions.assertThrows( + ForeignKeyException.class, () -> new Package(resourcePath, true)); + Assertions.assertEquals( + "The reference's fields property must be an array if the outer fields is an array.", + fke.getMessage()); + } + + @Test + @DisplayName("Test that a schema with mismatched String and Array foreign key must throw") + void testCompoundForeignKeyValidationError2() throws Exception{ + Path resourcePath = TestUtil.getResourcePath("/fixtures/datapackages/foreign-keys-extended-invalid2.json"); + ForeignKeyException fke = Assertions.assertThrows( + ForeignKeyException.class, () -> new Package(resourcePath, true)); + Assertions.assertEquals( + "The reference's fields property must be a string if the outer fields is a string.", + fke.getMessage()); } } diff --git a/src/test/java/io/frictionlessdata/datapackage/TestUtil.java b/src/test/java/io/frictionlessdata/datapackage/TestUtil.java index 24a1cf5..d89dbd8 100644 --- a/src/test/java/io/frictionlessdata/datapackage/TestUtil.java +++ b/src/test/java/io/frictionlessdata/datapackage/TestUtil.java @@ -1,9 +1,6 @@ package io.frictionlessdata.datapackage; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; +import java.io.*; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; @@ -35,6 +32,9 @@ public static Path getResourcePath (String fileName) { } // Create file-URL of source file: URL sourceFileUrl = TestUtil.class.getResource(locFileName); + if (null == sourceFileUrl) { + throw new FileNotFoundException("Resource "+fileName+" not found"); + } // Get path of URL return Paths.get(sourceFileUrl.toURI()); } catch (Exception ex) { diff --git a/src/test/resources/fixtures/datapackages/foreign-keys-extended-invalid1.json b/src/test/resources/fixtures/datapackages/foreign-keys-extended-invalid1.json new file mode 100644 index 0000000..1eee803 --- /dev/null +++ b/src/test/resources/fixtures/datapackages/foreign-keys-extended-invalid1.json @@ -0,0 +1,76 @@ +{ + "name": "foreign-keys", + "resources": [ + { + "name": "teams", + "data": [ + [ + "id", + "name", + "city" + ], + [ + "1", + "Arsenal", + "London" + ], + [ + "2", + "Real", + "Madrid" + ], + [ + "3", + "Bayern", + "Munich" + ] + ], + "schema": { + "fields": [ + { + "name": "id", + "type": "integer" + }, + { + "name": "name", + "type": "string" + }, + { + "name": "city", + "type": "string" + } + ], + "foreignKeys": [ + { + "fields": ["id", "city"], + "reference": { + "resource": "cities", + "fields": "name" + } + } + ] + } + }, + { + "name": "cities", + "data": [ + [ + "name", + "country" + ], + [ + "London", + "England" + ], + [ + "Madrid", + "Spain" + ], + [ + "Munich", + "Germany" + ] + ] + } + ] +} \ No newline at end of file diff --git a/src/test/resources/fixtures/datapackages/foreign-keys-extended-invalid2.json b/src/test/resources/fixtures/datapackages/foreign-keys-extended-invalid2.json new file mode 100644 index 0000000..8a56511 --- /dev/null +++ b/src/test/resources/fixtures/datapackages/foreign-keys-extended-invalid2.json @@ -0,0 +1,76 @@ +{ + "name": "foreign-keys", + "resources": [ + { + "name": "teams", + "data": [ + [ + "id", + "name", + "city" + ], + [ + "1", + "Arsenal", + "London" + ], + [ + "2", + "Real", + "Madrid" + ], + [ + "3", + "Bayern", + "Munich" + ] + ], + "schema": { + "fields": [ + { + "name": "id", + "type": "integer" + }, + { + "name": "name", + "type": "string" + }, + { + "name": "city", + "type": "string" + } + ], + "foreignKeys": [ + { + "fields": "city", + "reference": { + "resource": "cities", + "fields": ["name", "country"] + } + } + ] + } + }, + { + "name": "cities", + "data": [ + [ + "name", + "country" + ], + [ + "London", + "England" + ], + [ + "Madrid", + "Spain" + ], + [ + "Munich", + "Germany" + ] + ] + } + ] +} \ No newline at end of file diff --git a/src/test/resources/fixtures/datapackages/foreign-keys-extended.json b/src/test/resources/fixtures/datapackages/foreign-keys-extended.json new file mode 100644 index 0000000..b3656ac --- /dev/null +++ b/src/test/resources/fixtures/datapackages/foreign-keys-extended.json @@ -0,0 +1,80 @@ +{ + "name": "foreign-keys", + "resources": [ + { + "name": "teams", + "data": [ + [ + "id", + "name", + "city" + ], + [ + "1", + "Arsenal", + "London" + ], + [ + "2", + "Real", + "Madrid" + ], + [ + "3", + "Bayern", + "Munich" + ] + ], + "schema": { + "fields": [ + { + "name": "id", + "type": "integer" + }, + { + "name": "name", + "type": "string" + }, + { + "name": "city", + "type": "string" + } + ], + "foreignKeys": [ + { + "fields": ["id", "city"], + "reference": { + "resource": "cities", + "fields": ["id","name"] + } + } + ] + } + }, + { + "name": "cities", + "data": [ + [ + "id", + "name", + "country" + ], + [ + "1", + "London", + "England" + ], + [ + "2", + "Madrid", + "Spain" + ], + [ + "3", + "Munich", + "Germany" + ] + ] + } + ] +} \ No newline at end of file diff --git a/src/test/resources/fixtures/datapackages/foreign-keys-invalid.json b/src/test/resources/fixtures/datapackages/foreign-keys-invalid.json new file mode 100644 index 0000000..e6e63db --- /dev/null +++ b/src/test/resources/fixtures/datapackages/foreign-keys-invalid.json @@ -0,0 +1,72 @@ +{ + "name": "foreign-keys", + "resources": [ + { + "name": "teams", + "data": [ + [ + "id", + "name", + "city" + ], + [ + "1", + "Arsenal", + "London" + ], + [ + "2", + "Real", + "Madrid" + ], + [ + "3", + "Bayern", + "Munich" + ] + ], + "schema": { + "fields": [ + { + "name": "id", + "type": "integer" + }, + { + "name": "name", + "type": "string" + }, + { + "name": "city", + "type": "string" + } + ], + "foreignKeys": [ + { + "fields": "city", + "reference": { + "resource": "cities", + "fields": "name" + } + } + ] + } + }, + { + "name": "cities", + "data": [ + [ + "name", + "country" + ], + [ + "London", + "England" + ], + [ + "Madrid", + "Spain" + ] + ] + } + ] +} \ No newline at end of file diff --git a/src/test/resources/fixtures/datapackages/foreign-keys.json b/src/test/resources/fixtures/datapackages/foreign-keys.json index e6e63db..cb5559f 100644 --- a/src/test/resources/fixtures/datapackages/foreign-keys.json +++ b/src/test/resources/fixtures/datapackages/foreign-keys.json @@ -65,6 +65,10 @@ [ "Madrid", "Spain" + ], + [ + "Munich", + "Germany" ] ] } From 73eff339d9ce5a83d0a4bfe3e41e4922287a136b Mon Sep 17 00:00:00 2001 From: iSnow <139699+iSnow@users.noreply.github.com> Date: Thu, 26 Jan 2023 00:24:43 +0100 Subject: [PATCH 2/3] Foreign keys & relations --- pom.xml | 8 +++++--- .../datapackage/resource/AbstractResource.java | 15 +++++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index 8b5bbb6..b40d92a 100644 --- a/pom.xml +++ b/pom.xml @@ -230,19 +230,21 @@ + + io.frictionlessdata tableschema-java - 0.6.2-SNAPSHOT + 0.6.12-SNAPSHOT compile ---> + diff --git a/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java b/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java index 90da29f..26c76a1 100644 --- a/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java +++ b/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java @@ -276,12 +276,19 @@ public void checkRelations(List> resources) throws Exception { } Object targetVal = targetRow.get((String)targetFields); Object val = row.get((String)fk.getFields()); - found = found | (targetVal.equals(val)); + found = targetVal.equals(val); if (found) break; - } else if (targetFields instanceof String[]) { - for (String targetField : (String[])targetFields) { - + } else if (targetFields instanceof List) { + List stringFields = (List) targetFields; + found = true; + for (int idx = 0; idx < stringFields.size(); idx++) { + String targetKey = stringFields.get(idx); + String fkKey = ((List)fk.getFields()).get(idx); + found = found & (targetRow.get(targetKey).toString().equals(row.get(fkKey).toString())); + } + if (found){ + break; } } cnt++; From 62a94f2103f7ae84f75b2315ebcce5424dd5caf0 Mon Sep 17 00:00:00 2001 From: iSnow <139699+iSnow@users.noreply.github.com> Date: Mon, 6 Feb 2023 23:21:44 +0100 Subject: [PATCH 3/3] Foreign keys & relations --- .../datapackage/resource/AbstractResource.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java b/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java index 26c76a1..1398f65 100644 --- a/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java +++ b/src/main/java/io/frictionlessdata/datapackage/resource/AbstractResource.java @@ -284,6 +284,9 @@ public void checkRelations(List> resources) throws Exception { found = true; for (int idx = 0; idx < stringFields.size(); idx++) { String targetKey = stringFields.get(idx); + if (null == targetRow.get(targetKey)) { + throw new ForeignKeyException("Foreign key '" + targetKey + "' violation in row \"" + cnt + "\""); + } String fkKey = ((List)fk.getFields()).get(idx); found = found & (targetRow.get(targetKey).toString().equals(row.get(fkKey).toString())); }