diff --git a/src/main/java/org/caosdb/server/grpc/EntityTransactionServiceImpl.java b/src/main/java/org/caosdb/server/grpc/EntityTransactionServiceImpl.java index ceb69a007dd29f3022b39f45637ef7ed91947dbb..caeac0e6a0f7e6414585fcae6e81d95036daab4a 100644 --- a/src/main/java/org/caosdb/server/grpc/EntityTransactionServiceImpl.java +++ b/src/main/java/org/caosdb/server/grpc/EntityTransactionServiceImpl.java @@ -654,6 +654,7 @@ public class EntityTransactionServiceImpl extends EntityTransactionServiceImplBa } final WriteTransaction transaction = new WriteTransaction(container); + transaction.setNoIdIsError(false); transaction.execute(); for (final EntityInterface entity : container) { diff --git a/src/main/java/org/caosdb/server/jobs/core/CheckParValid.java b/src/main/java/org/caosdb/server/jobs/core/CheckParValid.java index e663874189ee0301a5b30452468391711624b606..fa1c6fd36771e0ddcd08b330724ef1d6aa725956 100644 --- a/src/main/java/org/caosdb/server/jobs/core/CheckParValid.java +++ b/src/main/java/org/caosdb/server/jobs/core/CheckParValid.java @@ -60,7 +60,7 @@ public class CheckParValid extends EntityJob { // The parent has neither an id nor a name. // Therefore it cannot be identified. - throw ServerMessages.ENTITY_HAS_NO_NAME_AND_NO_ID; + throw ServerMessages.ENTITY_HAS_NO_NAME_OR_ID; } if (parent.hasId()) { @@ -139,8 +139,8 @@ public class CheckParValid extends EntityJob { if (par.getEntityStatus() != EntityStatus.IGNORE) { for (final Parent par2 : getEntity().getParents()) { if (par != par2 && par2.getEntityStatus() != EntityStatus.IGNORE) { - if ((par.hasId() && par2.hasId() && par.getId().equals(par2.getId())) - || (par.hasName() && par2.hasName() && par.getName().equals(par2.getName()))) { + if (par.hasId() && par2.hasId() && par.getId().equals(par2.getId()) + || par.hasName() && par2.hasName() && par.getName().equals(par2.getName())) { if (!Objects.equal(par.getFlag("inheritance"), par2.getFlag("inheritance"))) { getEntity().addError(ServerMessages.PARENT_DUPLICATES_ERROR); getEntity().setEntityStatus(EntityStatus.UNQUALIFIED); diff --git a/src/main/java/org/caosdb/server/transaction/WriteTransaction.java b/src/main/java/org/caosdb/server/transaction/WriteTransaction.java index 7a9a0519efc908e549a53f560af10a0ceffb3b27..600e6ae54769cb89fe570af25dfa56acfbeb8039 100644 --- a/src/main/java/org/caosdb/server/transaction/WriteTransaction.java +++ b/src/main/java/org/caosdb/server/transaction/WriteTransaction.java @@ -24,6 +24,7 @@ package org.caosdb.server.transaction; import java.io.IOException; import java.security.NoSuchAlgorithmException; import java.util.HashSet; +import java.util.List; import java.util.Objects; import java.util.Set; import org.apache.shiro.SecurityUtils; @@ -67,10 +68,16 @@ import org.caosdb.server.utils.ServerMessages; public class WriteTransaction extends Transaction<WritableContainer> implements WriteTransactionInterface { + private boolean noIdIsError = true; + public WriteTransaction(final WritableContainer container) { super(container); } + public void setNoIdIsError(final boolean noIdIsError) { + this.noIdIsError = noIdIsError; + } + @Override protected final void preTransaction() throws InterruptedException { // Acquire strong access. No other thread can have access until this strong access is released. @@ -354,7 +361,7 @@ public class WriteTransaction extends Transaction<WritableContainer> * @throws IOException * @throws NoSuchAlgorithmException */ - public static HashSet<Permission> deriveUpdate( + public HashSet<Permission> deriveUpdate( final EntityInterface newEntity, final EntityInterface oldEntity) throws NoSuchAlgorithmException, IOException, CaosDBException { final HashSet<Permission> needPermissions = new HashSet<>(); @@ -461,46 +468,34 @@ public class WriteTransaction extends Transaction<WritableContainer> } // properties - outerLoop: - for (final EntityInterface newProperty : newEntity.getProperties()) { + for (final Property newProperty : newEntity.getProperties()) { // find corresponding oldProperty for this new property and make a // diff. - if (newProperty.hasId()) { - for (final EntityInterface oldProperty : oldEntity.getProperties()) { - if (newProperty.getId().equals(oldProperty.getId())) { - // do not check again. - oldEntity.getProperties().remove(oldProperty); - - if (((Property) oldProperty).getPIdx() != ((Property) newProperty).getPIdx()) { - // change order of properties - needPermissions.add(EntityPermission.UPDATE_ADD_PROPERTY); - needPermissions.add(EntityPermission.UPDATE_REMOVE_PROPERTY); - updatetable = true; - } - - deriveUpdate(newProperty, oldProperty); - if (newProperty.getEntityStatus() == EntityStatus.QUALIFIED) { - needPermissions.add(EntityPermission.UPDATE_ADD_PROPERTY); - needPermissions.add(EntityPermission.UPDATE_REMOVE_PROPERTY); - updatetable = true; - } + final Property oldProperty = findOldEntity(newProperty, oldEntity.getProperties()); + if (oldProperty != null) { + // do not check again. + oldEntity.getProperties().remove(oldProperty); + + if (oldProperty.getPIdx() != newProperty.getPIdx()) { + // change order of properties + needPermissions.add(EntityPermission.UPDATE_ADD_PROPERTY); + needPermissions.add(EntityPermission.UPDATE_REMOVE_PROPERTY); + updatetable = true; + } - continue outerLoop; - } + deriveUpdate(newProperty, oldProperty); + if (newProperty.getEntityStatus() == EntityStatus.QUALIFIED) { + needPermissions.add(EntityPermission.UPDATE_ADD_PROPERTY); + needPermissions.add(EntityPermission.UPDATE_REMOVE_PROPERTY); + updatetable = true; } + } else { - newProperty.setEntityStatus(EntityStatus.UNQUALIFIED); - newProperty.addError(ServerMessages.ENTITY_HAS_NO_ID); - newProperty.addInfo("On updates, always specify the id not just the name."); - newEntity.addError(ServerMessages.ENTITY_HAS_UNQUALIFIED_PROPERTIES); - newEntity.setEntityStatus(EntityStatus.UNQUALIFIED); - return needPermissions; + // no corresponding property found -> this property is new. + needPermissions.add(EntityPermission.UPDATE_ADD_PROPERTY); + updatetable = true; } - - // no corresponding property found -> this property is new. - needPermissions.add(EntityPermission.UPDATE_ADD_PROPERTY); - updatetable = true; } // some old properties left (and not matched with new ones) -> there are @@ -511,30 +506,20 @@ public class WriteTransaction extends Transaction<WritableContainer> } // update parents - outerLoop: for (final Parent newParent : newEntity.getParents()) { // find corresponding oldParent - if (newParent.hasId()) { - for (final Parent oldParent : oldEntity.getParents()) { - if (oldParent.getId().equals(newParent.getId())) { - // still there! do not check this one again - oldEntity.getParents().remove(oldParent); - continue outerLoop; - } - } + final Parent oldProperty = findOldEntity(newParent, oldEntity.getParents()); + + if (oldProperty != null) { + // do not check again. + oldEntity.getParents().remove(oldProperty); + } else { - newParent.setEntityStatus(EntityStatus.UNQUALIFIED); - newParent.addError(ServerMessages.ENTITY_HAS_NO_ID); - newParent.addInfo("On updates, always specify the id not just the name."); - newEntity.addError(ServerMessages.ENTITY_HAS_UNQUALIFIED_PROPERTIES); - newEntity.setEntityStatus(EntityStatus.UNQUALIFIED); - return needPermissions; + // no corresponding parent found -> this parent is new. + needPermissions.add(EntityPermission.UPDATE_ADD_PARENT); + updatetable = true; } - - // no corresponding parent found -> this parent is new. - needPermissions.add(EntityPermission.UPDATE_ADD_PARENT); - updatetable = true; } // some old parents left (and not matched with new ones) -> there are @@ -553,6 +538,29 @@ public class WriteTransaction extends Transaction<WritableContainer> return needPermissions; } + private <T extends EntityInterface> T findOldEntity( + final EntityInterface newProperty, final List<T> oldEntities) { + if (newProperty.hasId()) { + for (final T oldProperty : oldEntities) { + if (Objects.equals(oldProperty.getId(), newProperty.getId())) { + return oldProperty; + } + } + } else if (noIdIsError) { + newProperty.addError(ServerMessages.ENTITY_HAS_NO_ID); + newProperty.addInfo("On updates, always specify the id not just the name."); + } else if (newProperty.hasName()) { + for (final T oldProperty : oldEntities) { + if (oldProperty.getName().equals(newProperty.getName())) { + return oldProperty; + } + } + } else { + newProperty.addError(ServerMessages.ENTITY_HAS_NO_NAME_OR_ID); + } + return null; + } + public String getSRID() { return getContainer().getRequestId(); } diff --git a/src/main/java/org/caosdb/server/utils/ServerMessages.java b/src/main/java/org/caosdb/server/utils/ServerMessages.java index cbca0b2dca1d967536be596547f57c20a2e7593f..a12b9bc4b7c031f39a4aad99e131439440cde621 100644 --- a/src/main/java/org/caosdb/server/utils/ServerMessages.java +++ b/src/main/java/org/caosdb/server/utils/ServerMessages.java @@ -74,9 +74,6 @@ public class ServerMessages { public static final Message NAME_DUPLICATES = new Message(MessageType.Error, 0, "Entity can not be identified due to name duplicates."); - public static final Message ENTITY_HAS_NO_NAME_AND_NO_ID = - new Message(MessageType.Error, 0, "Entity has no name and no ID."); - public static final Message ENTITY_HAS_NO_PROPERTIES = new Message(MessageType.Error, 0, "Entity has no properties."); diff --git a/src/test/java/org/caosdb/server/transaction/UpdateTest.java b/src/test/java/org/caosdb/server/transaction/UpdateTest.java index d22fbea8982b1f8d88c9ccdf069a0c8a816add25..b06ea0c7f9021174f52f425843ce5ae21643e54e 100644 --- a/src/test/java/org/caosdb/server/transaction/UpdateTest.java +++ b/src/test/java/org/caosdb/server/transaction/UpdateTest.java @@ -44,7 +44,7 @@ public class UpdateTest { throws NoSuchAlgorithmException, IOException, CaosDBException { final Entity newEntity = new Entity("Name"); final Entity oldEntity = new Entity("Name"); - WriteTransaction.deriveUpdate(newEntity, oldEntity); + new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); assertEquals(newEntity.getEntityStatus(), EntityStatus.VALID); } @@ -53,7 +53,7 @@ public class UpdateTest { throws NoSuchAlgorithmException, IOException, CaosDBException { final Entity newEntity = new Entity("NewName"); final Entity oldEntity = new Entity("OldName"); - WriteTransaction.deriveUpdate(newEntity, oldEntity); + new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); assertEquals(newEntity.getEntityStatus(), EntityStatus.QUALIFIED); } @@ -67,7 +67,7 @@ public class UpdateTest { final Entity oldEntity = new Entity(); oldEntity.addProperty(oldProperty); - WriteTransaction.deriveUpdate(newEntity, oldEntity); + new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); assertEquals(newEntity.getEntityStatus(), VALID); } @@ -83,7 +83,7 @@ public class UpdateTest { final Entity oldEntity = new Entity(); oldEntity.addProperty(oldProperty); - WriteTransaction.deriveUpdate(newEntity, oldEntity); + new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); assertEquals(newEntity.getEntityStatus(), QUALIFIED); assertEquals(newProperty.getEntityStatus(), VALID); assertEquals(newProperty2.getEntityStatus(), QUALIFIED); @@ -124,7 +124,7 @@ public class UpdateTest { oldEntity.addProperty(oldProperty); - WriteTransaction.deriveUpdate(newEntity, oldEntity); + new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); assertEquals(newUnit.getEntityStatus(), VALID); assertEquals(newProperty.getEntityStatus(), VALID); assertEquals(newEntity.getEntityStatus(), VALID); @@ -165,7 +165,7 @@ public class UpdateTest { oldEntity.addProperty(oldProperty); - WriteTransaction.deriveUpdate(newEntity, oldEntity); + new WriteTransaction(null).deriveUpdate(newEntity, oldEntity); assertEquals(newEntity.getEntityStatus(), QUALIFIED); assertEquals(newProperty.getEntityStatus(), QUALIFIED); }