Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Memory IdentityRepository and contract tests #4423

Closed
quantranhong1999 opened this issue Nov 8, 2021 · 5 comments
Closed

Memory IdentityRepository and contract tests #4423

quantranhong1999 opened this issue Nov 8, 2021 · 5 comments
Assignees
@chibenwa
Milestone
Sprint #16

Comments

@quantranhong1999
Copy link
Member

quantranhong1999 commented Nov 8, 2021 •
edited

Why

We need a repository APIs to store user custom identity.

How

  • Move Identity POJO from jmap-8621 to data-jmap, name field should be Option
case class Identity(id: IdentityId,
                    name: Option[IdentityName],
                    email: MailAddress,
                    replyTo: Option[List[EmailAddress]],
                    bcc: Option[List[EmailAddress]],
                    textSignature: Option[TextSignature],
                    htmlSignature: Option[HtmlSignature],
                    mayDelete: MayDeleteIdentity)
                    
case class IdentityCreationRequest(name: Option[IdentityName],
                    email: MailAddress,
                    replyTo: Option[List[EmailAddress]],
                    bcc: Option[List[EmailAddress]],
                    textSignature: Option[TextSignature],
                    htmlSignature: Option[HtmlSignature])
  • Define IdentityRepository:
Publisher<Identity> save(Username username, IdentityCreationRequest identityCreationRequest);

Publisher<Void> update(Username username, Identity newIdentity);

Publisher<Identity> findByIds(Username username, Set<IdentityId> ids);

Publisher<Identity> findAll(Username username);

Publisher<Void> remove(Username username, IdentityId id);

DoD

IdentityRepository interface + Memory implementation + contract tests

Note

  • Inject IdentityFactory and do merging identities stuff at this Repository layer (for update, findByIds, findAll, remove). Then we would separate a little logic from DAO layer.
  • If name, textSignature, htmlSignature is empty => set to "" string by default
  • User custom identities should have mayDelete = true by default
  • When update, if record not found => throw exception
  • We have to check email: is the user able to send from input address? (inject CanSendFrom). Otherwise throw exception
  • We should validate htmlSignature (This text MUST be an HTML snippet to be inserted into the <body></body> section of the HTML). I think we should validate this upon Jmap deserialization.
@chibenwa
Copy link
Member

chibenwa commented Nov 8, 2021

revoke => remove IMO

@chibenwa chibenwa added this to the Sprint #16 milestone Nov 8, 2021
@vttranlina
Copy link
Member

// this bellow just is common my comment

When I work with the JPA library, I realized that it has one code convention for the method name.

  • If the method name starts with "findBy...", the result can return have value or empty.
  • If the method name starts with "getBy...", the result implicates is a value or throw NotFound exception.

IMO it is a good convention, more clear. In James, sometimes I need to read inside the implement for sure

@vttranlina
Copy link
Member

If the POJO wrote by scala, IMO the interface/implement repository should be written by scala too. (I know it is not mandatory)
But it will help us less boilerplate than for the convert between two language

@quantranhong1999
Copy link
Member Author

When I work with the JPA library, I realized that it has one code convention for the method name.

Publisher<Identity> findByIds(Username username, Set<IdentityId> ids);

Publisher<Identity> findAll(Username username);

Does this comply with you?

This was referenced Nov 8, 2021
Closed
Closed
@chibenwa
Copy link
Member

chibenwa commented Nov 9, 2021

IMO it is a good convention, more clear. In James, sometimes I need to read inside the implement for sure

👍 looks like a good convention, let's try to stick to it!

@quantranhong1999 quantranhong1999 mentioned this issue Nov 9, 2021
Closed
@chibenwa chibenwa self-assigned this Nov 10, 2021
@Arsnael Arsnael closed this as completed Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chibenwa chibenwa

Labels
None yet
Projects
None yet
Milestone
Sprint #16
Development

No branches or pull requests
4 participants
@chibenwa @Arsnael @quantranhong1999 @vttranlina