diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e9b2b144f896c631cfb1129ec249c0bd35b386c1..c2de130bd802f9bd3be3d9f0e91a53a74401225b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -89,6 +89,7 @@ test: trigger_build: tags: [ docker ] stage: deploy + needs: [ test ] script: - *env diff --git a/.gitlab/merge_request_templates/Default.md b/.gitlab/merge_request_templates/Default.md deleted file mode 100644 index ac68afc814247e5a64395315e630d5de44a5f306..0000000000000000000000000000000000000000 --- a/.gitlab/merge_request_templates/Default.md +++ /dev/null @@ -1,48 +0,0 @@ -# Summary - - Insert a meaningful description for this merge request here. What is the - new/changed behavior? Which bug has been fixed? Are there related Issues? - -# Focus - - Point the reviewer to the core of the code change. Where should they start - reading? What should they focus on (e.g. security, performance, - maintainability, user-friendliness, compliance with the specs, finding more - corner cases, concrete questions)? - -# Test Environment - - How to set up a test environment for manual testing? - -# Check List for the Author - -Please, prepare your MR for a review. Be sure to write a summary and a -focus and create gitlab comments for the reviewer. They should guide the -reviewer through the changes, explain your changes and also point out open -questions. For further good practices have a look at [our review -guidelines](https://gitlab.com/caosdb/caosdb/-/blob/dev/REVIEW_GUIDELINES.md) - -- [ ] All automated tests pass -- [ ] Reference related Issues -- [ ] Up-to-date CHANGELOG.md -- [ ] Annotations in code (Gitlab comments) - - Intent of new code - - Problems with old code - - Why this implementation? - - -# Check List for the Reviewer - - -- [ ] I understand the intent of this MR -- [ ] All automated tests pass -- [ ] Up-to-date CHANGELOG.md -- [ ] The test environment setup works and the intended behavior is - reproducible in the test environment -- [ ] In-code documentation and comments are up-to-date. -- [ ] Check: Are there specifications? Are they satisfied? - -For further good practices have a look at [our review guidelines](https://gitlab.com/caosdb/caosdb/-/blob/dev/REVIEW_GUIDELINES.md). - - -/assign me diff --git a/CHANGELOG.md b/CHANGELOG.md index d0754bff61238dde64e09c4d758231bac6c7c561..a12fdf048e9eb9e555a7cd18b692abe69f5bf421 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Missing serialization of file descriptors in the GRPC-API during retrievals. * [caosdb-server#131](https://gitlab.com/caosdb/caosdb-server/-/issues/131) Query: AND does not work with sub-properties +* Add previously missing `Value.equals` functions * [caosdb-server#132](https://gitlab.com/caosdb/caosdb-server/-/issues/132) Query: subproperties should not require parentheses * [caosdb-server#174](https://gitlab.indiscale.com/caosdb/src/caosdb-server/-/issues/174) diff --git a/src/main/java/org/caosdb/datetime/FragmentDateTime.java b/src/main/java/org/caosdb/datetime/FragmentDateTime.java index 52faf5f2040de56c05bb7ce4b5aee130fadf67c3..38c16a988a0103a121767b99078c0ca41c6263d5 100644 --- a/src/main/java/org/caosdb/datetime/FragmentDateTime.java +++ b/src/main/java/org/caosdb/datetime/FragmentDateTime.java @@ -20,9 +20,13 @@ * * ** end header */ + +/** @review Daniel Hornung 2022-03-04 */ package org.caosdb.datetime; +import java.util.Objects; import java.util.TimeZone; +import org.caosdb.server.datatype.Value; public abstract class FragmentDateTime implements DateTimeInterface { @@ -53,4 +57,13 @@ public abstract class FragmentDateTime implements DateTimeInterface { this.nanosecond = nanosecond; this.timeZone = tz; } + + @Override + public boolean equals(Value val) { + if (val instanceof FragmentDateTime) { + FragmentDateTime that = (FragmentDateTime) val; + return Objects.equals(that.toDatabaseString(), this.toDatabaseString()); + } + return false; + } } diff --git a/src/main/java/org/caosdb/datetime/UTCDateTime.java b/src/main/java/org/caosdb/datetime/UTCDateTime.java index 0ecb3ac60dfe796692c344e590f3935c36fceac3..1dd86cc975d4b92066f2cf44fff51493ade5babe 100644 --- a/src/main/java/org/caosdb/datetime/UTCDateTime.java +++ b/src/main/java/org/caosdb/datetime/UTCDateTime.java @@ -20,13 +20,17 @@ * * ** end header */ + +/** @review Daniel Hornung 2022-03-04 */ package org.caosdb.datetime; import java.util.ArrayList; import java.util.Calendar; import java.util.GregorianCalendar; +import java.util.Objects; import java.util.TimeZone; import org.caosdb.server.datatype.AbstractDatatype.Table; +import org.caosdb.server.datatype.Value; import org.jdom2.Element; public class UTCDateTime implements Interval { @@ -360,4 +364,13 @@ public class UTCDateTime implements Interval { public boolean hasNanoseconds() { return this.nanoseconds != null; } + + @Override + public boolean equals(Value val) { + if (val instanceof UTCDateTime) { + UTCDateTime that = (UTCDateTime) val; + return Objects.equals(that.toDatabaseString(), this.toDatabaseString()); + } + return false; + } } diff --git a/src/main/java/org/caosdb/server/datatype/AbstractEnumValue.java b/src/main/java/org/caosdb/server/datatype/AbstractEnumValue.java index 06216c822c705fa6fbaef949386966af15ff6b13..be88002543155771d2937e216c64d52931051c25 100644 --- a/src/main/java/org/caosdb/server/datatype/AbstractEnumValue.java +++ b/src/main/java/org/caosdb/server/datatype/AbstractEnumValue.java @@ -1,10 +1,10 @@ /* - * ** header v3.0 * This file is a part of the CaosDB Project. * * Copyright (C) 2018 Research Group Biomedical Physics, * Max-Planck-Institute for Dynamics and Self-Organization Göttingen * + * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as * published by the Free Software Foundation, either version 3 of the @@ -17,9 +17,9 @@ * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see <https://www.gnu.org/licenses/>. - * - * ** end header */ + +/** @review Daniel Hornung 2022-03-04 */ package org.caosdb.server.datatype; import com.google.common.base.Objects; @@ -52,7 +52,15 @@ public abstract class AbstractEnumValue implements SingleValue { @Override public boolean equals(final Object obj) { if (obj instanceof AbstractEnumValue) { - final AbstractEnumValue that = (AbstractEnumValue) obj; + return equals((AbstractEnumValue) obj); + } + return false; + } + + @Override + public boolean equals(Value val) { + if (val instanceof AbstractEnumValue) { + final AbstractEnumValue that = (AbstractEnumValue) val; return Objects.equal(that.value, this.value); } return false; diff --git a/src/main/java/org/caosdb/server/datatype/CollectionValue.java b/src/main/java/org/caosdb/server/datatype/CollectionValue.java index 55cbf0489c3f965e19784c80439d291930d89841..fc94f660490284e560a34d1c407e2d74c51f6358 100644 --- a/src/main/java/org/caosdb/server/datatype/CollectionValue.java +++ b/src/main/java/org/caosdb/server/datatype/CollectionValue.java @@ -20,6 +20,8 @@ * * ** end header */ + +/** @review Daniel Hornung 2022-03-04 */ package org.caosdb.server.datatype; import java.util.ArrayList; @@ -80,4 +82,24 @@ public class CollectionValue implements Value, Iterable<IndexedSingleValue> { public int size() { return list.size(); } + + /** Compares if the content is equal, regardless of the order. */ + @Override + public boolean equals(Value val) { + if (val instanceof CollectionValue) { + CollectionValue that = (CollectionValue) val; + sort(); + that.sort(); + return this.list.equals(that.list); + } + return false; + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof Value) { + return this.equals((Value) obj); + } + return false; + } } diff --git a/src/main/java/org/caosdb/server/datatype/GenericValue.java b/src/main/java/org/caosdb/server/datatype/GenericValue.java index 02d735abcc36297e61b08f99455f077d832a3a4b..a4b1b0b1508b9debde1f34f1dd58786bdae840c8 100644 --- a/src/main/java/org/caosdb/server/datatype/GenericValue.java +++ b/src/main/java/org/caosdb/server/datatype/GenericValue.java @@ -89,8 +89,16 @@ public class GenericValue implements SingleValue { @Override public boolean equals(final Object obj) { - if (obj instanceof GenericValue) { - final GenericValue that = (GenericValue) obj; + if (obj instanceof Value) { + return equals((Value) obj); + } + return false; + } + + @Override + public boolean equals(Value val) { + if (val instanceof GenericValue) { + final GenericValue that = (GenericValue) val; return Objects.equal(that.value, this.value); } return false; diff --git a/src/main/java/org/caosdb/server/datatype/IndexedSingleValue.java b/src/main/java/org/caosdb/server/datatype/IndexedSingleValue.java index a9ed584b3ee858e6de2bbb84ad4a43da820cedb0..ec9280c3ab0cbdf6e577feec4d40a01f6402dbfe 100644 --- a/src/main/java/org/caosdb/server/datatype/IndexedSingleValue.java +++ b/src/main/java/org/caosdb/server/datatype/IndexedSingleValue.java @@ -22,6 +22,7 @@ */ package org.caosdb.server.datatype; +import java.util.Objects; import org.caosdb.server.datatype.AbstractDatatype.Table; import org.jdom2.Element; @@ -78,4 +79,21 @@ public class IndexedSingleValue implements SingleValue, Comparable<IndexedSingle public SingleValue getWrapped() { return this.wrapped; } + + @Override + public boolean equals(Value val) { + if (val instanceof IndexedSingleValue) { + IndexedSingleValue that = (IndexedSingleValue) val; + return Objects.equals(that.wrapped, this.wrapped); + } + return false; + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof Value) { + return this.equals((Value) obj); + } + return false; + } } diff --git a/src/main/java/org/caosdb/server/datatype/ReferenceValue.java b/src/main/java/org/caosdb/server/datatype/ReferenceValue.java index 89601b50a46cbf838a995a222b0d4c43150f8a19..525d3c43160adc4b47925c19cafe2acd2b6b2246 100644 --- a/src/main/java/org/caosdb/server/datatype/ReferenceValue.java +++ b/src/main/java/org/caosdb/server/datatype/ReferenceValue.java @@ -4,8 +4,9 @@ * * Copyright (C) 2018 Research Group Biomedical Physics, * Max-Planck-Institute for Dynamics and Self-Organization Göttingen - * Copyright (C) 2020 IndiScale GmbH <info@indiscale.com> + * Copyright (C) 2022 IndiScale GmbH <info@indiscale.com> * Copyright (C) 2020 Timm Fitschen <t.fitschen@indiscale.com> + * Copyright (C) 2022 Daniel Hornung <d.hornung@indiscale.com> * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -22,6 +23,8 @@ * * ** end header */ + +/** @review Daniel Hornung 2022-03-04 */ package org.caosdb.server.datatype; import java.util.Objects; @@ -199,13 +202,29 @@ public class ReferenceValue implements SingleValue { @Override public boolean equals(final Object obj) { - if (obj instanceof ReferenceValue) { - final ReferenceValue that = (ReferenceValue) obj; + if (obj instanceof Value) { + return equals((Value) obj); + } + return false; + } + + /** + * Test if this is equal to the other object. + * + * <p>Two references are equal, if 1) they both have IDs and their content is equal or 2) at least + * one does not have an ID, but their names are equal. Otherwise they are considered unequal. + * + * @param val The other object. + */ + @Override + public boolean equals(Value val) { + if (val instanceof ReferenceValue) { + final ReferenceValue that = (ReferenceValue) val; if (that.getId() != null && getId() != null) { return that.getId().equals(getId()) && Objects.deepEquals(that.getVersion(), this.getVersion()); } else if (that.getName() != null && getName() != null) { - return that.getName().equals(getName()); + return Objects.equals(that.getName(), this.getName()); } } return false; diff --git a/src/main/java/org/caosdb/server/datatype/Value.java b/src/main/java/org/caosdb/server/datatype/Value.java index 540e6bc52b72744c4d7c1c651c40924d1da57c6d..a2612fb37dae11953b6083f3be8132b2dd61f77f 100644 --- a/src/main/java/org/caosdb/server/datatype/Value.java +++ b/src/main/java/org/caosdb/server/datatype/Value.java @@ -26,4 +26,6 @@ import org.jdom2.Element; public interface Value { public void addToElement(Element e); + + public abstract boolean equals(Value val); } diff --git a/src/main/java/org/caosdb/server/transaction/WriteTransaction.java b/src/main/java/org/caosdb/server/transaction/WriteTransaction.java index ee9aa81e49c99b15ac30a8164a4b9afc18b666f2..7b9bc7495a67aa458ad40f25435cb0af7da290d7 100644 --- a/src/main/java/org/caosdb/server/transaction/WriteTransaction.java +++ b/src/main/java/org/caosdb/server/transaction/WriteTransaction.java @@ -3,8 +3,9 @@ * * Copyright (C) 2018 Research Group Biomedical Physics, * Max-Planck-Institute for Dynamics and Self-Organization Göttingen - * Copyright (C) 2019-2021 IndiScale GmbH <info@indiscale.com> - * Copyright (C) 2019-2021 Timm Fitschen <t.fitschen@indiscale.com> + * Copyright (C) 2019-2022 IndiScale GmbH <info@indiscale.com> + * Copyright (C) 2019-2022 Timm Fitschen <t.fitschen@indiscale.com> + * Copyright (C) 2022 Daniel Hornung <d.hornung@indiscale.com> * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -41,6 +42,7 @@ import org.caosdb.server.entity.DeleteEntity; import org.caosdb.server.entity.EntityInterface; import org.caosdb.server.entity.FileProperties; import org.caosdb.server.entity.InsertEntity; +import org.caosdb.server.entity.Message; import org.caosdb.server.entity.RetrieveEntity; import org.caosdb.server.entity.UpdateEntity; import org.caosdb.server.entity.container.TransactionContainer; @@ -404,15 +406,25 @@ public class WriteTransaction extends Transaction<WritableContainer> && oldEntity.hasRole() && !newEntity.getRole().equals(oldEntity.getRole()) || newEntity.hasRole() ^ oldEntity.hasRole()) { - needPermissions.add(EntityPermission.UPDATE_ROLE); + if (!(newEntity instanceof Property && oldEntity instanceof Property)) { + needPermissions.add(EntityPermission.UPDATE_ROLE); + } updatetable = true; } // entity value - if (newEntity.hasValue() - && oldEntity.hasValue() - && !newEntity.getValue().equals(oldEntity.getValue()) - || newEntity.hasValue() ^ oldEntity.hasValue()) { + if (newEntity.hasValue() && oldEntity.hasValue()) { + try { + newEntity.parseValue(); + oldEntity.parseValue(); + } catch (Message m) { + // ignore, parsing is handled elsewhere + } + if (!newEntity.getValue().equals(oldEntity.getValue())) { + needPermissions.add(EntityPermission.UPDATE_VALUE); + updatetable = true; + } + } else if (newEntity.hasValue() ^ oldEntity.hasValue()) { needPermissions.add(EntityPermission.UPDATE_VALUE); updatetable = true; } diff --git a/src/test/java/org/caosdb/server/transaction/UpdateTest.java b/src/test/java/org/caosdb/server/transaction/UpdateTest.java index fb121c260a7706e806ddc73e9005ea4a440f1510..2e8465eac53e98b54df3108a4824caaf88fa1129 100644 --- a/src/test/java/org/caosdb/server/transaction/UpdateTest.java +++ b/src/test/java/org/caosdb/server/transaction/UpdateTest.java @@ -1,9 +1,10 @@ /* - * ** header v3.0 * This file is a part of the CaosDB Project. * * Copyright (C) 2018 Research Group Biomedical Physics, * Max-Planck-Institute for Dynamics and Self-Organization Göttingen + * Copyright (C) 2022 Indiscale GmbH <info@indiscale.com> + * Copyright (C) 2022 Daniel Hornung <d.hornung@indiscale.com> * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -17,24 +18,28 @@ * * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see <https://www.gnu.org/licenses/>. - * - * ** end header */ package org.caosdb.server.transaction; import static org.caosdb.server.utils.EntityStatus.QUALIFIED; import static org.caosdb.server.utils.EntityStatus.VALID; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.security.NoSuchAlgorithmException; +import java.util.HashSet; import org.caosdb.server.CaosDBException; import org.caosdb.server.CaosDBServer; +import org.caosdb.server.datatype.CollectionValue; import org.caosdb.server.datatype.GenericValue; +import org.caosdb.server.datatype.ReferenceValue; import org.caosdb.server.entity.Entity; import org.caosdb.server.entity.EntityInterface; import org.caosdb.server.entity.StatementStatus; import org.caosdb.server.entity.wrapper.Property; +import org.caosdb.server.permissions.EntityPermission; +import org.caosdb.server.permissions.Permission; import org.caosdb.server.utils.EntityStatus; import org.junit.BeforeClass; import org.junit.Test; @@ -176,4 +181,102 @@ public class UpdateTest { assertEquals(newEntity.getEntityStatus(), QUALIFIED); assertEquals(newProperty.getEntityStatus(), QUALIFIED); } + + @Test + public void testDeriveUpdate_Collections() + throws NoSuchAlgorithmException, CaosDBException, IOException { + + final Entity newEntity = new Entity(); + final Property newProperty = new Property(1); + newProperty.setDatatype("List<Person>"); + CollectionValue newValue = new CollectionValue(); + newValue.add(new ReferenceValue(1234)); + newValue.add(null); + newValue.add(new GenericValue(2345)); + newValue.add(new GenericValue(3465)); + newProperty.setValue(newValue); + newEntity.addProperty(newProperty); + newEntity.setEntityStatus(QUALIFIED); + newProperty.setEntityStatus(QUALIFIED); + + // old entity represents the stored entity. + final Entity oldEntity = new Entity(); + final Property oldProperty = new Property(1); + oldProperty.setDatatype("List<Person>"); + CollectionValue oldValue = new CollectionValue(); + // Values are shuffled but have the correct index + oldValue.add(1, null); + oldValue.add(3, new GenericValue(3465)); + oldValue.add(2, new ReferenceValue(2345)); + oldValue.add(0, new ReferenceValue(1234)); + oldProperty.setValue(oldValue); + oldEntity.addProperty(oldProperty); + + HashSet<Permission> permissions = new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); + // Both have been identified as equals + assertTrue(permissions.isEmpty()); + assertEquals(newEntity.getEntityStatus(), VALID); + assertEquals(newProperty.getEntityStatus(), VALID); + + // NEW TEST CASE + newValue.add(null); // newValue has another null + newProperty.setValue(newValue); + oldEntity.addProperty(oldProperty); // Add again, because deriveUpdate throws it away + newEntity.setEntityStatus(QUALIFIED); + newProperty.setEntityStatus(QUALIFIED); + + HashSet<Permission> permissions2 = + new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); + HashSet<Permission> expected = new HashSet<Permission>(); + expected.add(EntityPermission.UPDATE_ADD_PROPERTY); + expected.add(EntityPermission.UPDATE_REMOVE_PROPERTY); + assertEquals(expected, permissions2); + assertEquals(newEntity.getEntityStatus(), QUALIFIED); + assertEquals(newProperty.getEntityStatus(), QUALIFIED); + + // NEW TEST CASE + // now change the order of oldValue + CollectionValue oldValue2 = new CollectionValue(); + // Values are shuffled but have the correct index + oldValue2.add(0, null); + oldValue2.add(1, new GenericValue(3465)); + oldValue2.add(2, new ReferenceValue(2345)); + oldValue2.add(3, new ReferenceValue(1234)); + oldValue2.add(4, null); + oldProperty.setValue(oldValue2); + + oldEntity.addProperty(oldProperty); // Add again, because deriveUpdate throws it away + newEntity.setEntityStatus(QUALIFIED); + newProperty.setEntityStatus(QUALIFIED); + + HashSet<Permission> permissions3 = + new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); + assertEquals(expected, permissions3); + assertEquals(newEntity.getEntityStatus(), QUALIFIED); + assertEquals(newProperty.getEntityStatus(), QUALIFIED); + } + + /** For issue #217: Server gets list property datatype wrong if description is updated. */ + @Test + public void testDeriveUpdate_UpdateList() + throws NoSuchAlgorithmException, CaosDBException, IOException { + final Property oldProperty = new Property(1); + final Property newProperty = new Property(1); + oldProperty.setDatatype("List<Person>"); + newProperty.setDatatype("List<Person>"); + final Entity oldEntity = new Entity(); + final Entity newEntity = new Entity(); + + oldProperty.setRole("Record"); + newProperty.setRole("Property"); + oldEntity.addProperty(oldProperty); + newEntity.addProperty(newProperty); + + // The only difference between old and new + newEntity.setDescription("New description."); + + new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); + // check if newEntity's Property is QUALIFIED. + assertEquals(QUALIFIED, newEntity.getProperties().get(0).getEntityStatus()); + } }