From 193affdeed95ef1e5d1b138cfce6f387110e8859 Mon Sep 17 00:00:00 2001 From: Daniel <d.hornung@indiscale.com> Date: Tue, 28 Sep 2021 15:18:12 +0200 Subject: [PATCH] DOC STY: More comments, some style fixes. --- caosdb-proto | 2 +- .../server/entity/wrapper/Property.java | 2 ++ .../grpc/EntityTransactionServiceImpl.java | 20 ++++++----- .../server/transaction/WriteTransaction.java | 35 +++++++++++-------- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/caosdb-proto b/caosdb-proto index 75e826bd..640b1dd5 160000 --- a/caosdb-proto +++ b/caosdb-proto @@ -1 +1 @@ -Subproject commit 75e826bd318c39e63d324f71e035f08355ffc51f +Subproject commit 640b1dd5a085a3f1ed26880103da0148c017e28f diff --git a/src/main/java/org/caosdb/server/entity/wrapper/Property.java b/src/main/java/org/caosdb/server/entity/wrapper/Property.java index b3ce45b5..064e2bd3 100644 --- a/src/main/java/org/caosdb/server/entity/wrapper/Property.java +++ b/src/main/java/org/caosdb/server/entity/wrapper/Property.java @@ -50,6 +50,7 @@ public class Property extends EntityWrapper { super(new Entity()); } + /** Return the Property Index, the index of a property with respect to a containing Entity. */ public int getPIdx() { return this.pIdx; } @@ -58,6 +59,7 @@ public class Property extends EntityWrapper { private EntityInterface domain = null; private boolean isName; + /** Set the Property Index. */ public void setPIdx(final int i) { this.pIdx = i; } diff --git a/src/main/java/org/caosdb/server/grpc/EntityTransactionServiceImpl.java b/src/main/java/org/caosdb/server/grpc/EntityTransactionServiceImpl.java index 9b5b6fc1..7f5434df 100644 --- a/src/main/java/org/caosdb/server/grpc/EntityTransactionServiceImpl.java +++ b/src/main/java/org/caosdb/server/grpc/EntityTransactionServiceImpl.java @@ -607,6 +607,10 @@ public class EntityTransactionServiceImpl extends EntityTransactionServiceImplBa /** * Handle mixed-write transactions. * + * <p>The current implementation fails fast, without attempts to execute a single request, if + * there are requests with non-integer IDs. This will change in the near future, once string IDs + * are supported by the server. + * * @param requests * @return * @throws Exception @@ -618,11 +622,11 @@ public class EntityTransactionServiceImpl extends EntityTransactionServiceImplBa SecurityUtils.getSubject(), getTimestamp(), getSRID(), new HashMap<String, String>()); // put entities into the transaction object - for (final TransactionRequest sub_request : requests.getRequestsList()) { - switch (sub_request.getWrappedRequestsCase()) { + for (final TransactionRequest subRequest : requests.getRequestsList()) { + switch (subRequest.getWrappedRequestsCase()) { case INSERT_REQUEST: { - final InsertRequest insertRequest = sub_request.getInsertRequest(); + final InsertRequest insertRequest = subRequest.getInsertRequest(); final Entity insertEntity = insertRequest.getEntityRequest().getEntity(); final InsertEntity entity = @@ -635,7 +639,7 @@ public class EntityTransactionServiceImpl extends EntityTransactionServiceImplBa } break; case UPDATE_REQUEST: - final UpdateRequest updateRequest = sub_request.getUpdateRequest(); + final UpdateRequest updateRequest = subRequest.getUpdateRequest(); final Entity updateEntity = updateRequest.getEntityRequest().getEntity(); try { @@ -651,7 +655,7 @@ public class EntityTransactionServiceImpl extends EntityTransactionServiceImplBa } break; case DELETE_REQUEST: - final DeleteRequest deleteRequest = sub_request.getDeleteRequest(); + final DeleteRequest deleteRequest = subRequest.getDeleteRequest(); try { final DeleteEntity entity = new DeleteEntity(getId(deleteRequest.getId())); container.add(entity); @@ -664,7 +668,7 @@ public class EntityTransactionServiceImpl extends EntityTransactionServiceImplBa default: throw new CaosDBException( "Cannot process a " - + sub_request.getWrappedRequestsCase().name() + + subRequest.getWrappedRequestsCase().name() + " in a write request."); } } @@ -687,7 +691,6 @@ public class EntityTransactionServiceImpl extends EntityTransactionServiceImplBa .addResponsesBuilder() .setInsertResponse(InsertResponse.newBuilder().setIdResponse(idResponse)); } else if (entity instanceof UpdateEntity) { - builder .addResponsesBuilder() .setUpdateResponse(UpdateResponse.newBuilder().setIdResponse(idResponse)); @@ -704,6 +707,8 @@ public class EntityTransactionServiceImpl extends EntityTransactionServiceImplBa * Handle a request which contains string id (which cannot be converted to integer ids) and return * a response which has the "ENTITY_DOES_NOT_EXIST" error for all entities with affected ids. * + * <p>This does not attempt to execute a single request. + * * @param request * @return */ @@ -754,7 +759,6 @@ public class EntityTransactionServiceImpl extends EntityTransactionServiceImplBa + " in a write request."); } } - return builder.build(); } diff --git a/src/main/java/org/caosdb/server/transaction/WriteTransaction.java b/src/main/java/org/caosdb/server/transaction/WriteTransaction.java index 600e6ae5..daaf78ec 100644 --- a/src/main/java/org/caosdb/server/transaction/WriteTransaction.java +++ b/src/main/java/org/caosdb/server/transaction/WriteTransaction.java @@ -74,6 +74,7 @@ public class WriteTransaction extends Transaction<WritableContainer> super(container); } + /** Set if it is an error, if an Entity has no ID but just a name upon update. */ public void setNoIdIsError(final boolean noIdIsError) { this.noIdIsError = noIdIsError; } @@ -470,8 +471,8 @@ public class WriteTransaction extends Transaction<WritableContainer> // properties for (final Property newProperty : newEntity.getProperties()) { - // find corresponding oldProperty for this new property and make a - // diff. + // find corresponding oldProperty for this new property and make a diff (existing property, + // same property index in this entity, equal content?). final Property oldProperty = findOldEntity(newProperty, oldEntity.getProperties()); if (oldProperty != null) { // do not check again. @@ -538,25 +539,31 @@ public class WriteTransaction extends Transaction<WritableContainer> return needPermissions; } + /** + * Attempt to find a (sparse) entity among a list of entities. + * + * <p>If no match by ID can be found, matching by name is attempted next, but only if noIdIsError + * is false. + */ 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; + final EntityInterface newEntity, final List<T> oldEntities) { + if (newEntity.hasId()) { + for (final T oldEntity : oldEntities) { + if (Objects.equals(oldEntity.getId(), newEntity.getId())) { + return oldEntity; } } } 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; + newEntity.addError(ServerMessages.ENTITY_HAS_NO_ID); + newEntity.addInfo("On updates, always specify the id not just the name."); + } else if (newEntity.hasName()) { + for (final T oldEntity : oldEntities) { + if (oldEntity.getName().equals(newEntity.getName())) { + return oldEntity; } } } else { - newProperty.addError(ServerMessages.ENTITY_HAS_NO_NAME_OR_ID); + newEntity.addError(ServerMessages.ENTITY_HAS_NO_NAME_OR_ID); } return null; } -- GitLab