From 6e51b1a3be006ab20363f46ceadd41f2861dcebf Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Tue, 11 Jul 2023 13:04:13 +0200 Subject: [PATCH 01/11] test: add test for Share entity class - Updated Share class changing variables names and returning the Share object in the setters - Renamed getCreationAt to getCreatedAt in order to align it with other POJOs --- .../carbonio/files/dal/dao/ebean/Node.java | 2 +- .../carbonio/files/dal/dao/ebean/Share.java | 88 +++++++---------- .../impl/ebean/ShareRepositoryEbean.java | 3 +- .../datafetchers/ShareDataFetcher.java | 2 +- .../files/dal/dao/ebean/ShareTest.java | 99 +++++++++++++++++++ 5 files changed, 136 insertions(+), 58 deletions(-) create mode 100644 core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/ShareTest.java diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Node.java b/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Node.java index a67c5e2c..ce8db3fa 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Node.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Node.java @@ -26,7 +26,7 @@ /** *

Represents an Ebean {@link Node} entity that matches a record of the {@link * Files.Db.Tables#NODE} table.

- *

The implementation of constructors and setters should not care to check if the values in + *

The implementation of the constructor and setters should not care to check if the values in * input are valid or not because, when these methods are called, these controls * must be already done.

*/ diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Share.java b/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Share.java index 4f1be448..e388412d 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Share.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Share.java @@ -7,6 +7,7 @@ import com.zextras.carbonio.files.Files; import io.ebean.annotation.Cache; import java.util.Optional; +import javax.annotation.Nullable; import javax.persistence.Column; import javax.persistence.EmbeddedId; import javax.persistence.Entity; @@ -17,7 +18,7 @@ /** *

Represents an Ebean {@link Share} entity that matches a record of the {@link * Files.Db.Tables#SHARE} table.

- *

The implementation of constructors and setters should not care to check if the values in + *

The implementation of the constructor and setters should not care to check if the values in * input are valid or not because, when these methods are called, these controls * must be already done.

*/ @@ -27,123 +28,100 @@ public class Share { @EmbeddedId - private SharePK mComposedPrimaryKey; - - @Column(name = Files.Db.NodeCustomAttributes.NODE_ID, nullable = false) - private String mNodeId; + private final SharePK composedPrimaryKey; @Column(name = Files.Db.Share.PERMISSIONS) - private Short mPermissions; + private Short permissions; @Column(name = Files.Db.Share.CREATED_AT, nullable = false) - private long mCreatedAt; + private final Long createdAt; @Column(name = Files.Db.Share.EXPIRED_AT) - private Long mExpiredAt; + private Long expiredAt; @ManyToOne @JoinColumn(name = Files.Db.Share.NODE_ID, referencedColumnName = Files.Db.Node.ID, insertable = false, updatable = false) private Node node; @Column(name = Files.Db.Share.DIRECT, nullable = false) - private Boolean mDirect; + private Boolean direct; @Column(name = Files.Db.Share.CREATED_VIA_LINK, nullable = false) private Boolean createdViaLink; - /** - *

Creates a new {@link Share} entity.

- *

This constructor does not set the expiration timestamp of the share.

- * - * @param nodeId is a {@link String} of the node id. - * @param targetUserId is a {@link String} of the target user id which the node will be shared - * to. - * @param permissions is a {@link ACL} representing the permissions of this share. - * @param createdAt is a long of the creation timestamp. - * @param direct is a {@link Boolean} used for setting if the share is direct or inherited. - */ - public Share( - String nodeId, - String targetUserId, - ACL permissions, - long createdAt, - Boolean direct, - Boolean createdViaLink - ) { - this(nodeId, targetUserId, permissions, createdAt, direct, createdViaLink, null); - } - /** *

Creates a new {@link Share} entity.

* * @param nodeId is a {@link String} of the node id. * @param targetUserId is a {@link String} of the target user id which the node will be shared * to. - * @param permissions is a {@link ACL} representing the permissions of this share. - * @param createdAt is a long of the creation timestamp. - * @param direct is a {@link Boolean} used for setting if the share is direct or inherited. - * @param expiredAt is a {@link Long} of the expiration timestamp. + * @param permissions is an {@link ACL} representing the permissions of the share. + * @param createdAt is a {@link Long} of the creation timestamp. + * @param direct is a {@link Boolean} used to set if the share is direct or inherited. + * @param expiredAt is a {@link Long} of the expiration timestamp. It could be nullable. */ public Share( String nodeId, String targetUserId, ACL permissions, - long createdAt, + Long createdAt, Boolean direct, Boolean createdViaLink, - Long expiredAt + @Nullable Long expiredAt ) { - mComposedPrimaryKey = new SharePK(nodeId, targetUserId); - mPermissions = permissions.encode(); - mCreatedAt = createdAt; - mDirect = direct; + this.composedPrimaryKey = new SharePK(nodeId, targetUserId); + this.permissions = permissions.encode(); + this.createdAt = createdAt; + this.direct = direct; this.createdViaLink = createdViaLink; - mExpiredAt = expiredAt; + this.expiredAt = expiredAt; } public String getNodeId() { - return mComposedPrimaryKey.getNodeId(); + return composedPrimaryKey.getNodeId(); } public String getTargetUserId() { - return mComposedPrimaryKey.getTargetUserId(); + return composedPrimaryKey.getTargetUserId(); } public ACL getPermissions() { - return ACL.decode(mPermissions); + return ACL.decode(permissions); } public Share setPermissions(ACL permissions) { - mPermissions = permissions.encode(); + this.permissions = permissions.encode(); return this; } - public long getCreationAt() { - return mCreatedAt; + public long getCreatedAt() { + return createdAt; } public Optional getExpiredAt() { - return Optional.ofNullable(mExpiredAt); + return Optional.ofNullable(expiredAt); } public Share setExpiredAt(long expiredAt) { - mExpiredAt = expiredAt; + this.expiredAt = expiredAt; return this; } public Boolean isDirect() { - return mDirect; + return direct; } - public void setDirect(Boolean mDirect) { - this.mDirect = mDirect; + public Share setDirect(Boolean mDirect) { + this.direct = mDirect; + return this; } - public Boolean getCreatedViaLink() { + public Boolean isCreatedViaLink() { return createdViaLink; } - public void setCreatedViaLink(Boolean createdViaLink) { + public Share setCreatedViaLink(Boolean createdViaLink) { this.createdViaLink = createdViaLink; + return this; } } diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java index 16c93caf..76d70fd7 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java @@ -85,7 +85,8 @@ public Optional upsertShare( permissions, System.currentTimeMillis(), direct, - createdViaCollaborationLink + createdViaCollaborationLink, + null ); expireTimestamp.ifPresent(share::setExpiredAt); mDB.getEbeanDatabase().save(share); diff --git a/core/src/main/java/com/zextras/carbonio/files/graphql/datafetchers/ShareDataFetcher.java b/core/src/main/java/com/zextras/carbonio/files/graphql/datafetchers/ShareDataFetcher.java index 59bb6fc6..82070011 100644 --- a/core/src/main/java/com/zextras/carbonio/files/graphql/datafetchers/ShareDataFetcher.java +++ b/core/src/main/java/com/zextras/carbonio/files/graphql/datafetchers/ShareDataFetcher.java @@ -70,7 +70,7 @@ public ShareDataFetcher( private DataFetcherResult> convertShareToDataFetcherResult(Share share) { Map shareContext = new HashMap<>(); Map result = new HashMap<>(); - result.put(Files.GraphQL.Share.CREATED_AT, share.getCreationAt()); + result.put(Files.GraphQL.Share.CREATED_AT, share.getCreatedAt()); result.put(Files.GraphQL.Share.PERMISSION, share.getPermissions().getSharePermission()); share .getExpiredAt() diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/ShareTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/ShareTest.java new file mode 100644 index 00000000..6ee4f453 --- /dev/null +++ b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/ShareTest.java @@ -0,0 +1,99 @@ +// SPDX-FileCopyrightText: 2023 Zextras +// +// SPDX-License-Identifier: AGPL-3.0-only + +package com.zextras.carbonio.files.dal.dao.ebean; + +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; + +class ShareTest { + + @Test + void givenAllShareAttributesTheConstructorShouldCreateShareObjectCorrectly() { + // Given & When + Share share = new Share( + "868b43cc-3a8f-4c14-a66d-f520d8e7e8bd", + "c6bf990d-86b9-49ad-a6c0-12260308b7c5", + ACL.decode(ACL.READ), + 5L, + true, + true, + 10L + ); + + // Then + Assertions.assertThat(share.getNodeId()).isEqualTo("868b43cc-3a8f-4c14-a66d-f520d8e7e8bd"); + Assertions.assertThat(share.getTargetUserId()).isEqualTo("c6bf990d-86b9-49ad-a6c0-12260308b7c5"); + Assertions.assertThat(share.getPermissions()).isEqualTo(ACL.decode(ACL.READ)); + Assertions.assertThat(share.getCreatedAt()).isEqualTo(5L); + Assertions.assertThat(share.isDirect()).isTrue(); + Assertions.assertThat(share.isCreatedViaLink()).isTrue(); + Assertions.assertThat(share.getExpiredAt()).contains(10L); + } + + @Test + void givenDifferentShareAttributesTheSettersShouldUpdateShareObjectCorrectly() { + // Given + Share share = new Share( + "868b43cc-3a8f-4c14-a66d-f520d8e7e8bd", + "c6bf990d-86b9-49ad-a6c0-12260308b7c5", + ACL.decode(ACL.SHARE), + 5L, + true, + true, + null + ); + + // When + share + .setDirect(false) + .setExpiredAt(20L) + .setPermissions(ACL.decode(ACL.OWNER)) + .setCreatedViaLink(false); + + // Then + Assertions.assertThat(share.getNodeId()).isEqualTo("868b43cc-3a8f-4c14-a66d-f520d8e7e8bd"); + Assertions.assertThat(share.getTargetUserId()).isEqualTo("c6bf990d-86b9-49ad-a6c0-12260308b7c5"); + Assertions.assertThat(share.getPermissions()).isEqualTo(ACL.decode(ACL.OWNER)); + Assertions.assertThat(share.getCreatedAt()).isEqualTo(5L); + Assertions.assertThat(share.isDirect()).isFalse(); + Assertions.assertThat(share.isCreatedViaLink()).isFalse(); + Assertions.assertThat(share.getExpiredAt()).contains(20L); + } + + + @Test + void givenANullExpiredAtTheGetExpiredAtShouldReturnAnOptionalEmpty() { + // Given & When + Share share = new Share( + "868b43cc-3a8f-4c14-a66d-f520d8e7e8bd", + "c6bf990d-86b9-49ad-a6c0-12260308b7c5", + ACL.decode(ACL.SHARE), + 5L, + true, + true, + null + ); + + // Then + Assertions.assertThat(share.getExpiredAt()).isEmpty(); + } + + @Test + void givenNodeIdAndUserIdTheSharePkConstructorShouldCreateSharePkObjectCorrectly() { + // Given & When + NodeCustomAttributesPK nodeCustomAttributesPK = new NodeCustomAttributesPK( + "868b43cc-3a8f-4c14-a66d-f520d8e7e8bd", + "c6bf990d-86b9-49ad-a6c0-12260308b7c5" + ); + + // Then + Assertions + .assertThat(nodeCustomAttributesPK.getNodeId()) + .isEqualTo("868b43cc-3a8f-4c14-a66d-f520d8e7e8bd"); + Assertions + .assertThat(nodeCustomAttributesPK.getUserId()) + .isEqualTo("c6bf990d-86b9-49ad-a6c0-12260308b7c5"); + } +} From b82b8224b81b5c2c50f2ed7dfefa47474ee9af59 Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Tue, 11 Jul 2023 14:57:19 +0200 Subject: [PATCH 02/11] test: add test for CollaborationLink entity class - Removed the @WhenCreated annotation and added the createdAt parameter in the constructor - Updated the CollaborationLinkRepositoryEbean#createLink passing the creation timestamp to the CollaborationLink constructor using the Clock --- .../dal/dao/ebean/CollaborationLink.java | 48 ++++++++++++++++++- .../CollaborationLinkRepositoryEbean.java | 9 +++- .../dal/dao/ebean/CollaborationLinkTest.java | 43 +++++++++++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLinkTest.java diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLink.java b/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLink.java index 6e72b70f..e7a9ded8 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLink.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLink.java @@ -4,10 +4,10 @@ package com.zextras.carbonio.files.dal.dao.ebean; +import com.zextras.carbonio.files.Files; import com.zextras.carbonio.files.Files.Db; import com.zextras.carbonio.files.Files.Db.Tables; import com.zextras.carbonio.files.dal.dao.ebean.ACL.SharePermission; -import io.ebean.annotation.WhenCreated; import java.time.Instant; import java.util.UUID; import javax.persistence.Column; @@ -15,6 +15,21 @@ import javax.persistence.Id; import javax.persistence.Table; +/** + *

Represents an Ebean {@link CollaborationLink} entity that matches a record of the {@link + * Files.Db.Tables#COLLABORATION_LINK} table.

+ *

The collaboration link has properties mapped to the corresponding table columns:

+ *
    + *
  • {@code id}: The unique identifier of the link.
    • + *
  • {@code nodeId}: The identifier of the associated node.
    • + *
  • {@code invitationId}: The public identifier of the url link.
    • + *
  • {@code createdAt}: The timestamp indicating when the link was created.
    • + *
  • {@code permissions}: The permissions assigned to the link necessary to create the share + * between the user that clicked on the link and the related node.
    • + *
+ *

The constructor should not care to check if the values in input are valid or not because + * these controls must be already done before calling the constructor.

+ */ @Entity @Table(name = Tables.COLLABORATION_LINK) public class CollaborationLink { @@ -29,41 +44,70 @@ public class CollaborationLink { @Column(name = Db.CollaborationLink.INVITATION_ID, nullable = false, length = 8) private String invitationId; - @WhenCreated @Column(name = Db.CollaborationLink.CREATED_AT, nullable = false) private Instant createdAt; @Column(name = Db.CollaborationLink.PERMISSIONS, nullable = false) private short permissions; + /** + * Creates a new {@link CollaborationLink} entity that can be saved in the database. + * + * @param linkId is a {@link UUID} representing the collaboration link identifier. + * @param nodeId is a {@link String} representing the {@link Node} identifier associated to + * the collaboration link. + * @param invitationId is a {@link String} representing the public identifier of the URL link. + * @param createdAt is an {@link Instant} of the link creation timestamp. + * @param permissions is a short representing the permission necessary to create the + * {@link Share} between the user that used the link and the related node. + */ public CollaborationLink( UUID linkId, String nodeId, String invitationId, + Instant createdAt, short permissions ) { this.id = linkId; this.nodeId = nodeId; this.invitationId = invitationId; + this.createdAt = createdAt; this.permissions = permissions; } + /** + * @return an {@link UUID} representing the unique identifier of the collaboration link. + */ public UUID getId() { return id; } + /** + * @return a {@link String} representing the identifier of the associated node. + */ public String getNodeId() { return nodeId; } + /** + * @return a {@link String} of 8 alphanumeric characters representing the invitation + * identifier used to build the complete URL. + */ public String getInvitationId() { return invitationId; } + /** + * @return an {@link Instant} representing the creation timestamp of the collaboration link. + */ public Instant getCreatedAt() { return createdAt; } + /** + * @return a {@link SharePermission} representing the permission necessary to create the share + * between the user that used the link and the related node. + */ public SharePermission getPermissions() { return ACL.decode(permissions).getSharePermission(); } diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java index 03c0a948..826aff01 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java @@ -10,6 +10,7 @@ import com.zextras.carbonio.files.dal.dao.ebean.ACL.SharePermission; import com.zextras.carbonio.files.dal.dao.ebean.CollaborationLink; import com.zextras.carbonio.files.dal.repositories.interfaces.CollaborationLinkRepository; +import java.time.Clock; import java.util.Collection; import java.util.Optional; import java.util.UUID; @@ -17,10 +18,15 @@ public class CollaborationLinkRepositoryEbean implements CollaborationLinkRepository { + private final Clock clock; private final EbeanDatabaseManager ebeanDatabaseManager; @Inject - public CollaborationLinkRepositoryEbean(EbeanDatabaseManager ebeanDatabaseManager) { + public CollaborationLinkRepositoryEbean( + Clock clock, + EbeanDatabaseManager ebeanDatabaseManager + ) { + this.clock = clock; this.ebeanDatabaseManager = ebeanDatabaseManager; } @@ -35,6 +41,7 @@ public CollaborationLink createLink( linkId, nodeId, invitationId, + clock.instant(), permissions.encode() ); diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLinkTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLinkTest.java new file mode 100644 index 00000000..6da13b18 --- /dev/null +++ b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLinkTest.java @@ -0,0 +1,43 @@ +// SPDX-FileCopyrightText: 2023 Zextras +// +// SPDX-License-Identifier: AGPL-3.0-only + +package com.zextras.carbonio.files.dal.dao.ebean; + +import com.zextras.carbonio.files.dal.dao.ebean.ACL.SharePermission; +import java.time.Instant; +import java.util.UUID; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; + +public class CollaborationLinkTest { + + @Test + void givenAllCollaborationLinkAttributesTheConstructorShouldCreateCollaborationLinkObjectCorrectly() { + // Given & When + CollaborationLink collaborationLink = new CollaborationLink( + UUID.fromString("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5"), + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + Instant.ofEpochMilli(100L), + ACL.READ + ); + + // Then + Assertions + .assertThat(collaborationLink.getId()) + .isEqualTo(UUID.fromString("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5")); + Assertions + .assertThat(collaborationLink.getNodeId()) + .isEqualTo("ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c"); + Assertions + .assertThat(collaborationLink.getInvitationId()) + .isEqualTo("abcd1234"); + Assertions + .assertThat(collaborationLink.getCreatedAt()) + .isEqualTo(Instant.ofEpochMilli(100L)); + Assertions + .assertThat(collaborationLink.getPermissions()) + .isEqualTo(SharePermission.READ_ONLY); + } +} From 69fcced0fbff46b07a39eee404bb71659e2bc203 Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Wed, 12 Jul 2023 19:38:15 +0200 Subject: [PATCH 03/11] test: add test for Link entity class and add javadoc --- .../carbonio/files/dal/dao/ebean/Link.java | 125 +++++++++--------- .../impl/ebean/LinkRepositoryEbean.java | 4 +- .../files/dal/dao/ebean/LinkTest.java | 125 ++++++++++++++++++ 3 files changed, 193 insertions(+), 61 deletions(-) create mode 100644 core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/LinkTest.java diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Link.java b/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Link.java index b7fc4212..2ed68be5 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Link.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/dao/ebean/Link.java @@ -7,6 +7,7 @@ import com.zextras.carbonio.files.Files; import com.zextras.carbonio.files.Files.Db; import java.util.Optional; +import javax.annotation.Nullable; import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.Id; @@ -15,9 +16,17 @@ /** *

Represents an Ebean {@link Link} entity that matches a record of the {@link Files.Db.Link} * table.

- *

The implementation of constructors and setters should not care to check if the values in - * input are valid or not because, when these methods are called, these controls - * must be already done.

+ *

The public link has properties mapped to the corresponding table columns:

+ *
    + *
  • {@code id}: The unique identifier of the link.
    • + *
  • {@code nodeId}: The identifier of the associated node.
    • + *
  • {@code publicId}: The public identifier of the URL link.
    • + *
  • {@code createdAt}: The timestamp indicating when the link was created.
    • + *
  • {@code expiresAt}: The timestamp indicating when the link should expire.
    • + *
  • {@code description}: A small description of the link (maximum 300 characters).
    • + *
+ *

The constructor and setters should not care to check if the values in input are valid or not + * because, when these methods are called, these controls must be already done.

*/ @Entity @Table(name = Files.Db.Tables.LINK) @@ -25,97 +34,77 @@ public class Link { @Id @Column(name = Db.Link.ID, nullable = false, length = 36) - private String mId; + private String id; @Column(name = Db.Link.NODE_ID, nullable = false, length = 36) - private String mNodeId; + private String nodeId; @Column(name = Db.Link.PUBLIC_ID, nullable = false) - private String mPublicId; + private String publicId; @Column(name = Files.Db.Link.CREATED_AT, nullable = false) - private Long mCreatedAt; + private Long createdAt; @Column(name = Files.Db.Link.EXPIRES_AT) - private Long mExpiresAt; + private Long expiresAt; @Column(name = Db.Link.DESCRIPTION, length = 300) - private String mDescription; - - - /** - *

Creates a new {@link Link} entity that can be saved in the database.

- *

This constructor does not set the expiration timestamp and the description of the link.

- * - * @param linkId is a {@link String} of the unique identifier of the link. - * @param nodeId is a {@link String} of the related node id. - * @param publicId is a {@link String} of the public id used to build the complete url. - * @param createdAt is a {@link Long} of the creation timestamp. - */ - public Link( - String linkId, - String nodeId, - String publicId, - Long createdAt - ) { - mId = linkId; - mNodeId = nodeId; - mPublicId = publicId; - mCreatedAt = createdAt; - } + private String description; /** * Creates a new {@link Link} entity that can be saved in the database. * - * @param linkId is a {@link String} of the unique identifier of the link. - * @param nodeId is a {@link String} of the related node id. - * @param publicId is a {@link String} of the public id used to build the complete url. - * @param createdAt is a {@link Long} of the creation timestamp. - * @param expiresAt is a {@link Long} of the expiration timestamp. - * @param description is a {@link String} of the link description. + * @param linkId is a {@link String} representing the unique identifier of the link. + * @param nodeId is a {@link String} representing the {@link Node} identifier associated to + * the public link. + * @param publicId is a {@link String} representing the public identifier of the URL link. + * @param createdAt is a {@link Long} of the link creation timestamp. + * @param expiresAt is a {@link Long} of the link expiration timestamp. It can be nullable. + * @param description is a {@link String} of the link description. It can be nullable. */ public Link( String linkId, String nodeId, String publicId, Long createdAt, - Long expiresAt, - String description + @Nullable Long expiresAt, + @Nullable String description ) { - mId = linkId; - mNodeId = nodeId; - mPublicId = publicId; - mCreatedAt = createdAt; - mExpiresAt = expiresAt; - mDescription = description; + id = linkId; + this.nodeId = nodeId; + this.publicId = publicId; + this.createdAt = createdAt; + this.expiresAt = expiresAt; + this.description = description; } /** - * @return a {@link String} representing the link id. + * @return a {@link String} representing the unique identifier of the public link. */ public String getLinkId() { - return mId; + return id; } /** - * @return a {@link String} representing the id of the related node. + * @return a {@link String} representing the identifier of the associated node. */ public String getNodeId() { - return mNodeId; + return nodeId; } /** - * @return a {@link String} of the public id used to build the complete url. + * @return a {@link String} of 8 alphanumeric characters representing the public + * identifier used to build the complete URL. */ public String getPublicId() { - return mPublicId; + return publicId; } /** - * @return a {@link Long} representing the creation timestamp of the link. + * @return a {@link Long} representing the creation timestamp of the public link. */ public Long getCreatedAt() { - return mCreatedAt; + return createdAt; } /** @@ -123,15 +112,24 @@ public Long getCreatedAt() { * the link, if exists. */ public Optional getExpiresAt() { - return Optional.ofNullable(mExpiresAt); + return Optional.ofNullable(expiresAt); } - public void setExpiresAt(Long expiresAt) { + /** + * Allows to set/unset the expiration timestamp of the existing link. If the timestamp is equal to + * zero than the expiration is disabled and the public link will not expire. + * + * @param expiresAt is a {@link Long} representing the expiration timestamp. + * @return the current {@link Link}. + */ + public Link setExpiresAt(Long expiresAt) { if (expiresAt == 0) { - mExpiresAt = null; + this.expiresAt = null; } else { - mExpiresAt = expiresAt; + this.expiresAt = expiresAt; } + + return this; } /** @@ -139,10 +137,17 @@ public void setExpiresAt(Long expiresAt) { * exists. */ public Optional getDescription() { - return Optional.ofNullable(mDescription); + return Optional.ofNullable(description); } - public void setDescription(String description) { - mDescription = description; + /** + * Allows to add/change the description of the existing public link. + * + * @param description is a {@link String} of the link description. + * @return the current {@link Link}. + */ + public Link setDescription(String description) { + this.description = description; + return this; } } diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java index a8d02198..6474a63b 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java @@ -40,7 +40,9 @@ public Link createLink( linkId, nodeId, publicId, - System.currentTimeMillis() + System.currentTimeMillis(), + null, + null ); optExpiresAt.ifPresent(link::setExpiresAt); diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/LinkTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/LinkTest.java new file mode 100644 index 00000000..dfb3ca6c --- /dev/null +++ b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/LinkTest.java @@ -0,0 +1,125 @@ +// SPDX-FileCopyrightText: 2023 Zextras +// +// SPDX-License-Identifier: AGPL-3.0-only + +package com.zextras.carbonio.files.dal.dao.ebean; + +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; + +public class LinkTest { + + @Test + void givenAllPublicLinkAttributesTheConstructorShouldCreateLinkObjectCorrectly() { + // Given & When + Link publicLink = new Link( + "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + 5L, + 10L, + "fake description" + ); + + // Then + Assertions + .assertThat(publicLink.getLinkId()) + .isEqualTo("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5"); + Assertions + .assertThat(publicLink.getNodeId()) + .isEqualTo("ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c"); + Assertions + .assertThat(publicLink.getPublicId()) + .isEqualTo("abcd1234"); + Assertions + .assertThat(publicLink.getCreatedAt()) + .isEqualTo(5L); + Assertions + .assertThat(publicLink.getExpiresAt()) + .isPresent() + .get() + .isEqualTo(10L); + Assertions + .assertThat(publicLink.getDescription()) + .isPresent() + .get() + .isEqualTo("fake description"); + } + + @Test + void givenPartialPublicLinkAttributesTheSettersShouldUpdateLinkObjectCorrectly() { + // Given + Link publicLink = new Link( + "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + 5L, + null, + null + ); + + // When + publicLink + .setExpiresAt(10L) + .setDescription("fake description"); + + // Then + Assertions + .assertThat(publicLink.getLinkId()) + .isEqualTo("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5"); + Assertions + .assertThat(publicLink.getNodeId()) + .isEqualTo("ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c"); + Assertions + .assertThat(publicLink.getPublicId()) + .isEqualTo("abcd1234"); + Assertions + .assertThat(publicLink.getCreatedAt()) + .isEqualTo(5L); + Assertions + .assertThat(publicLink.getExpiresAt()) + .isPresent() + .get() + .isEqualTo(10L); + Assertions + .assertThat(publicLink.getDescription()) + .isPresent() + .get() + .isEqualTo("fake description"); + } + + @Test + void givenANullExpiredAtTheGetExpiredAtShouldReturnAnOptionalEmpty() { + // Given & When + Link publicLink = new Link( + "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + 5L, + null, + null + ); + + // Then + Assertions.assertThat(publicLink.getExpiresAt()).isEmpty(); + } + + @Test + void givenAnExpirationTimestampEqualsToZeroTheGetExpiredAtShouldReturnAnOptionalEmpty() { + // Given + Link publicLink = new Link( + "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + 5L, + 10L, + null + ); + + // When + publicLink.setExpiresAt(0L); + + // Then + Assertions.assertThat(publicLink.getExpiresAt()).isEmpty(); + } +} From 5065d71d1e395ec6338e8b831a59cdf6411c2a46 Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Mon, 6 Nov 2023 15:54:19 +0100 Subject: [PATCH 04/11] chore: update language-formatters-pre-commit-hooks to 2.11.0 --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1375d193..1c3ed643 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -43,7 +43,7 @@ repos: - id: reuse - repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks - rev: v2.10.0 + rev: v2.11.0 hooks: - id: pretty-format-java From c7817b74f9982151bdfc190be099ca50096641a3 Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Mon, 6 Nov 2023 16:05:18 +0100 Subject: [PATCH 05/11] chore(format): reformat CollaborationLinkRepositoryEbean class --- .../CollaborationLinkRepositoryEbean.java | 67 ++++++++----------- 1 file changed, 27 insertions(+), 40 deletions(-) diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java index 826aff01..df56c2b1 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java @@ -22,32 +22,19 @@ public class CollaborationLinkRepositoryEbean implements CollaborationLinkReposi private final EbeanDatabaseManager ebeanDatabaseManager; @Inject - public CollaborationLinkRepositoryEbean( - Clock clock, - EbeanDatabaseManager ebeanDatabaseManager - ) { + public CollaborationLinkRepositoryEbean(Clock clock, EbeanDatabaseManager ebeanDatabaseManager) { this.clock = clock; this.ebeanDatabaseManager = ebeanDatabaseManager; } @Override public CollaborationLink createLink( - UUID linkId, - String nodeId, - String invitationId, - SharePermission permissions - ) { - CollaborationLink collaborationLink = new CollaborationLink( - linkId, - nodeId, - invitationId, - clock.instant(), - permissions.encode() - ); + UUID linkId, String nodeId, String invitationId, SharePermission permissions) { - ebeanDatabaseManager - .getEbeanDatabase() - .insert(collaborationLink); + CollaborationLink collaborationLink = + new CollaborationLink(linkId, nodeId, invitationId, clock.instant(), permissions.encode()); + + ebeanDatabaseManager.getEbeanDatabase().insert(collaborationLink); return getLinkById(linkId).get(); } @@ -55,41 +42,41 @@ public CollaborationLink createLink( @Override public Optional getLinkById(UUID linkId) { return ebeanDatabaseManager - .getEbeanDatabase() - .find(CollaborationLink.class) - .where() - .idEq(linkId) - .findOneOrEmpty(); + .getEbeanDatabase() + .find(CollaborationLink.class) + .where() + .idEq(linkId) + .findOneOrEmpty(); } @Override public Optional getLinkByInvitationId(String invitationId) { return ebeanDatabaseManager - .getEbeanDatabase() - .find(CollaborationLink.class) - .where() - .eq(Files.Db.CollaborationLink.INVITATION_ID, invitationId) - .findOneOrEmpty(); + .getEbeanDatabase() + .find(CollaborationLink.class) + .where() + .eq(Files.Db.CollaborationLink.INVITATION_ID, invitationId) + .findOneOrEmpty(); } @Override public Stream getLinksByNodeId(String nodeId) { return ebeanDatabaseManager - .getEbeanDatabase() - .find(CollaborationLink.class) - .where() - .eq(Files.Db.CollaborationLink.NODE_ID, nodeId) - .findList() - .stream(); + .getEbeanDatabase() + .find(CollaborationLink.class) + .where() + .eq(Files.Db.CollaborationLink.NODE_ID, nodeId) + .findList() + .stream(); } @Override public void deleteLinks(Collection linkIds) { ebeanDatabaseManager - .getEbeanDatabase() - .find(CollaborationLink.class) - .where() - .idIn(linkIds) - .delete(); + .getEbeanDatabase() + .find(CollaborationLink.class) + .where() + .idIn(linkIds) + .delete(); } } From 4d306cc95cd54b0ed88c90049f8d81829be993d7 Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Mon, 6 Nov 2023 16:09:23 +0100 Subject: [PATCH 06/11] test: add UTs and javadoc for the CollaborationLinkRepositoryEbean class --- .../CollaborationLinkRepositoryEbean.java | 2 +- .../CollaborationLinkRepository.java | 66 ++++- .../CollaborationLinkRepositoryEbeanTest.java | 247 ++++++++++++++++++ 3 files changed, 306 insertions(+), 9 deletions(-) create mode 100644 core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbeanTest.java diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java index df56c2b1..6a41e2f7 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbean.java @@ -36,7 +36,7 @@ public CollaborationLink createLink( ebeanDatabaseManager.getEbeanDatabase().insert(collaborationLink); - return getLinkById(linkId).get(); + return collaborationLink; } @Override diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/CollaborationLinkRepository.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/CollaborationLinkRepository.java index 0951557d..ae4ad325 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/CollaborationLinkRepository.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/CollaborationLinkRepository.java @@ -12,24 +12,74 @@ import java.util.stream.Stream; /** - *

This is the only class allowed to execute CRUD operations on a {@link CollaborationLink} - * element. In particular it can create a new {@link CollaborationLink}, update it, access to and - * delete an existing one.

+ * This is the only class allowed to execute CRUD operations on a {@link CollaborationLink} element. + * In particular, it can create a new {@link CollaborationLink}, access it and delete an existing + * one. */ public interface CollaborationLinkRepository { + /** + * Creates a new {@link CollaborationLink} and, after saving it in the database, it returns the + * {@link CollaborationLink} just created. + * + *

This method considers the parameters in input already valid so it does not do any kind of + * control on them. + * + * @param linkId is an {@link UUID} representing the internal unique identifier of the + * collaboration link to create + * @param nodeId is a {@link String} representing the unique identifier of the node which the link + * is created to + * @param invitationId is a {@link String} of 8 characters representing the public identifier + * (hash) of the collaboration link to create + * @param permissions is a {@link SharePermission} representing the permission rights that the + * share will be created when this collaboration link will be used + * @return the {@link CollaborationLink} just created and saved in the database. + */ CollaborationLink createLink( - UUID linkId, - String nodeId, - String invitationId, - SharePermission permissions - ); + UUID linkId, String nodeId, String invitationId, SharePermission permissions); + /** + * Given an internal collaboration link identifier, it allows to retrieve a {@link + * CollaborationLink} from the database if it exists. + * + * @param linkId is a {@link UUID} representing the unique identifier of the collaboration link to + * retrieve + * @return an {@link Optional} containing the requested {@link CollaborationLink} if it exists, + * otherwise it returns an {@link Optional#empty()}. + */ Optional getLinkById(UUID linkId); + /** + * Given a public invitation link identifier, it allows to retrieve a {@link CollaborationLink} + * from the database if it exists. + * + * @param invitationId is a {@link String} of 8 characters representing the public identifier + * (hash) of the collaboration link to retrieve + * @return an {@link Optional} containing the requested {@link CollaborationLink} if it exists, + * otherwise it returns an {@link Optional#empty()}. + */ Optional getLinkByInvitationId(String invitationId); + /** + * Given a node identifier, it allows to retrieve a stream of {@link CollaborationLink} from the + * database if there are links associated to the related node. + * + * @param nodeId is a {@link String} representing the unique identifier of the node which a list + * of links are associated to + * @return a {@link Stream} containing the requested {@link CollaborationLink}s if they exists, + * otherwise it returns an empty {@link Stream}. + */ Stream getLinksByNodeId(String nodeId); + /** + * Given a collection of identifiers, it deletes the existing {@link CollaborationLink}s from the + * database. + * + *

if a collaboration link identifier is not associated to a {@link CollaborationLink} then the + * delete operation for that specific identifier does nothing. + * + * @param linkIds is a {@link Collection} of {@link UUID}s representing the unique identifier of + * collaboration links to delete + */ void deleteLinks(Collection linkIds); } diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbeanTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbeanTest.java new file mode 100644 index 00000000..5b75ab46 --- /dev/null +++ b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbeanTest.java @@ -0,0 +1,247 @@ +// SPDX-FileCopyrightText: 2023 Zextras +// +// SPDX-License-Identifier: AGPL-3.0-only + +package com.zextras.carbonio.files.dal.repositories.impl.ebean; + +import com.zextras.carbonio.files.dal.EbeanDatabaseManager; +import com.zextras.carbonio.files.dal.dao.ebean.ACL.SharePermission; +import com.zextras.carbonio.files.dal.dao.ebean.CollaborationLink; +import com.zextras.carbonio.files.dal.repositories.interfaces.CollaborationLinkRepository; +import io.ebean.Database; +import io.ebean.ExpressionList; +import java.time.Clock; +import java.time.Instant; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Stream; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +public class CollaborationLinkRepositoryEbeanTest { + + private CollaborationLinkRepository collaborationLinkRepository; + private Database ebeanDatabaseMock; + private Clock clockMock; + + private static Stream collaborationLinksProvider() { + return Stream.of( + Arguments.of("8f89bae5-6b92-4be7-bc25-5014094d1a63", Collections.emptyList(), 0), + Arguments.of( + "0293d9c6-4600-417b-becc-7f1418141c98", + Collections.singletonList(Mockito.mock(CollaborationLink.class)), + 1)); + } + + @BeforeEach + void setup() { + ebeanDatabaseMock = Mockito.mock(Database.class, Mockito.RETURNS_DEEP_STUBS); + EbeanDatabaseManager ebeanDatabaseManagerMock = Mockito.mock(EbeanDatabaseManager.class); + Mockito.when(ebeanDatabaseManagerMock.getEbeanDatabase()).thenReturn(ebeanDatabaseMock); + clockMock = Mockito.mock(Clock.class); + collaborationLinkRepository = + new CollaborationLinkRepositoryEbean(clockMock, ebeanDatabaseManagerMock); + } + + @Test + void givenAllCollaborationLinkAttributesTheCreateLinkShouldReturnACollaborationLink() { + // Given + Mockito.when(clockMock.instant()).thenReturn(Instant.ofEpochMilli(1)); + + // When + CollaborationLink createdCollaborationLink = + collaborationLinkRepository.createLink( + UUID.fromString("00000000-0000-0000-0000-000000000000"), + "11111111-1111-1111-1111-111111111111", + "1234abcd", + SharePermission.READ_AND_WRITE); + + // Then + ArgumentCaptor linkCaptor = ArgumentCaptor.forClass(CollaborationLink.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).insert(linkCaptor.capture()); + + CollaborationLink savedCollaborationLink = linkCaptor.getValue(); + + Assertions.assertThat(createdCollaborationLink).isEqualTo(savedCollaborationLink); + Assertions.assertThat(createdCollaborationLink.getId()) + .isEqualTo(UUID.fromString("00000000-0000-0000-0000-000000000000")); + Assertions.assertThat(createdCollaborationLink.getNodeId()) + .isEqualTo("11111111-1111-1111-1111-111111111111"); + Assertions.assertThat(createdCollaborationLink.getInvitationId()).isEqualTo("1234abcd"); + Assertions.assertThat(createdCollaborationLink.getCreatedAt()) + .isEqualTo(Instant.ofEpochMilli(1)); + Assertions.assertThat(createdCollaborationLink.getPermissions()) + .isEqualTo(SharePermission.READ_AND_WRITE); + } + + @Test + void givenAnExistingCollaborationLinkIdTheGetLinkByIdShouldReturnTheRelatedCollaborationLink() { + // Given + CollaborationLink collaborationLinkMock = Mockito.mock(CollaborationLink.class); + Mockito.when( + ebeanDatabaseMock + .find(CollaborationLink.class) + .where() + .idEq(UUID.fromString("00000000-0000-0000-0000-000000000000")) + .findOneOrEmpty()) + .thenReturn(Optional.of(collaborationLinkMock)); + + // When + Optional optCollaborationLink = + collaborationLinkRepository.getLinkById( + UUID.fromString("00000000-0000-0000-0000-000000000000")); + + // Then + Assertions.assertThat(optCollaborationLink).isPresent().contains(collaborationLinkMock); + } + + @Test + void givenANotExistingCollaborationLinkIdTheGetLinkByIdShouldReturnOptionEmpty() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(CollaborationLink.class) + .where() + .idEq(UUID.fromString("00000000-0000-0000-0000-000000000000")) + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + Optional optCollaborationLink = + collaborationLinkRepository.getLinkById( + UUID.fromString("00000000-0000-0000-0000-000000000000")); + + // Then + Assertions.assertThat(optCollaborationLink).isNotPresent(); + } + + @Test + void + givenAnExistingInvitationIdTheGetLinkByInvitationIdShouldReturnTheRelatedCollaborationLink() { + // Given + CollaborationLink collaborationLinkMock = Mockito.mock(CollaborationLink.class); + Mockito.when( + ebeanDatabaseMock + .find(CollaborationLink.class) + .where() + .eq("invitation_id", "1234abcd") + .findOneOrEmpty()) + .thenReturn(Optional.of(collaborationLinkMock)); + + // When + Optional optCollaborationLink = + collaborationLinkRepository.getLinkByInvitationId("1234abcd"); + + // Then + Assertions.assertThat(optCollaborationLink).isPresent().contains(collaborationLinkMock); + } + + @Test + void givenANotExistingInvitationIdTheGetLinkByInvitationIdShouldReturnOptionEmpty() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(CollaborationLink.class) + .where() + .eq("invitation_id", "1234abcd") + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + Optional optCollaborationLink = + collaborationLinkRepository.getLinkByInvitationId("1234abcd"); + + // Then + Assertions.assertThat(optCollaborationLink).isNotPresent(); + } + + @ParameterizedTest + @MethodSource("collaborationLinksProvider") + void givenAnExistingNodeIdTheGetLinksByNodeIdShouldReturnAStreamOfCollaborationLinks( + String nodeId, List collaborationLinks, int listSize) { + // Given + Mockito.when( + ebeanDatabaseMock + .find(CollaborationLink.class) + .where() + .eq("node_id", nodeId) + .findList()) + .thenReturn(collaborationLinks); + + // When + Stream collaborationLinkStream = + collaborationLinkRepository.getLinksByNodeId(nodeId); + + // Then + Assertions.assertThat(collaborationLinkStream).hasSize(listSize); + } + + @Test + void givenANotExistingNodeIdTheGetLinkByNodeIdShouldReturnAnEmptyStream() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(CollaborationLink.class) + .where() + .eq("node_id", "not-existing-node-id") + .findList()) + .thenReturn(Collections.emptyList()); + + // When + Stream collaborationLinkStream = + collaborationLinkRepository.getLinksByNodeId("not-existing-node-id"); + + // Then + Assertions.assertThat(collaborationLinkStream).hasSize(0); + } + + @Test + void givenAListOfCollaborationLinkIdTheDeleteLinksShouldDeleteAllTheSpecifiedLinks() { + // Given + List collaborationLinkIds = + Arrays.asList( + UUID.fromString("00000000-0000-0000-0000-000000000000"), + UUID.fromString("11111111-1111-1111-1111-111111111111"), + UUID.fromString("0293d9c6-4600-417b-becc-7f1418141c98")); + + ExpressionList expressionListMock = Mockito.mock(ExpressionList.class); + ArgumentCaptor> uuidsCaptor = ArgumentCaptor.forClass(List.class); + Mockito.when( + ebeanDatabaseMock.find(CollaborationLink.class).where().idIn(uuidsCaptor.capture())) + .thenReturn(expressionListMock); + + // When + collaborationLinkRepository.deleteLinks(collaborationLinkIds); + + // Then + Mockito.verify(expressionListMock, Mockito.times(1)).delete(); + + Assertions.assertThat(uuidsCaptor.getValue()).isEqualTo(collaborationLinkIds); + } + + @Test + void givenAnEmptyListOfCollaborationLinkIdTheDeleteLinksShouldDoNothing() { + // Given + ExpressionList expressionListMock = Mockito.mock(ExpressionList.class); + ArgumentCaptor> uuidsCaptor = ArgumentCaptor.forClass(List.class); + Mockito.when( + ebeanDatabaseMock.find(CollaborationLink.class).where().idIn(uuidsCaptor.capture())) + .thenReturn(expressionListMock); + + // When + collaborationLinkRepository.deleteLinks(Collections.emptyList()); + + // Then + Mockito.verify(expressionListMock, Mockito.times(1)).delete(); + Assertions.assertThat(uuidsCaptor.getValue()).hasSize(0); + } +} From bd663736e7b1306387980c3ae641175b6648dac3 Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Sat, 11 Nov 2023 11:54:34 +0100 Subject: [PATCH 07/11] chore(format): reformat LinkRepositoryEbean class --- .../impl/ebean/LinkRepositoryEbean.java | 65 ++++++++----------- 1 file changed, 26 insertions(+), 39 deletions(-) diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java index 6474a63b..d90a1d64 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java @@ -29,21 +29,13 @@ public LinkRepositoryEbean(EbeanDatabaseManager ebeanDatabaseManager) { } public Link createLink( - String linkId, - String nodeId, - String publicId, - Optional optExpiresAt, - Optional optDescription - ) { - - Link link = new Link( - linkId, - nodeId, - publicId, - System.currentTimeMillis(), - null, - null - ); + String linkId, + String nodeId, + String publicId, + Optional optExpiresAt, + Optional optDescription) { + + Link link = new Link(linkId, nodeId, publicId, System.currentTimeMillis(), null, null); optExpiresAt.ifPresent(link::setExpiresAt); optDescription.ifPresent(link::setDescription); @@ -55,11 +47,11 @@ public Link createLink( public Optional getLinkById(String linkId) { return ebeanDatabaseManager - .getEbeanDatabase() - .find(Link.class) - .where() - .eq(Db.Link.ID, linkId) - .findOneOrEmpty(); + .getEbeanDatabase() + .find(Link.class) + .where() + .eq(Db.Link.ID, linkId) + .findOneOrEmpty(); } public Optional getLinkByNotExpiredPublicId(String publicId) { @@ -75,21 +67,16 @@ public Optional getLinkByNotExpiredPublicId(String publicId) { .findOneOrEmpty(); } - public Stream getLinksByNodeId( - String nodeId, - LinkSort sort - ) { - Query query = ebeanDatabaseManager - .getEbeanDatabase() - .find(Link.class) - .where() - .eq(Db.Link.NODE_ID, nodeId) - .query(); + public Stream getLinksByNodeId(String nodeId, LinkSort sort) { + Query query = + ebeanDatabaseManager + .getEbeanDatabase() + .find(Link.class) + .where() + .eq(Db.Link.NODE_ID, nodeId) + .query(); - return sort - .getOrderEbeanQuery(query) - .findList() - .stream(); + return sort.getOrderEbeanQuery(query).findList().stream(); } public Link updateLink(Link link) { @@ -99,11 +86,11 @@ public Link updateLink(Link link) { public void deleteLink(String linkId) { ebeanDatabaseManager - .getEbeanDatabase() - .find(Link.class) - .where() - .eq(Db.Link.ID, linkId) - .delete(); + .getEbeanDatabase() + .find(Link.class) + .where() + .eq(Db.Link.ID, linkId) + .delete(); } public void deleteLinksBulk(Collection linkIds) { From 3b7cd978d89abbe8ba2bcb47a5ac2275c699fe15 Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Sat, 11 Nov 2023 11:55:36 +0100 Subject: [PATCH 08/11] test: add UTs and javadoc for the LinkRepositoryEbean class --- .../impl/ebean/LinkRepositoryEbean.java | 2 +- .../interfaces/LinkRepository.java | 88 ++++- .../impl/ebean/LinkRepositoryEbeanTest.java | 306 ++++++++++++++++++ 3 files changed, 382 insertions(+), 14 deletions(-) create mode 100644 core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbeanTest.java diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java index d90a1d64..5fd7937e 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbean.java @@ -42,7 +42,7 @@ public Link createLink( ebeanDatabaseManager.getEbeanDatabase().save(link); - return getLinkById(linkId).get(); + return link; } public Optional getLinkById(String linkId) { diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/LinkRepository.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/LinkRepository.java index b3e88b62..e23b7894 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/LinkRepository.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/LinkRepository.java @@ -12,33 +12,95 @@ import java.util.stream.Stream; /** - *

This is the only class allowed to execute CRUD operations on a {@link Link} element.

- *

In particular it can create a new {@link Link}, update it, access to and delete an existing - * one.

+ * This is the only class allowed to execute CRUD operations on a {@link Link} element. In + * particular, it can create a new {@link Link}, update it, access to and delete an existing one. */ public interface LinkRepository { + /** + * Creates a new {@link Link} and, after saving it in the database, it returns the {@link Link} + * just created. + * + * @param linkId is a {@link String} representing the internal unique identifier (UUID) of the + * link to create + * @param nodeId is a {@link String} representing the unique identifier of the node which the link + * is created to + * @param publicId is a {@link String} of 32 characters representing the public identifier (hash) + * of the link to create + * @param optExpiresAt is an {@link Optional} of {@link Long} representing the expiration + * timestamp of the link to create if the value is present + * @param optDescription is an {@link Optional} of {@link String} representing the description of + * the link to create if the value is present + * @return the {@link Link} just created and saved in the database. + */ Link createLink( - String linkId, - String nodeId, - String publicId, - Optional optExpiresAt, - Optional optDescription - ); + String linkId, + String nodeId, + String publicId, + Optional optExpiresAt, + Optional optDescription); + /** + * Given an internal link identifier, it allows to retrieve a {@link Link} from the database if it + * exists. + * + * @param linkId is a {@link String} representing the unique identifier (UUDI) of the link to + * retrieve + * @return an {@link Optional} containing the requested {@link Link} if it exists, otherwise it + * returns an {@link Optional#empty()}. + */ Optional getLinkById(String linkId); + /** + * Given a public link identifier, it allows to retrieve a not expired {@link Link} from the + * database if it exists. + * + * @param publicId is a {@link String} of 32 characters representing the public identifier (hash) + * of the link to retrieve + * @return an {@link Optional} containing the requested {@link Link} if it exists, otherwise it + * returns an {@link Optional#empty()}. + */ Optional getLinkByNotExpiredPublicId(String publicId); - Stream getLinksByNodeId( - String nodeId, - LinkSort sort - ); + /** + * Given a node identifier, it allows to retrieve a stream of {@link Link} from the database if + * there are links associated to the related node. + * + * @param nodeId is a {@link String} representing the unique identifier of the node which a list + * of links are associated to + * @return a {@link Stream} containing the requested {@link Link}s if they exist, otherwise it + * returns an empty {@link Stream}. + */ + Stream getLinksByNodeId(String nodeId, LinkSort sort); + /** + * Given an updated {@link Link}, it updates it in the database and returns the {@link Link} just + * saved. + * + * @param link is the updated {@link Link} to save in the database + * @return the {@link Link} just updated in the database. + */ Link updateLink(Link link); + /** + * Given a link identifier, it deletes the existing {@link Link} from the database. + * + *

if the link identifier is not associated to a {@link Link} then the delete operation does + * nothing. + * + * @param linkId is a {@link String} representing the unique identifier of the link to delete + */ void deleteLink(String linkId); + /** + * Given a collection of identifiers, it deletes the existing {@link Link}s from the database. + * + *

if a link identifier is not associated to a {@link Link} then the delete operation for that + * specific identifier does nothing. + * + * @param linkIds is a {@link Collection} of {@link String}s representing the unique identifier of + * links to delete + */ void deleteLinksBulk(Collection linkIds); /** diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbeanTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbeanTest.java new file mode 100644 index 00000000..0802fdd6 --- /dev/null +++ b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbeanTest.java @@ -0,0 +1,306 @@ +// SPDX-FileCopyrightText: 2023 Zextras +// +// SPDX-License-Identifier: AGPL-3.0-only + +package com.zextras.carbonio.files.dal.repositories.impl.ebean; + +import com.zextras.carbonio.files.dal.EbeanDatabaseManager; +import com.zextras.carbonio.files.dal.dao.ebean.Link; +import com.zextras.carbonio.files.dal.repositories.impl.ebean.utilities.LinkSort; +import com.zextras.carbonio.files.dal.repositories.interfaces.LinkRepository; +import io.ebean.Database; +import io.ebean.Query; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.stream.Stream; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +public class LinkRepositoryEbeanTest { + + private LinkRepository linkRepository; + private Database ebeanDatabaseMock; + + @BeforeEach + void setup() { + ebeanDatabaseMock = Mockito.mock(Database.class, Mockito.RETURNS_DEEP_STUBS); + EbeanDatabaseManager ebeanDatabaseManagerMock = Mockito.mock(EbeanDatabaseManager.class); + Mockito.when(ebeanDatabaseManagerMock.getEbeanDatabase()).thenReturn(ebeanDatabaseMock); + + linkRepository = new LinkRepositoryEbean(ebeanDatabaseManagerMock); + } + + @Test + void givenAllLinkAttributesTheCreateLinkShouldReturnANewLink() { + // Given + final long expirationTimestamp = System.currentTimeMillis(); + + // When + final Link createdLink = + linkRepository.createLink( + "00000000-0000-0000-0000-000000000000", + "11111111-1111-1111-1111-111111111111", + "1234abcd", + Optional.of(expirationTimestamp), + Optional.of("fake description")); + + // Then + ArgumentCaptor savedLinkCaptor = ArgumentCaptor.forClass(Link.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).save(savedLinkCaptor.capture()); + + Assertions.assertThat(savedLinkCaptor.getValue()).isEqualTo(createdLink); + Assertions.assertThat(createdLink.getLinkId()) + .isEqualTo("00000000-0000-0000-0000-000000000000"); + Assertions.assertThat(createdLink.getNodeId()) + .isEqualTo("11111111-1111-1111-1111-111111111111"); + Assertions.assertThat(createdLink.getPublicId()).isEqualTo("1234abcd"); + Assertions.assertThat(createdLink.getExpiresAt()).isPresent().contains(expirationTimestamp); + Assertions.assertThat(createdLink.getDescription()).isPresent().contains("fake description"); + } + + @Test + void givenOnlyTheMandatoryLinkAttributesTheCreateLinkShouldReturnANewLink() { + // Given & When + final Link createdLink = + linkRepository.createLink( + "00000000-0000-0000-0000-000000000000", + "11111111-1111-1111-1111-111111111111", + "1234abcd", + Optional.empty(), + Optional.empty()); + + // Then + ArgumentCaptor savedLinkCaptor = ArgumentCaptor.forClass(Link.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).save(savedLinkCaptor.capture()); + + Assertions.assertThat(savedLinkCaptor.getValue()).isEqualTo(createdLink); + Assertions.assertThat(createdLink.getLinkId()) + .isEqualTo("00000000-0000-0000-0000-000000000000"); + Assertions.assertThat(createdLink.getNodeId()) + .isEqualTo("11111111-1111-1111-1111-111111111111"); + Assertions.assertThat(createdLink.getPublicId()).isEqualTo("1234abcd"); + Assertions.assertThat(createdLink.getExpiresAt()).isEmpty(); + Assertions.assertThat(createdLink.getDescription()).isEmpty(); + } + + @Test + void givenAnExistingLinkIdTheGetLinkByIdShouldReturnTheRelatedLink() { + // Given + Link linkMock = Mockito.mock(Link.class); + Mockito.when( + ebeanDatabaseMock + .find(Link.class) + .where() + .eq("id", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.of(linkMock)); + + // When + Optional optLink = linkRepository.getLinkById("00000000-0000-0000-0000-000000000000"); + + // Then + Assertions.assertThat(optLink).isPresent().contains(linkMock); + } + + @Test + void givenANotExistingLinkIdTheGetLinkByIdShouldReturnAnOptionalEmpty() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(Link.class) + .where() + .eq(Mockito.eq("id"), Mockito.anyString()) + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + Optional optLink = linkRepository.getLinkById("00000000-0000-0000-0000-000000000000"); + + // Then + Assertions.assertThat(optLink).isEmpty(); + } + + @Test + void givenAnExistingPublicLinkIdTheGetLinkByPublicIdShouldReturnTheRelatedLink() { + // Given + Link linkMock = Mockito.mock(Link.class); + Mockito.when( + ebeanDatabaseMock + .find(Link.class) + .where() + .eq("public_id", "1234abcd") + .or() + .isNull("expire_at") + .gt(Mockito.eq("expire_at"), Mockito.anyLong()) + .endOr() + .findOneOrEmpty()) + .thenReturn(Optional.of(linkMock)); + + // When + Optional optLink = linkRepository.getLinkByNotExpiredPublicId("1234abcd"); + + // Then + Assertions.assertThat(optLink).isPresent().contains(linkMock); + } + + @Test + void givenANotExistingLinkPublicIdTheGetLinkByPublicIdShouldReturnAnOptionalEmpty() { + // Given + Link linkMock = Mockito.mock(Link.class); + Mockito.when( + ebeanDatabaseMock + .find(Link.class) + .where() + .eq(Mockito.eq("public_id"), Mockito.anyString()) + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + Optional optLink = linkRepository.getLinkByNotExpiredPublicId("1234abdc"); + + // Then + Assertions.assertThat(optLink).isEmpty(); + } + + @Test + void + givenAnExistingNodeIdAndACreatedAtAscSortTheGetLinkByNodeIdShouldReturnAStreamOfAssociatedLinks() { + // Given + Link oldestLinkMock = Mockito.mock(Link.class); + Link youngestLinkMock = Mockito.mock(Link.class); + + Query queryLinkMock = Mockito.mock(Query.class, Mockito.RETURNS_DEEP_STUBS); + Mockito.when( + ebeanDatabaseMock + .find(Link.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .query()) + .thenReturn(queryLinkMock); + + Mockito.when(queryLinkMock.order().asc("created_at").findList()) + .thenReturn(Arrays.asList(oldestLinkMock, youngestLinkMock)); + + // When + Stream links = + linkRepository.getLinksByNodeId( + "11111111-1111-1111-1111-111111111111", LinkSort.CREATED_AT_ASC); + + // Then + Assertions.assertThat(links).hasSize(2).containsExactly(oldestLinkMock, youngestLinkMock); + } + + @Test + void + givenAnExistingNodeIdAndACreatedAtDescSortTheGetLinkByNodeIdShouldReturnAStreamOfAssociatedLinks() { + // Given + Link oldestLinkMock = Mockito.mock(Link.class); + Link youngestLinkMock = Mockito.mock(Link.class); + + Query queryLinkMock = Mockito.mock(Query.class, Mockito.RETURNS_DEEP_STUBS); + Mockito.when( + ebeanDatabaseMock + .find(Link.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .query()) + .thenReturn(queryLinkMock); + + Mockito.when(queryLinkMock.order().desc("created_at").findList()) + .thenReturn(Arrays.asList(youngestLinkMock, oldestLinkMock)); + + // When + Stream links = + linkRepository.getLinksByNodeId( + "11111111-1111-1111-1111-111111111111", LinkSort.CREATED_AT_DESC); + + // Then + Assertions.assertThat(links).hasSize(2).containsExactly(youngestLinkMock, oldestLinkMock); + } + + @Test + void givenAnExistingNodeIdWithoutAssociatedLinksTheGetLinkByNodeIdShouldReturnAnEmptyStream() { + // Given + Query queryLinkMock = Mockito.mock(Query.class, Mockito.RETURNS_DEEP_STUBS); + Mockito.when( + ebeanDatabaseMock + .find(Link.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .query()) + .thenReturn(queryLinkMock); + + Mockito.when(queryLinkMock.order().asc("created_at").findList()) + .thenReturn(Collections.emptyList()); + + // When + Stream links = + linkRepository.getLinksByNodeId( + "11111111-1111-1111-1111-111111111111", LinkSort.CREATED_AT_ASC); + + // Then + Assertions.assertThat(links).hasSize(0); + } + + @Test + void givenAnUpdatedLinkTheUpdateLinkShouldSaveTheChangesAndReturnTheUpdatedLink() { + // Given + Link updatedLinkMock = Mockito.mock(Link.class); + Mockito.when(updatedLinkMock.getLinkId()).thenReturn("00000000-0000-0000-0000-000000000000"); + + Mockito.when( + ebeanDatabaseMock + .find(Link.class) + .where() + .eq("id", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.of(updatedLinkMock)); + + // When + Link returnedLink = linkRepository.updateLink(updatedLinkMock); + + // Then + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).update(updatedLinkMock); + Assertions.assertThat(returnedLink).isEqualTo(updatedLinkMock); + } + + @Test + void givenAListOfLinkIdTheDeleteLinksShouldDeleteAllTheSpecifiedLinks() { + // Given + ArgumentCaptor linkIdsCaptor = ArgumentCaptor.forClass(String.class); + Mockito.when( + ebeanDatabaseMock + .find(Link.class) + .where() + .eq(Mockito.eq("id"), linkIdsCaptor.capture()) + .delete()) + .thenReturn(1); + + // When + linkRepository.deleteLinksBulk( + Arrays.asList( + "00000000-0000-0000-0000-000000000000", "11111111-1111-1111-1111-111111111111")); + + // Then + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).beginTransaction(); + List deletedLinkIds = linkIdsCaptor.getAllValues(); + + Assertions.assertThat(deletedLinkIds) + .contains("00000000-0000-0000-0000-000000000000", "11111111-1111-1111-1111-111111111111"); + } + + @Test + void givenAnEmptyListOfLinkIdTheDeleteLinksShouldDoNothing() { + // Given & When + linkRepository.deleteLinksBulk(Collections.emptyList()); + + // Then + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).beginTransaction(); + Mockito.verifyNoMoreInteractions(ebeanDatabaseMock); + } +} From 258214c673902106a66a8c34a569eaf47fe8320d Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Sat, 11 Nov 2023 15:34:50 +0100 Subject: [PATCH 09/11] chore(format): reformat ShareRepository and ShareRepositoryEbean class --- .../impl/ebean/ShareRepositoryEbean.java | 168 +++++++----------- .../interfaces/ShareRepository.java | 98 ++++------ 2 files changed, 102 insertions(+), 164 deletions(-) diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java index 76d70fd7..38e9af39 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java @@ -32,41 +32,33 @@ public ShareRepositoryEbean(EbeanDatabaseManager ebeanDatabaseManager) { * * @param nodeId is a {@link String} representing the id of the node on the requested share. * @param userId is a {@link String} representing the id of the user on the requested share. - * * @return an {@link Optional} of {@link Share} if it exists, an {@link Optional#empty()} - * otherwise. + * otherwise. */ - private Optional getRealShare( - String nodeId, - String userId - ) { + private Optional getRealShare(String nodeId, String userId) { return mDB.getEbeanDatabase() - .find(Share.class) - .where() - .eq(Files.Db.Share.NODE_ID, nodeId) - .eq(Files.Db.Share.SHARE_TARGET_UUID, userId) - .findOneOrEmpty(); + .find(Share.class) + .where() + .eq(Files.Db.Share.NODE_ID, nodeId) + .eq(Files.Db.Share.SHARE_TARGET_UUID, userId) + .findOneOrEmpty(); } - public Optional getShare( - String nodeId, - String userId - ) { + public Optional getShare(String nodeId, String userId) { return getRealShare(nodeId, userId); } public Optional upsertShare( - String nodeId, - String targetUserId, - ACL permissions, - Boolean direct, - Boolean createdViaCollaborationLink, - Optional expireTimestamp - ) { + String nodeId, + String targetUserId, + ACL permissions, + Boolean direct, + Boolean createdViaCollaborationLink, + Optional expireTimestamp) { Optional targetShare = getShare(nodeId, targetUserId); if (targetShare.isPresent()) { if (!targetShare.get().isDirect()) { - //We're converting an inherited share to a direct share + // We're converting an inherited share to a direct share Share updateShare = targetShare.get(); if (direct) { updateShare.setDirect(true); @@ -79,15 +71,15 @@ public Optional upsertShare( return Optional.empty(); } } else { - final Share share = new Share( - nodeId, - targetUserId, - permissions, - System.currentTimeMillis(), - direct, - createdViaCollaborationLink, - null - ); + final Share share = + new Share( + nodeId, + targetUserId, + permissions, + System.currentTimeMillis(), + direct, + createdViaCollaborationLink, + null); expireTimestamp.ifPresent(share::setExpiredAt); mDB.getEbeanDatabase().save(share); return Optional.of(share); @@ -95,13 +87,12 @@ public Optional upsertShare( } public void upsertShareBulk( - List nodeIds, - String targetUserId, - ACL permissions, - Boolean direct, - Boolean createdViaCollaborationLink, - Optional expireTimestamp - ) { + List nodeIds, + String targetUserId, + ACL permissions, + Boolean direct, + Boolean createdViaCollaborationLink, + Optional expireTimestamp) { try (Transaction transaction = mDB.getEbeanDatabase().beginTransaction()) { // use JDBC batch @@ -109,16 +100,15 @@ public void upsertShareBulk( transaction.setBatchSize(50); // these go to batch buffer - nodeIds.forEach(nodeId -> - upsertShare( - nodeId, - targetUserId, - permissions, - direct, - createdViaCollaborationLink, - expireTimestamp - ) - ); + nodeIds.forEach( + nodeId -> + upsertShare( + nodeId, + targetUserId, + permissions, + direct, + createdViaCollaborationLink, + expireTimestamp)); // flush batch and commit transaction.commit(); } @@ -129,19 +119,13 @@ public Share updateShare(Share share) { return share; } - public boolean deleteShare( - String nodeId, - String targetUserId - ) { + public boolean deleteShare(String nodeId, String targetUserId) { return getShare(nodeId, targetUserId) - .map(share -> mDB.getEbeanDatabase().delete(share)) - .orElse(false); + .map(share -> mDB.getEbeanDatabase().delete(share)) + .orElse(false); } - public void deleteSharesBulk( - List nodeIds, - String targetUserId - ) { + public void deleteSharesBulk(List nodeIds, String targetUserId) { try (Transaction transaction = mDB.getEbeanDatabase().beginTransaction()) { // use JDBC batch @@ -156,39 +140,24 @@ public void deleteSharesBulk( } } - public void deleteSharesBulk( - List nodeIds - ) { - mDB.getEbeanDatabase() - .find(Share.class) - .where() - .in(Files.Db.Share.NODE_ID, nodeIds) - .delete(); + public void deleteSharesBulk(List nodeIds) { + mDB.getEbeanDatabase().find(Share.class).where().in(Files.Db.Share.NODE_ID, nodeIds).delete(); // TODO Uniform the delete behaviour with other delete method when we implement unique shareId } - public List getShares( - List nodeIds, - String targetUserId - ) { + public List getShares(List nodeIds, String targetUserId) { return mDB.getEbeanDatabase() - .find(Share.class) - .where() - .in(Files.Db.Share.NODE_ID, nodeIds) - .eq(Files.Db.Share.SHARE_TARGET_UUID, targetUserId) - .findList(); + .find(Share.class) + .where() + .in(Files.Db.Share.NODE_ID, nodeIds) + .eq(Files.Db.Share.SHARE_TARGET_UUID, targetUserId) + .findList(); } - public List getShares( - String nodeId, - List targetUserIds - ) { - Query query = mDB.getEbeanDatabase() - .find(Share.class) - .where() - .eq(Files.Db.Share.NODE_ID, nodeId) - .query(); + public List getShares(String nodeId, List targetUserIds) { + Query query = + mDB.getEbeanDatabase().find(Share.class).where().eq(Files.Db.Share.NODE_ID, nodeId).query(); if (!targetUserIds.isEmpty()) { query.where().in(Files.Db.Share.SHARE_TARGET_UUID, targetUserIds); @@ -199,26 +168,21 @@ public List getShares( public List getShares(List nodeIds) { return mDB.getEbeanDatabase() - .find(Share.class) - .where() - .in(Db.Share.NODE_ID, nodeIds) - .findList(); + .find(Share.class) + .where() + .in(Db.Share.NODE_ID, nodeIds) + .findList(); } - public List getSharesUsersIds( - String nodeId, - List sorts - ) { - Query query = mDB.getEbeanDatabase() - .createQuery(Share.class) - .where() - .eq(Files.Db.Share.NODE_ID, nodeId) - .query(); + public List getSharesUsersIds(String nodeId, List sorts) { + Query query = + mDB.getEbeanDatabase() + .createQuery(Share.class) + .where() + .eq(Files.Db.Share.NODE_ID, nodeId) + .query(); sorts.forEach(sort -> sort.getOrderEbeanQuery(query)); - return query.findList() - .stream() - .map(Share::getTargetUserId) - .collect(Collectors.toList()); + return query.findList().stream().map(Share::getTargetUserId).collect(Collectors.toList()); } } diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/ShareRepository.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/ShareRepository.java index 117519d4..d378c871 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/ShareRepository.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/interfaces/ShareRepository.java @@ -11,8 +11,9 @@ import java.util.Optional; /** - *

This is the only class allowed to execute CRUD operations on a {@link Share} element.

- *

In particular it can create a new {@link Share}, access to and delete an existing one.

+ * This is the only class allowed to execute CRUD operations on a {@link Share} element. + * + *

In particular it can create a new {@link Share}, access to and delete an existing one. */ public interface ShareRepository { @@ -24,66 +25,59 @@ public interface ShareRepository { * * @return an {@link Optional) containing the {@link Share} requested if exists. */ - Optional getShare( - String nodeId, - String userId - ); + Optional getShare(String nodeId, String userId); /** - *

Creates a new {@link Share} saving it in the database, then returns an {@link Optional} of - * the {@link Share} just created.

+ * Creates a new {@link Share} saving it in the database, then returns an {@link Optional} of the + * {@link Share} just created. + * *

This method returns an optional because creation can fail when it tries to create a share - * that already exists for a user on a particular node.

+ * that already exists for a user on a particular node. * * @param nodeId is a {@link String} of the node id. - * @param targetUserId is a {@link String} of the target user id which the node will be shared - * to. + * @param targetUserId is a {@link String} of the target user id which the node will be shared to. * @param permissions is a {@link ACL} representing the permissions of this share. * @param direct is a boolean used to determine if the share is direct or inherited. * @param createdViaCollaborationLink * @param expireTimestamp is an {@link Optional} for creating a temporary share with an - * expiration date. - * + * expiration date. * @return an {@link Optional} of the new {@link Share} just created and saved in the database. */ Optional upsertShare( - String nodeId, - String targetUserId, - ACL permissions, - Boolean direct, - Boolean createdViaCollaborationLink, - Optional expireTimestamp - ); + String nodeId, + String targetUserId, + ACL permissions, + Boolean direct, + Boolean createdViaCollaborationLink, + Optional expireTimestamp); /** - *

Creates new {@link Share} for a list of nodes saving it in the database. + * Creates new {@link Share} for a list of nodes saving it in the database. + * *

This method considers the parameters in input already valid so it does not do any kind of - * control on them.

This method is created as a utility for the propagation of the share on - * sub nodes. + * control on them. This method is created as a utility for the propagation of the share on sub + * nodes. * * @param nodeIds is a {@link List} of the nodeIds where to create the share. - * @param targetUserId is a {@link String} of the target user id which the node will be shared - * to. + * @param targetUserId is a {@link String} of the target user id which the node will be shared to. * @param permissions is a {@link ACL} representing the permissions of this share. * @param direct is a {@link Boolean} used to determine if the share is direct or inherited. * @param expireTimestamp is an {@link Optional} for creating a temporary share with an - * expiration date. + * expiration date. */ void upsertShareBulk( - List nodeIds, - String targetUserId, - ACL permissions, - Boolean direct, - Boolean createdViaCollaborationLink, - Optional expireTimestamp - ); + List nodeIds, + String targetUserId, + ACL permissions, + Boolean direct, + Boolean createdViaCollaborationLink, + Optional expireTimestamp); /** * This method is used to update a share on database. It requires the updated share and it will * update it on database and cache. * * @param share is the {@link Share} to update - * * @return the updated {@link Share} */ Share updateShare(Share share); @@ -93,14 +87,10 @@ void upsertShareBulk( * * @param nodeId is a {@link String} of the id of shared node. * @param targetUserId is a {@link String} of the target user id which the node is shared to. - * * @return true if the {@link Share} exists and the deletion from the database is done - * successfully, false otherwise. + * successfully, false otherwise. */ - boolean deleteShare( - String nodeId, - String targetUserId - ); + boolean deleteShare(String nodeId, String targetUserId); /** * Deletes the shares on a list of nodes for a user. This method considers the parameters in input @@ -110,10 +100,7 @@ boolean deleteShare( * @param nodeIds is a {@link List} of the nodeIds where to delete the share. * @param targetUserId is a {@link String} of the target user. */ - void deleteSharesBulk( - List nodeIds, - String targetUserId - ); + void deleteSharesBulk(List nodeIds, String targetUserId); /** * Deletes all shares of each given node. This method considers the parameters in input already @@ -128,13 +115,9 @@ void deleteSharesBulk( * * @param nodeIds is a {@link List} of the nodes I'm requesting the shares. * @param targetUserId is a {@link String} of the target user id which the nodes are shared to. - * * @return the {@link List} of found {@link Share}s. */ - List getShares( - List nodeIds, - String targetUserId - ); + List getShares(List nodeIds, String targetUserId); /** * Returns the list of shares, if present, for the specific node. If targetUserIds is not empty it @@ -142,19 +125,14 @@ List getShares( * * @param nodeId is a {@link String} representing the id of the node i'm requesting shares. * @param targetUserIds is a {@link List} of the target user ids for which I'm requesting - * the shares. - * + * the shares. * @return the {@link List} of found {@link Share}s. */ - List getShares( - String nodeId, - List targetUserIds - ); + List getShares(String nodeId, List targetUserIds); /** * @param nodeIds is a {@link List} of the node ids. For each one of them the query - * retrieves all the related shares. - * + * retrieves all the related shares. * @return a {@link List} of found {@link Share}s for the specific node ids in input. */ List getShares(List nodeIds); @@ -164,11 +142,7 @@ List getShares( * * @param nodeId is a {@link String} of the id of the node for which retrieve the shares. * @param sorts is a {@link List} used for ordering the returned shares. - * * @return the {@link List} of ids of users. */ - List getSharesUsersIds( - String nodeId, - List sorts - ); + List getSharesUsersIds(String nodeId, List sorts); } From dbd4cd18e35e3f7af59df034c6d3d56b4a09194d Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Mon, 13 Nov 2023 12:45:26 +0100 Subject: [PATCH 10/11] test: add UTs and javadoc for the ShareRepositoryEbean class --- .gitignore | 5 +- .../impl/ebean/ShareRepositoryEbean.java | 45 +- .../impl/ebean/ShareRepositoryEbeanTest.java | 839 ++++++++++++++++++ 3 files changed, 873 insertions(+), 16 deletions(-) create mode 100644 core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbeanTest.java diff --git a/.gitignore b/.gitignore index 5794d201..96a73a6b 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,9 @@ core/.classpath core/.project core/.settings/ +# LSP +.metadata/ +jdt.*/ # JetBrains IDE .idea/ @@ -51,4 +54,4 @@ Thumbs.db *.wmv # Reuse license -LICENSES \ No newline at end of file +LICENSES diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java index 38e9af39..c6ffafe9 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java @@ -12,6 +12,7 @@ import com.zextras.carbonio.files.dal.dao.ebean.Share; import com.zextras.carbonio.files.dal.repositories.impl.ebean.utilities.ShareSort; import com.zextras.carbonio.files.dal.repositories.interfaces.ShareRepository; +import io.ebean.ExpressionList; import io.ebean.Query; import io.ebean.Transaction; import java.util.List; @@ -20,11 +21,11 @@ public class ShareRepositoryEbean implements ShareRepository { - private final EbeanDatabaseManager mDB; + private final EbeanDatabaseManager ebeanDatabaseManager; @Inject public ShareRepositoryEbean(EbeanDatabaseManager ebeanDatabaseManager) { - mDB = ebeanDatabaseManager; + this.ebeanDatabaseManager = ebeanDatabaseManager; } /** @@ -36,7 +37,8 @@ public ShareRepositoryEbean(EbeanDatabaseManager ebeanDatabaseManager) { * otherwise. */ private Optional getRealShare(String nodeId, String userId) { - return mDB.getEbeanDatabase() + return ebeanDatabaseManager + .getEbeanDatabase() .find(Share.class) .where() .eq(Files.Db.Share.NODE_ID, nodeId) @@ -64,6 +66,7 @@ public Optional upsertShare( updateShare.setDirect(true); } updateShare.setPermissions(permissions); + updateShare.setCreatedViaLink(createdViaCollaborationLink); expireTimestamp.ifPresent(updateShare::setExpiredAt); return Optional.of(updateShare(updateShare)); } else { @@ -81,7 +84,7 @@ public Optional upsertShare( createdViaCollaborationLink, null); expireTimestamp.ifPresent(share::setExpiredAt); - mDB.getEbeanDatabase().save(share); + ebeanDatabaseManager.getEbeanDatabase().save(share); return Optional.of(share); } } @@ -93,7 +96,7 @@ public void upsertShareBulk( Boolean direct, Boolean createdViaCollaborationLink, Optional expireTimestamp) { - try (Transaction transaction = mDB.getEbeanDatabase().beginTransaction()) { + try (Transaction transaction = ebeanDatabaseManager.getEbeanDatabase().beginTransaction()) { // use JDBC batch transaction.setBatchMode(true); @@ -115,18 +118,18 @@ public void upsertShareBulk( } public Share updateShare(Share share) { - mDB.getEbeanDatabase().update(share); + ebeanDatabaseManager.getEbeanDatabase().update(share); return share; } public boolean deleteShare(String nodeId, String targetUserId) { return getShare(nodeId, targetUserId) - .map(share -> mDB.getEbeanDatabase().delete(share)) + .map(share -> ebeanDatabaseManager.getEbeanDatabase().delete(share)) .orElse(false); } public void deleteSharesBulk(List nodeIds, String targetUserId) { - try (Transaction transaction = mDB.getEbeanDatabase().beginTransaction()) { + try (Transaction transaction = ebeanDatabaseManager.getEbeanDatabase().beginTransaction()) { // use JDBC batch transaction.setBatchMode(true); @@ -141,13 +144,19 @@ public void deleteSharesBulk(List nodeIds, String targetUserId) { } public void deleteSharesBulk(List nodeIds) { - mDB.getEbeanDatabase().find(Share.class).where().in(Files.Db.Share.NODE_ID, nodeIds).delete(); + ebeanDatabaseManager + .getEbeanDatabase() + .find(Share.class) + .where() + .in(Files.Db.Share.NODE_ID, nodeIds) + .delete(); // TODO Uniform the delete behaviour with other delete method when we implement unique shareId } public List getShares(List nodeIds, String targetUserId) { - return mDB.getEbeanDatabase() + return ebeanDatabaseManager + .getEbeanDatabase() .find(Share.class) .where() .in(Files.Db.Share.NODE_ID, nodeIds) @@ -156,18 +165,23 @@ public List getShares(List nodeIds, String targetUserId) { } public List getShares(String nodeId, List targetUserIds) { - Query query = - mDB.getEbeanDatabase().find(Share.class).where().eq(Files.Db.Share.NODE_ID, nodeId).query(); + ExpressionList query = + ebeanDatabaseManager + .getEbeanDatabase() + .find(Share.class) + .where() + .eq(Files.Db.Share.NODE_ID, nodeId); if (!targetUserIds.isEmpty()) { - query.where().in(Files.Db.Share.SHARE_TARGET_UUID, targetUserIds); + query.in(Files.Db.Share.SHARE_TARGET_UUID, targetUserIds); } return query.findList(); } public List getShares(List nodeIds) { - return mDB.getEbeanDatabase() + return ebeanDatabaseManager + .getEbeanDatabase() .find(Share.class) .where() .in(Db.Share.NODE_ID, nodeIds) @@ -176,7 +190,8 @@ public List getShares(List nodeIds) { public List getSharesUsersIds(String nodeId, List sorts) { Query query = - mDB.getEbeanDatabase() + ebeanDatabaseManager + .getEbeanDatabase() .createQuery(Share.class) .where() .eq(Files.Db.Share.NODE_ID, nodeId) diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbeanTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbeanTest.java new file mode 100644 index 00000000..090ea31c --- /dev/null +++ b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbeanTest.java @@ -0,0 +1,839 @@ +// SPDX-FileCopyrightText: 2023 Zextras +// +// SPDX-License-Identifier: AGPL-3.0-only + +package com.zextras.carbonio.files.dal.repositories.impl.ebean; + +import com.zextras.carbonio.files.dal.EbeanDatabaseManager; +import com.zextras.carbonio.files.dal.dao.ebean.ACL; +import com.zextras.carbonio.files.dal.dao.ebean.ACL.SharePermission; +import com.zextras.carbonio.files.dal.dao.ebean.Share; +import com.zextras.carbonio.files.dal.repositories.impl.ebean.utilities.ShareSort; +import com.zextras.carbonio.files.dal.repositories.interfaces.ShareRepository; +import io.ebean.Database; +import io.ebean.ExpressionList; +import io.ebean.OrderBy; +import io.ebean.Query; +import io.ebean.Transaction; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +public class ShareRepositoryEbeanTest { + + private Database ebeanDatabaseMock; + private ShareRepository shareRepository; + + @BeforeEach + void setup() { + ebeanDatabaseMock = Mockito.mock(Database.class, Mockito.RETURNS_DEEP_STUBS); + EbeanDatabaseManager ebeanDatabaseManagerMock = Mockito.mock(EbeanDatabaseManager.class); + Mockito.when(ebeanDatabaseManagerMock.getEbeanDatabase()).thenReturn(ebeanDatabaseMock); + + shareRepository = new ShareRepositoryEbean(ebeanDatabaseManagerMock); + } + + @Test + void givenANodeIdWithoutSharesAndAllShareAttributesTheUpsertShareShouldCreateANewShare() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + Optional optShare = + shareRepository.upsertShare( + "11111111-1111-1111-1111-111111111111", + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_AND_WRITE), + true, + false, + Optional.of(1L)); + + // Then + ArgumentCaptor createdShareCaptor = ArgumentCaptor.forClass(Share.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).save(createdShareCaptor.capture()); + + Assertions.assertThat(optShare).isPresent(); + Share createdShare = optShare.get(); + + Assertions.assertThat(createdShare).isEqualTo(createdShareCaptor.getValue()); + + Assertions.assertThat(createdShare.getNodeId()) + .isEqualTo("11111111-1111-1111-1111-111111111111"); + Assertions.assertThat(createdShare.getTargetUserId()) + .isEqualTo("00000000-0000-0000-0000-000000000000"); + Assertions.assertThat(createdShare.getPermissions()) + .isEqualTo(ACL.decode(SharePermission.READ_AND_WRITE)); + Assertions.assertThat(createdShare.isDirect()).isTrue(); + Assertions.assertThat(createdShare.isCreatedViaLink()).isFalse(); + Assertions.assertThat(createdShare.getExpiredAt()).isPresent().contains(1L); + } + + @Test + void + givenANodeIdWithoutSharesAndOnlyTheMandatoryShareAttributesTheUpsertShareShouldCreateANewShare() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + Optional optShare = + shareRepository.upsertShare( + "11111111-1111-1111-1111-111111111111", + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_ONLY), + false, + false, + Optional.empty()); + + // Then + ArgumentCaptor createdShareCaptor = ArgumentCaptor.forClass(Share.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).save(createdShareCaptor.capture()); + + Assertions.assertThat(optShare).isPresent(); + Share createdShare = optShare.get(); + + Assertions.assertThat(createdShare).isEqualTo(createdShareCaptor.getValue()); + + Assertions.assertThat(createdShare.getNodeId()) + .isEqualTo("11111111-1111-1111-1111-111111111111"); + Assertions.assertThat(createdShare.getTargetUserId()) + .isEqualTo("00000000-0000-0000-0000-000000000000"); + Assertions.assertThat(createdShare.getPermissions()) + .isEqualTo(ACL.decode(SharePermission.READ_ONLY)); + Assertions.assertThat(createdShare.isDirect()).isFalse(); + Assertions.assertThat(createdShare.isCreatedViaLink()).isFalse(); + Assertions.assertThat(createdShare.getExpiredAt()).isEmpty(); + } + + @Test + void + givenANodeIdAUserIdAlreadySharedToIndirectlyAndAllShareAttributesChangedTheUpsertShareShouldUpdateAndReturnTheExistingShare() { + // Given + Share existingShare = + new Share( + "11111111-1111-1111-1111-111111111111", + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_AND_WRITE), + 1L, + false, + false, + 2L); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.of(existingShare)); + + // When + Optional optShare = + shareRepository.upsertShare( + "11111111-1111-1111-1111-111111111111", + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_ONLY), + true, + true, + Optional.of(5L)); + + // Then + ArgumentCaptor updatedShareCaptor = ArgumentCaptor.forClass(Share.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).update(updatedShareCaptor.capture()); + + Assertions.assertThat(optShare).isPresent(); + Share updatedShare = optShare.get(); + + Assertions.assertThat(updatedShare).isEqualTo(updatedShareCaptor.getValue()); + + Assertions.assertThat(updatedShare.getNodeId()) + .isEqualTo("11111111-1111-1111-1111-111111111111"); + Assertions.assertThat(updatedShare.getTargetUserId()) + .isEqualTo("00000000-0000-0000-0000-000000000000"); + Assertions.assertThat(updatedShare.getCreatedAt()).isEqualTo(1L); + Assertions.assertThat(updatedShare.getPermissions()) + .isEqualTo(ACL.decode(SharePermission.READ_ONLY)); + Assertions.assertThat(updatedShare.isDirect()).isTrue(); + Assertions.assertThat(updatedShare.isCreatedViaLink()).isTrue(); + Assertions.assertThat(updatedShare.getExpiredAt()).isPresent().contains(5L); + } + + @Test + void + givenANodeIdAUserIdAlreadySharedToIndirectlyAndNoShareAttributesChangedTheUpsertShareShouldUpdateAndReturnTheExistingShare() { + // Given + Share existingShare = + new Share( + "11111111-1111-1111-1111-111111111111", + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_AND_WRITE), + 1L, + false, + false, + 2L); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.of(existingShare)); + + // When + Optional optShare = + shareRepository.upsertShare( + "11111111-1111-1111-1111-111111111111", + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_AND_WRITE), + false, + false, + Optional.empty()); + + // Then + ArgumentCaptor updatedShareCaptor = ArgumentCaptor.forClass(Share.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).update(updatedShareCaptor.capture()); + + Assertions.assertThat(optShare).isPresent(); + Share updatedShare = optShare.get(); + + Assertions.assertThat(updatedShare).isEqualTo(updatedShareCaptor.getValue()); + + Assertions.assertThat(updatedShare.getNodeId()) + .isEqualTo("11111111-1111-1111-1111-111111111111"); + Assertions.assertThat(updatedShare.getTargetUserId()) + .isEqualTo("00000000-0000-0000-0000-000000000000"); + Assertions.assertThat(updatedShare.getCreatedAt()).isEqualTo(1L); + Assertions.assertThat(updatedShare.getPermissions()) + .isEqualTo(ACL.decode(SharePermission.READ_AND_WRITE)); + Assertions.assertThat(updatedShare.isDirect()).isFalse(); + Assertions.assertThat(updatedShare.isCreatedViaLink()).isFalse(); + Assertions.assertThat(updatedShare.getExpiredAt()).isPresent().contains(2L); + } + + // TODO: this test is checking an odd logic in the upsertShare method (it should be removed when + // the logic will be simplified) + @Test + void + givenANodeIdAUserIdAlreadySharedToDirectlyAndAllShareAttributesChangedTheUpsertShareShouldNotUpdateTheShareAndReturnOptionalEmpty() { + // Given + Share existingShare = + new Share( + "11111111-1111-1111-1111-111111111111", + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_AND_WRITE), + 1L, + true, + false, + 2L); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.of(existingShare)); + + // When + Optional optShare = + shareRepository.upsertShare( + "11111111-1111-1111-1111-111111111111", + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_ONLY), + false, + true, + Optional.empty()); + + // Then + Mockito.verify(ebeanDatabaseMock, Mockito.times(0)).update(Mockito.any()); + Assertions.assertThat(optShare).isEmpty(); + } + + @Test + void + givenAListOfNodeIdsWithoutSharesAndAllShareAttributesTheUpsertShareBulkShouldCreateNewShares() { + // Given + Transaction transactionMock = Mockito.mock(Transaction.class); + Mockito.when(ebeanDatabaseMock.beginTransaction()).thenReturn(transactionMock); + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq(Mockito.eq("node_id"), Mockito.anyString()) + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + shareRepository.upsertShareBulk( + Arrays.asList( + "11111111-1111-1111-1111-111111111111", "11111111-2222-2222-2222-111111111111"), + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_AND_WRITE), + true, + false, + Optional.of(1L)); + + // Then + Mockito.verify(transactionMock, Mockito.times(1)).setBatchMode(true); + Mockito.verify(transactionMock, Mockito.times(1)).setBatchSize(50); + Mockito.verify(transactionMock, Mockito.times(1)).commit(); + + ArgumentCaptor createdSharesCaptor = ArgumentCaptor.forClass(Share.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(2)).save(createdSharesCaptor.capture()); + + List createdShares = createdSharesCaptor.getAllValues(); + Share firstCreatedShare = createdShares.get(0); + Share secondCreatedShare = createdShares.get(1); + + Assertions.assertThat(firstCreatedShare.getNodeId()) + .isEqualTo("11111111-1111-1111-1111-111111111111"); + Assertions.assertThat(secondCreatedShare.getNodeId()) + .isEqualTo("11111111-2222-2222-2222-111111111111"); + + Assertions.assertThat(firstCreatedShare.getTargetUserId()) + .isEqualTo(secondCreatedShare.getTargetUserId()) + .isEqualTo("00000000-0000-0000-0000-000000000000"); + Assertions.assertThat(firstCreatedShare.getPermissions()) + .isEqualTo(secondCreatedShare.getPermissions()) + .isEqualTo(ACL.decode(SharePermission.READ_AND_WRITE)); + Assertions.assertThat(firstCreatedShare.isDirect()) + .isEqualTo(secondCreatedShare.isDirect()) + .isTrue(); + Assertions.assertThat(firstCreatedShare.isCreatedViaLink()) + .isEqualTo(secondCreatedShare.isCreatedViaLink()) + .isFalse(); + Assertions.assertThat(firstCreatedShare.getExpiredAt()) + .isEqualTo(secondCreatedShare.getExpiredAt()) + .isPresent() + .contains(1L); + } + + @Test + void givenAnEmptyListOfNodeIdAndAllShareAttributesTheUpsertShareBulkShouldDoNothing() { + // Given + Transaction transactionMock = Mockito.mock(Transaction.class); + Mockito.when(ebeanDatabaseMock.beginTransaction()).thenReturn(transactionMock); + + // When + shareRepository.upsertShareBulk( + Collections.emptyList(), + "00000000-0000-0000-0000-000000000000", + ACL.decode(SharePermission.READ_AND_WRITE), + true, + false, + Optional.of(1L)); + + // Then + Mockito.verify(transactionMock, Mockito.times(1)).setBatchMode(true); + Mockito.verify(transactionMock, Mockito.times(1)).setBatchSize(50); + Mockito.verify(transactionMock, Mockito.times(1)).commit(); + // SKIPMockito.verifyNoMoreInteractions(ebeanDatabaseMock); + } + + @Test + void givenAnUpdatedShareTheUpdateShareShouldSaveTheChangesAndReturnTheUpdatedShare() { + // Given + Share updatedShareMock = Mockito.mock(Share.class); + + // When + Share returnedShare = shareRepository.updateShare(updatedShareMock); + + // Then + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).update(updatedShareMock); + Assertions.assertThat(returnedShare).isEqualTo(updatedShareMock); + } + + @Test + void givenANodeIdAndAUserIdSharedToDirectlyTheGetShareShouldReturnTheRelatedShare() { + // Given + Share shareMock = Mockito.mock(Share.class); + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.of(shareMock)); + + // When + Optional optShare = + shareRepository.getShare( + "11111111-1111-1111-1111-111111111111", "00000000-0000-0000-0000-000000000000"); + + // Then + Assertions.assertThat(optShare).isPresent().contains(shareMock); + } + + @Test + void givenANodeIdAndAUserIdNotSharedToTheGetShareShouldReturnAnOptionalEmpty() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq(Mockito.eq("node_id"), Mockito.anyString()) + .eq(Mockito.eq("target_uuid"), Mockito.anyString()) + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + Optional optShare = + shareRepository.getShare( + "11111111-1111-1111-1111-111111111111", "00000000-0000-0000-0000-000000000000"); + + // Then + Assertions.assertThat(optShare).isEmpty(); + } + + @Test + void givenAListOfNodeIdsAndAUserIdSharedToTheGetSharesShouldReturnTheListOfShares() { + // Given + Share shareMock1 = Mockito.mock(Share.class); + Share shareMock2 = Mockito.mock(Share.class); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .in( + "node_id", + Arrays.asList( + "11111111-1111-1111-1111-111111111111", + "11111111-2222-2222-2222-111111111111")) + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findList()) + .thenReturn(Arrays.asList(shareMock1, shareMock2)); + + // When + List shares = + shareRepository.getShares( + Arrays.asList( + "11111111-1111-1111-1111-111111111111", "11111111-2222-2222-2222-111111111111"), + "00000000-0000-0000-0000-000000000000"); + + // Then + Assertions.assertThat(shares).hasSize(2).containsExactly(shareMock1, shareMock2); + } + + @Test + void givenAListOfNodeIdsAndAUserIdNotSharedToTheGetSharesShouldReturnAnEmptyOfShares() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .in( + "node_id", + Arrays.asList( + "11111111-1111-1111-1111-111111111111", + "11111111-2222-2222-2222-111111111111")) + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findList()) + .thenReturn(Collections.emptyList()); + + // When + List shares = + shareRepository.getShares( + Arrays.asList( + "11111111-1111-1111-1111-111111111111", "11111111-2222-2222-2222-111111111111"), + "00000000-0000-0000-0000-000000000000"); + + // Then + Assertions.assertThat(shares).isEmpty(); + } + + @Test + void givenANodeIdAndListOfUserIdsSharedToTheGetSharesShouldReturnTheListOfShares() { + // Given + Share shareMock1 = Mockito.mock(Share.class); + Share shareMock2 = Mockito.mock(Share.class); + + ExpressionList expressionListShare = + Mockito.mock(ExpressionList.class, Mockito.RETURNS_DEEP_STUBS); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111")) + .thenReturn(expressionListShare); + + Mockito.when(expressionListShare.findList()).thenReturn(Arrays.asList(shareMock1, shareMock2)); + + // When + List shares = + shareRepository.getShares( + "11111111-1111-1111-1111-111111111111", + Arrays.asList( + "00000000-0000-0000-0000-000000000000", "00000000-1111-1111-1111-000000000000")); + + // Then + Mockito.verify(expressionListShare, Mockito.times(1)) + .in( + "target_uuid", + Arrays.asList( + "00000000-0000-0000-0000-000000000000", "00000000-1111-1111-1111-000000000000")); + + Assertions.assertThat(shares).hasSize(2).containsExactly(shareMock1, shareMock2); + } + + @Test + void givenANodeIdAndAnEmptyListOfUserIdsTheGetSharesShouldReturnAnEmptyListOfShares() { + // Given + ExpressionList expressionListShare = + Mockito.mock(ExpressionList.class, Mockito.RETURNS_DEEP_STUBS); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111")) + .thenReturn(expressionListShare); + + Mockito.when(expressionListShare.findList()).thenReturn(Collections.emptyList()); + + // When + List shares = + shareRepository.getShares("11111111-1111-1111-1111-111111111111", Collections.emptyList()); + + // Then + Mockito.verify(expressionListShare, Mockito.times(0)) + .in(Mockito.eq("target_uuid"), Mockito.anyList()); + + Assertions.assertThat(shares).isEmpty(); + } + + @Test + void givenAListOfNodeIdsWithTwoSharesTheGetSharesShouldReturnTheListOfShares() { + // Given + Share shareMock1 = Mockito.mock(Share.class); + Share shareMock2 = Mockito.mock(Share.class); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .in( + "node_id", + Arrays.asList( + "11111111-1111-1111-1111-111111111111", + "11111111-2222-2222-2222-111111111111")) + .findList()) + .thenReturn(Arrays.asList(shareMock1, shareMock2)); + + // When + List shares = + shareRepository.getShares( + Arrays.asList( + "11111111-1111-1111-1111-111111111111", "11111111-2222-2222-2222-111111111111")); + + // Then + Assertions.assertThat(shares).hasSize(2).containsExactly(shareMock1, shareMock2); + } + + @Test + void givenAListOfNodeIdsWithoutAShareTheGetSharesShouldReturnAnEmptyListOfShares() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .in(Mockito.eq("node_id"), Mockito.anyList()) + .findList()) + .thenReturn(Collections.emptyList()); + + // When + List shares = shareRepository.getShares(Collections.emptyList()); + + // Then + Assertions.assertThat(shares).isEmpty(); + } + + @Test + void + givenANodeIdWithTwoSharesAndNoSortTheGetSharesUsersIdsShouldReturnTheListOfUserIdsSharedTo() { + // Given + Share shareMock1 = Mockito.mock(Share.class); + Mockito.when(shareMock1.getTargetUserId()).thenReturn("00000000-0000-0000-0000-000000000000"); + + Share shareMock2 = Mockito.mock(Share.class); + Mockito.when(shareMock2.getTargetUserId()).thenReturn("00000000-1111-1111-1111-000000000000"); + + Query queryMock = Mockito.mock(Query.class); + Mockito.when( + ebeanDatabaseMock + .createQuery(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .query()) + .thenReturn(queryMock); + + Mockito.when(queryMock.findList()).thenReturn(Arrays.asList(shareMock1, shareMock2)); + + // When + List userIds = + shareRepository.getSharesUsersIds( + "11111111-1111-1111-1111-111111111111", Collections.emptyList()); + + // Then + Assertions.assertThat(userIds) + .hasSize(2) + .containsExactly( + "00000000-0000-0000-0000-000000000000", "00000000-1111-1111-1111-000000000000"); + Mockito.verify(queryMock, Mockito.times(0)).order(); + } + + @Test + void + givenANodeIdWithTwoSharesAndTheTargetUserAscSortTheGetSharesUsersIdsShouldReturnTheSortedListOfUserIdsSharedTo() { + // Given + Share shareMock1 = Mockito.mock(Share.class); + Mockito.when(shareMock1.getTargetUserId()).thenReturn("00000000-0000-0000-0000-000000000000"); + + Share shareMock2 = Mockito.mock(Share.class); + Mockito.when(shareMock2.getTargetUserId()).thenReturn("00000000-1111-1111-1111-000000000000"); + + Query queryMock = Mockito.mock(Query.class); + OrderBy orderByMock = Mockito.mock(OrderBy.class); + + Mockito.when( + ebeanDatabaseMock + .createQuery(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .query()) + .thenReturn(queryMock); + + Mockito.when(queryMock.order()).thenReturn(orderByMock); + Mockito.when(queryMock.findList()).thenReturn(Arrays.asList(shareMock1, shareMock2)); + + // When + List userIds = + shareRepository.getSharesUsersIds( + "11111111-1111-1111-1111-111111111111", + Collections.singletonList(ShareSort.TARGET_USER_ASC)); + + // Then + Assertions.assertThat(userIds) + .hasSize(2) + .containsExactly( + "00000000-0000-0000-0000-000000000000", "00000000-1111-1111-1111-000000000000"); + Mockito.verify(orderByMock, Mockito.times(1)).asc("target_uuid"); + } + + @Test + void givenANodeIdWithoutSharesTheGetSharesUsersIdsShouldReturnAnEmptyListOfUserIds() { + // Given + Query queryMock = Mockito.mock(Query.class); + Mockito.when( + ebeanDatabaseMock + .createQuery(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .query()) + .thenReturn(queryMock); + + Mockito.when(queryMock.findList()).thenReturn(Collections.emptyList()); + + // When + List userIds = + shareRepository.getSharesUsersIds( + "11111111-1111-1111-1111-111111111111", Collections.emptyList()); + + // Then + Assertions.assertThat(userIds).isEmpty(); + Mockito.verify(queryMock, Mockito.times(0)).order(); + } + + @Test + void givenANodeIdAndAUserIdSharedToTheDeleteShareShouldDeleteTheShareAndReturnTrue() { + // Given + Share shareMock = Mockito.mock(Share.class); + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.of(shareMock)); + + Mockito.when(ebeanDatabaseMock.delete(shareMock)).thenReturn(true); + + // When + boolean isShareDeleted = + shareRepository.deleteShare( + "11111111-1111-1111-1111-111111111111", "00000000-0000-0000-0000-000000000000"); + + // Then + Assertions.assertThat(isShareDeleted).isTrue(); + + ArgumentCaptor deletedShareCaptor = ArgumentCaptor.forClass(Share.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(1)).delete(deletedShareCaptor.capture()); + + Assertions.assertThat(shareMock).isEqualTo(deletedShareCaptor.getValue()); + } + + @Test + void givenANodeIdAndAUserIdNotSharedToTheDeleteShareShouldReturnFalse() { + // Given + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + boolean isShareDeleted = + shareRepository.deleteShare( + "11111111-1111-1111-1111-111111111111", "00000000-0000-0000-0000-000000000000"); + + // Then + Assertions.assertThat(isShareDeleted).isFalse(); + Mockito.verify(ebeanDatabaseMock, Mockito.times(0)).delete(Mockito.any(Share.class)); + } + + @Test + void givenAListOfNodeIdsAndAUserIdSharedToTheDeleteSharesBulkShouldDeleteTheRelatedShares() { + // Given + Transaction transactionMock = Mockito.mock(Transaction.class); + Mockito.when(ebeanDatabaseMock.beginTransaction()).thenReturn(transactionMock); + + Share shareMock1 = Mockito.mock(Share.class); + Share shareMock2 = Mockito.mock(Share.class); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-1111-1111-1111-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.of(shareMock1)); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq("node_id", "11111111-2222-2222-2222-111111111111") + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.of(shareMock2)); + + // When + shareRepository.deleteSharesBulk( + Arrays.asList( + "11111111-1111-1111-1111-111111111111", "11111111-2222-2222-2222-111111111111"), + "00000000-0000-0000-0000-000000000000"); + + // Then + Mockito.verify(transactionMock, Mockito.times(1)).setBatchMode(true); + Mockito.verify(transactionMock, Mockito.times(1)).setBatchSize(50); + Mockito.verify(transactionMock, Mockito.times(1)).commit(); + + ArgumentCaptor deletedSharesCaptor = ArgumentCaptor.forClass(Share.class); + Mockito.verify(ebeanDatabaseMock, Mockito.times(2)).delete(deletedSharesCaptor.capture()); + + List deletedShares = deletedSharesCaptor.getAllValues(); + Share firstDeletedShare = deletedShares.get(0); + Share secondDeletedShare = deletedShares.get(1); + + Assertions.assertThat(firstDeletedShare).isEqualTo(shareMock1); + Assertions.assertThat(secondDeletedShare).isEqualTo(shareMock2); + } + + @Test + void givenAListOfNodeIdsAndAUserIdNotSharedToTheDeleteSharesBulkShouldDoNothing() { + // Given + Transaction transactionMock = Mockito.mock(Transaction.class); + Mockito.when(ebeanDatabaseMock.beginTransaction()).thenReturn(transactionMock); + + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .eq(Mockito.eq("node_id"), Mockito.anyString()) + .eq("target_uuid", "00000000-0000-0000-0000-000000000000") + .findOneOrEmpty()) + .thenReturn(Optional.empty()); + + // When + shareRepository.deleteSharesBulk( + Arrays.asList( + "11111111-1111-1111-1111-111111111111", "11111111-2222-2222-2222-111111111111"), + "00000000-0000-0000-0000-000000000000"); + + // Then + Mockito.verify(transactionMock, Mockito.times(1)).setBatchMode(true); + Mockito.verify(transactionMock, Mockito.times(1)).setBatchSize(50); + Mockito.verify(transactionMock, Mockito.times(1)).commit(); + + Mockito.verify(ebeanDatabaseMock, Mockito.times(0)).delete(Mockito.any(Share.class)); + } + + @Test + void givenAListOfNodeIdsSharedToTheDeleteSharesBulkShouldDeleteTheRelatedShares() { + // Given + ExpressionList expressionListMock = Mockito.mock(ExpressionList.class); + Mockito.when( + ebeanDatabaseMock + .find(Share.class) + .where() + .in( + "node_id", + Arrays.asList( + "11111111-1111-1111-1111-111111111111", + "11111111-2222-2222-2222-111111111111"))) + .thenReturn(expressionListMock); + + // When + shareRepository.deleteSharesBulk( + Arrays.asList( + "11111111-1111-1111-1111-111111111111", "11111111-2222-2222-2222-111111111111")); + + // Then + Mockito.verify(expressionListMock, Mockito.times(1)).delete(); + } + + @Test + void givenAListOfNodeIdsNotSharedToTheDeleteSharesBulkShouldTryToDeleteZeroShares() { + // Given + ExpressionList expressionListMock = Mockito.mock(ExpressionList.class); + Mockito.when(ebeanDatabaseMock.find(Share.class).where().in("node_id", Collections.emptyList())) + .thenReturn(expressionListMock); + + // When + shareRepository.deleteSharesBulk(Collections.emptyList()); + + // Then + Mockito.verify(expressionListMock, Mockito.times(1)).delete(); + } +} From 7fc1d58d6e9f341e44bde41f7836d503523ce30f Mon Sep 17 00:00:00 2001 From: Federico Rispo Date: Wed, 14 Feb 2024 11:19:41 +0100 Subject: [PATCH 11/11] fix: add sonarqube suggestions and fix failing IT --- .pre-commit-config.yaml | 2 +- .../impl/ebean/ShareRepositoryEbean.java | 3 +- .../files/api/GetPublicLinksApiIT.java | 11 +- .../dal/dao/ebean/CollaborationLinkTest.java | 41 +++--- .../files/dal/dao/ebean/LinkTest.java | 132 +++++++----------- .../CollaborationLinkRepositoryEbeanTest.java | 6 +- .../impl/ebean/LinkRepositoryEbeanTest.java | 4 +- .../impl/ebean/ShareRepositoryEbeanTest.java | 2 +- 8 files changed, 86 insertions(+), 115 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1c3ed643..1375d193 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -43,7 +43,7 @@ repos: - id: reuse - repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks - rev: v2.11.0 + rev: v2.10.0 hooks: - id: pretty-format-java diff --git a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java index c6ffafe9..f7b5efa7 100644 --- a/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java +++ b/core/src/main/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbean.java @@ -17,7 +17,6 @@ import io.ebean.Transaction; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; public class ShareRepositoryEbean implements ShareRepository { @@ -198,6 +197,6 @@ public List getSharesUsersIds(String nodeId, List sorts) { .query(); sorts.forEach(sort -> sort.getOrderEbeanQuery(query)); - return query.findList().stream().map(Share::getTargetUserId).collect(Collectors.toList()); + return query.findList().stream().map(Share::getTargetUserId).toList(); } } diff --git a/core/src/test/java/com/zextras/carbonio/files/api/GetPublicLinksApiIT.java b/core/src/test/java/com/zextras/carbonio/files/api/GetPublicLinksApiIT.java index 9c5c5c8e..3d1878a3 100644 --- a/core/src/test/java/com/zextras/carbonio/files/api/GetPublicLinksApiIT.java +++ b/core/src/test/java/com/zextras/carbonio/files/api/GetPublicLinksApiIT.java @@ -86,7 +86,8 @@ void createShare(String nodeId, String targetUserId, SharePermission permission) @Test void - givenAnExistingFileWithTwoExistingLinksTheGetLinksShouldReturnAListOfAssociatedLinksOrderedByCreationDescending() { + givenAnExistingFileWithTwoExistingLinksTheGetLinksShouldReturnAListOfAssociatedLinksOrderedByCreationDescending() + throws InterruptedException { // Given createFile("00000000-0000-0000-0000-000000000000", "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"); @@ -97,6 +98,11 @@ void createShare(String nodeId, String targetUserId, SharePermission permission) Optional.of(5L), Optional.of("super-description")); + // These sleep is necessary because some time the creation of the two links is so fast that + // causes the same creation timestamp of the two links. When the LinkRepository will have an + // injected clock in the LinkRepository, then we will have a better solution for this ugly trick + Thread.sleep(10L); + linkRepository.createLink( "0c04783b-bdfb-446f-870c-625f5ae02a0a", "00000000-0000-0000-0000-000000000000", @@ -130,7 +136,8 @@ void createShare(String nodeId, String targetUserId, SharePermission permission) TestUtils.jsonResponseToList(httpResponse.getBodyPayload(), "getLinks"); Assertions.assertThat(publicLinks).hasSize(2); - + System.out.println(publicLinks.get(0).get("id") + " " + publicLinks.get(0).get("created_at")); + System.out.println(publicLinks.get(1).get("id") + " " + publicLinks.get(1).get("created_at")); Assertions.assertThat(publicLinks.get(0)) .containsEntry("id", "0c04783b-bdfb-446f-870c-625f5ae02a0a") .containsEntry( diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLinkTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLinkTest.java index 6da13b18..774f37a0 100644 --- a/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLinkTest.java +++ b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/CollaborationLinkTest.java @@ -10,34 +10,27 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; -public class CollaborationLinkTest { +class CollaborationLinkTest { @Test - void givenAllCollaborationLinkAttributesTheConstructorShouldCreateCollaborationLinkObjectCorrectly() { + void + givenAllCollaborationLinkAttributesTheConstructorShouldCreateCollaborationLinkObjectCorrectly() { // Given & When - CollaborationLink collaborationLink = new CollaborationLink( - UUID.fromString("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5"), - "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", - "abcd1234", - Instant.ofEpochMilli(100L), - ACL.READ - ); + CollaborationLink collaborationLink = + new CollaborationLink( + UUID.fromString("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5"), + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + Instant.ofEpochMilli(100L), + ACL.READ); // Then - Assertions - .assertThat(collaborationLink.getId()) - .isEqualTo(UUID.fromString("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5")); - Assertions - .assertThat(collaborationLink.getNodeId()) - .isEqualTo("ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c"); - Assertions - .assertThat(collaborationLink.getInvitationId()) - .isEqualTo("abcd1234"); - Assertions - .assertThat(collaborationLink.getCreatedAt()) - .isEqualTo(Instant.ofEpochMilli(100L)); - Assertions - .assertThat(collaborationLink.getPermissions()) - .isEqualTo(SharePermission.READ_ONLY); + Assertions.assertThat(collaborationLink.getId()) + .isEqualTo(UUID.fromString("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5")); + Assertions.assertThat(collaborationLink.getNodeId()) + .isEqualTo("ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c"); + Assertions.assertThat(collaborationLink.getInvitationId()).isEqualTo("abcd1234"); + Assertions.assertThat(collaborationLink.getCreatedAt()).isEqualTo(Instant.ofEpochMilli(100L)); + Assertions.assertThat(collaborationLink.getPermissions()).isEqualTo(SharePermission.READ_ONLY); } } diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/LinkTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/LinkTest.java index dfb3ca6c..88b372e3 100644 --- a/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/LinkTest.java +++ b/core/src/test/java/com/zextras/carbonio/files/dal/dao/ebean/LinkTest.java @@ -7,98 +7,70 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; -public class LinkTest { +class LinkTest { @Test void givenAllPublicLinkAttributesTheConstructorShouldCreateLinkObjectCorrectly() { // Given & When - Link publicLink = new Link( - "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", - "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", - "abcd1234", - 5L, - 10L, - "fake description" - ); + Link publicLink = + new Link( + "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + 5L, + 10L, + "fake description"); // Then - Assertions - .assertThat(publicLink.getLinkId()) - .isEqualTo("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5"); - Assertions - .assertThat(publicLink.getNodeId()) - .isEqualTo("ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c"); - Assertions - .assertThat(publicLink.getPublicId()) - .isEqualTo("abcd1234"); - Assertions - .assertThat(publicLink.getCreatedAt()) - .isEqualTo(5L); - Assertions - .assertThat(publicLink.getExpiresAt()) - .isPresent() - .get() - .isEqualTo(10L); - Assertions - .assertThat(publicLink.getDescription()) - .isPresent() - .get() - .isEqualTo("fake description"); + Assertions.assertThat(publicLink.getLinkId()).isEqualTo("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5"); + Assertions.assertThat(publicLink.getNodeId()).isEqualTo("ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c"); + Assertions.assertThat(publicLink.getPublicId()).isEqualTo("abcd1234"); + Assertions.assertThat(publicLink.getCreatedAt()).isEqualTo(5L); + Assertions.assertThat(publicLink.getExpiresAt()).isPresent().get().isEqualTo(10L); + Assertions.assertThat(publicLink.getDescription()) + .isPresent() + .get() + .isEqualTo("fake description"); } @Test void givenPartialPublicLinkAttributesTheSettersShouldUpdateLinkObjectCorrectly() { // Given - Link publicLink = new Link( - "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", - "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", - "abcd1234", - 5L, - null, - null - ); + Link publicLink = + new Link( + "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + 5L, + null, + null); // When - publicLink - .setExpiresAt(10L) - .setDescription("fake description"); + publicLink.setExpiresAt(10L).setDescription("fake description"); // Then - Assertions - .assertThat(publicLink.getLinkId()) - .isEqualTo("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5"); - Assertions - .assertThat(publicLink.getNodeId()) - .isEqualTo("ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c"); - Assertions - .assertThat(publicLink.getPublicId()) - .isEqualTo("abcd1234"); - Assertions - .assertThat(publicLink.getCreatedAt()) - .isEqualTo(5L); - Assertions - .assertThat(publicLink.getExpiresAt()) - .isPresent() - .get() - .isEqualTo(10L); - Assertions - .assertThat(publicLink.getDescription()) - .isPresent() - .get() - .isEqualTo("fake description"); + Assertions.assertThat(publicLink.getLinkId()).isEqualTo("a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5"); + Assertions.assertThat(publicLink.getNodeId()).isEqualTo("ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c"); + Assertions.assertThat(publicLink.getPublicId()).isEqualTo("abcd1234"); + Assertions.assertThat(publicLink.getCreatedAt()).isEqualTo(5L); + Assertions.assertThat(publicLink.getExpiresAt()).isPresent().get().isEqualTo(10L); + Assertions.assertThat(publicLink.getDescription()) + .isPresent() + .get() + .isEqualTo("fake description"); } @Test void givenANullExpiredAtTheGetExpiredAtShouldReturnAnOptionalEmpty() { // Given & When - Link publicLink = new Link( - "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", - "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", - "abcd1234", - 5L, - null, - null - ); + Link publicLink = + new Link( + "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + 5L, + null, + null); // Then Assertions.assertThat(publicLink.getExpiresAt()).isEmpty(); @@ -107,14 +79,14 @@ void givenANullExpiredAtTheGetExpiredAtShouldReturnAnOptionalEmpty() { @Test void givenAnExpirationTimestampEqualsToZeroTheGetExpiredAtShouldReturnAnOptionalEmpty() { // Given - Link publicLink = new Link( - "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", - "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", - "abcd1234", - 5L, - 10L, - null - ); + Link publicLink = + new Link( + "a7fd1b7c-9f2c-40e4-83d7-463d4d7ab9c5", + "ac3da3d3-b1c0-41f1-b0ab-2b6d7328808c", + "abcd1234", + 5L, + 10L, + null); // When publicLink.setExpiresAt(0L); diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbeanTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbeanTest.java index 5b75ab46..b634b6ca 100644 --- a/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbeanTest.java +++ b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/CollaborationLinkRepositoryEbeanTest.java @@ -27,7 +27,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mockito; -public class CollaborationLinkRepositoryEbeanTest { +class CollaborationLinkRepositoryEbeanTest { private CollaborationLinkRepository collaborationLinkRepository; private Database ebeanDatabaseMock; @@ -201,7 +201,7 @@ void givenANotExistingNodeIdTheGetLinkByNodeIdShouldReturnAnEmptyStream() { collaborationLinkRepository.getLinksByNodeId("not-existing-node-id"); // Then - Assertions.assertThat(collaborationLinkStream).hasSize(0); + Assertions.assertThat(collaborationLinkStream).isEmpty(); } @Test @@ -242,6 +242,6 @@ void givenAnEmptyListOfCollaborationLinkIdTheDeleteLinksShouldDoNothing() { // Then Mockito.verify(expressionListMock, Mockito.times(1)).delete(); - Assertions.assertThat(uuidsCaptor.getValue()).hasSize(0); + Assertions.assertThat(uuidsCaptor.getValue()).isEmpty(); } } diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbeanTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbeanTest.java index 0802fdd6..14b4770d 100644 --- a/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbeanTest.java +++ b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/LinkRepositoryEbeanTest.java @@ -21,7 +21,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mockito; -public class LinkRepositoryEbeanTest { +class LinkRepositoryEbeanTest { private LinkRepository linkRepository; private Database ebeanDatabaseMock; @@ -244,7 +244,7 @@ void givenAnExistingNodeIdWithoutAssociatedLinksTheGetLinkByNodeIdShouldReturnAn "11111111-1111-1111-1111-111111111111", LinkSort.CREATED_AT_ASC); // Then - Assertions.assertThat(links).hasSize(0); + Assertions.assertThat(links).isEmpty(); } @Test diff --git a/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbeanTest.java b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbeanTest.java index 090ea31c..a2861415 100644 --- a/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbeanTest.java +++ b/core/src/test/java/com/zextras/carbonio/files/dal/repositories/impl/ebean/ShareRepositoryEbeanTest.java @@ -25,7 +25,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mockito; -public class ShareRepositoryEbeanTest { +class ShareRepositoryEbeanTest { private Database ebeanDatabaseMock; private ShareRepository shareRepository;