Uploaded image for project: 'Syncope'
  1. Syncope
  2. SYNCOPE-866

Check for existence of key before adding template

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-M4
    • Component/s: core
    • Labels:
      None

      Description

      The create(String key) method in the MailTemplateLogic and ReportTemplateLogic classes of the org.apache.syncope.core.logic package should check if the key being added to the MailTemplateDAO instance is already present. If such a key already exists, it should throw an appropriate exception for the same.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user tmess567 opened a pull request:

          https://github.com/apache/syncope/pull/18

          Added Exception for existing key (SYNCOPE-866)

          I have added a new Exception named *KeyAlreadyExistsException* and attached it to the *MailTemplateLogic* and *ReportTemplateLogic* classes to check for duplicate keys and throw the new Exception.

          I have also tested this out on Swagger and attached screenshots of the error for both *MailTemplates*

          ![mailtemplatekeyalreadyexistsexception](https://cloud.githubusercontent.com/assets/9494086/15772586/843cf8dc-298f-11e6-9627-f05962c92fad.png)

          and *ReportTemplates*.

          ![reporttemplatekeyalreadyexistsexception](https://cloud.githubusercontent.com/assets/9494086/15772587/844468e2-298f-11e6-9330-3fc0d40ebb94.png)

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/tmess567/syncope SYNCOPE-866

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/syncope/pull/18.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #18


          commit 42ecce4b5b35f7b540c77b65faf39f6b1bd9b7db
          Author: Tushar <tusharm567@gmail.com>
          Date: 2016-06-03T07:54:02Z

          Added Exception for existing key (SYNCOPE-866)


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user tmess567 opened a pull request: https://github.com/apache/syncope/pull/18 Added Exception for existing key ( SYNCOPE-866 ) I have added a new Exception named * KeyAlreadyExistsException * and attached it to the * MailTemplateLogic * and * ReportTemplateLogic * classes to check for duplicate keys and throw the new Exception. I have also tested this out on Swagger and attached screenshots of the error for both * MailTemplates * ! [mailtemplatekeyalreadyexistsexception] ( https://cloud.githubusercontent.com/assets/9494086/15772586/843cf8dc-298f-11e6-9627-f05962c92fad.png ) and * ReportTemplates *. ! [reporttemplatekeyalreadyexistsexception] ( https://cloud.githubusercontent.com/assets/9494086/15772587/844468e2-298f-11e6-9330-3fc0d40ebb94.png ) You can merge this pull request into a Git repository by running: $ git pull https://github.com/tmess567/syncope SYNCOPE-866 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/syncope/pull/18.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #18 commit 42ecce4b5b35f7b540c77b65faf39f6b1bd9b7db Author: Tushar <tusharm567@gmail.com> Date: 2016-06-03T07:54:02Z Added Exception for existing key ( SYNCOPE-866 )
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ilgrosso commented on a diff in the pull request:

          https://github.com/apache/syncope/pull/18#discussion_r65673131

          — Diff: core/logic/src/main/java/org/apache/syncope/core/logic/MailTemplateLogic.java —
          @@ -83,6 +84,13 @@ public MailTemplateTO transform(final MailTemplate input) {

          @PreAuthorize("hasRole('" + StandardEntitlement.MAIL_TEMPLATE_CREATE + "')")
          public MailTemplateTO create(final String key) {
          + for (MailTemplateTO mailTemplateTO: list()) {
          — End diff –

          Take a look at [ResourceLogic](https://github.com/apache/syncope/blob/master/core/logic/src/main/java/org/apache/syncope/core/logic/ResourceLogic.java#L121-L123) and do accodingly.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/18#discussion_r65673131 — Diff: core/logic/src/main/java/org/apache/syncope/core/logic/MailTemplateLogic.java — @@ -83,6 +84,13 @@ public MailTemplateTO transform(final MailTemplate input) { @PreAuthorize("hasRole('" + StandardEntitlement.MAIL_TEMPLATE_CREATE + "')") public MailTemplateTO create(final String key) { + for (MailTemplateTO mailTemplateTO: list()) { — End diff – Take a look at [ResourceLogic] ( https://github.com/apache/syncope/blob/master/core/logic/src/main/java/org/apache/syncope/core/logic/ResourceLogic.java#L121-L123 ) and do accodingly.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ilgrosso commented on a diff in the pull request:

          https://github.com/apache/syncope/pull/18#discussion_r65673143

          — Diff: core/logic/src/main/java/org/apache/syncope/core/logic/ReportTemplateLogic.java —
          @@ -84,6 +85,12 @@ public ReportTemplateTO transform(final ReportTemplate input) {

          @PreAuthorize("hasRole('" + StandardEntitlement.REPORT_TEMPLATE_CREATE + "')")
          public ReportTemplateTO create(final String key) {
          + for (ReportTemplateTO reportTemplateTO: list()) {
          — End diff –

          Take a look at [ResourceLogic](https://github.com/apache/syncope/blob/master/core/logic/src/main/java/org/apache/syncope/core/logic/ResourceLogic.java#L121-L123) and do accodingly.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/18#discussion_r65673143 — Diff: core/logic/src/main/java/org/apache/syncope/core/logic/ReportTemplateLogic.java — @@ -84,6 +85,12 @@ public ReportTemplateTO transform(final ReportTemplate input) { @PreAuthorize("hasRole('" + StandardEntitlement.REPORT_TEMPLATE_CREATE + "')") public ReportTemplateTO create(final String key) { + for (ReportTemplateTO reportTemplateTO: list()) { — End diff – Take a look at [ResourceLogic] ( https://github.com/apache/syncope/blob/master/core/logic/src/main/java/org/apache/syncope/core/logic/ResourceLogic.java#L121-L123 ) and do accodingly.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ilgrosso commented on a diff in the pull request:

          https://github.com/apache/syncope/pull/18#discussion_r65673151

          — Diff: core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/KeyAlreadyExistsException.java —
          @@ -0,0 +1,35 @@
          +/*
          — End diff –

          This class is not needed, please remove and use instead `org.apache.syncope.core.persistence.api.dao.DuplicateException` (see above).

          Show
          githubbot ASF GitHub Bot added a comment - Github user ilgrosso commented on a diff in the pull request: https://github.com/apache/syncope/pull/18#discussion_r65673151 — Diff: core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/KeyAlreadyExistsException.java — @@ -0,0 +1,35 @@ +/* — End diff – This class is not needed, please remove and use instead `org.apache.syncope.core.persistence.api.dao.DuplicateException` (see above).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ilgrosso commented on the issue:

          https://github.com/apache/syncope/pull/18

          Thanks for this PR @tmess567, I appreciate it very much.
          I have added some comments to avoid duplicate classes and stay close to what other Logic classes do.

          Please fix with an additional commit.
          Thank you very much.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/18 Thanks for this PR @tmess567, I appreciate it very much. I have added some comments to avoid duplicate classes and stay close to what other Logic classes do. Please fix with an additional commit. Thank you very much.
          Show
          githubbot ASF GitHub Bot added a comment - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/18 Please also add a test case on [ReportTemplateITCase] ( https://github.com/apache/syncope/blob/master/fit/core-reference/src/test/java/org/apache/syncope/fit/core/ReportTemplateITCase.java ) and on [MailTemplateITCase] ( https://github.com/apache/syncope/blob/master/fit/core-reference/src/test/java/org/apache/syncope/fit/core/MailTemplateITCase.java ) named `issueSYNCOPE866()`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tmess567 commented on the issue:

          https://github.com/apache/syncope/pull/18

          So, in the crud method, I will be calling the issueSYNCOPE866() method? Or should I just add a Test Case directly into the crud method?

          Show
          githubbot ASF GitHub Bot added a comment - Github user tmess567 commented on the issue: https://github.com/apache/syncope/pull/18 So, in the crud method, I will be calling the issueSYNCOPE866() method? Or should I just add a Test Case directly into the crud method?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ilgrosso commented on the issue:

          https://github.com/apache/syncope/pull/18

          Ok, let's do things one step at a time.
          Please first address my comments in code, we will come up with testing later on, ok?

          Show
          githubbot ASF GitHub Bot added a comment - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/18 Ok, let's do things one step at a time. Please first address my comments in code, we will come up with testing later on, ok?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tmess567 commented on the issue:

          https://github.com/apache/syncope/pull/18

          That's done. I'll commit right away.

          Show
          githubbot ASF GitHub Bot added a comment - Github user tmess567 commented on the issue: https://github.com/apache/syncope/pull/18 That's done. I'll commit right away.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ilgrosso commented on the issue:

          https://github.com/apache/syncope/pull/18

          Ok, the changes are just fine.

          Now, you need to add a couple of methods like as follows

          ```java
          @Test
          public void issueSYNCOPE866() {

          }
          ```

          to the two classes mentioned above.

          In the body of such methods you will, by taking reference with other methods in the same class, essentially:

          1. create a report template with an existing key
          1. assert that such creation operation fails with

          The following is a working sample - that you can just grab and put there - for report template:

          ```java
          @Test
          public void issueSYNCOPE866() {
          ReportTemplateTO reportTemplateTO = new ReportTemplateTO();
          reportTemplateTO.setKey("empty");
          try

          { reportTemplateService.create(reportTemplateTO); fail(); }

          catch (SyncopeClientException e)

          { assertEquals(ClientExceptionType.EntityExists, e.getType()); }

          }
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/18 Ok, the changes are just fine. Now, you need to add a couple of methods like as follows ```java @Test public void issueSYNCOPE866() { } ``` to the two classes mentioned above. In the body of such methods you will, by taking reference with other methods in the same class, essentially: 1. create a report template with an existing key 1. assert that such creation operation fails with The following is a working sample - that you can just grab and put there - for report template: ```java @Test public void issueSYNCOPE866() { ReportTemplateTO reportTemplateTO = new ReportTemplateTO(); reportTemplateTO.setKey("empty"); try { reportTemplateService.create(reportTemplateTO); fail(); } catch (SyncopeClientException e) { assertEquals(ClientExceptionType.EntityExists, e.getType()); } } ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tmess567 commented on the issue:

          https://github.com/apache/syncope/pull/18

          Done. Please check.

          Show
          githubbot ASF GitHub Bot added a comment - Github user tmess567 commented on the issue: https://github.com/apache/syncope/pull/18 Done. Please check.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ilgrosso commented on the issue:

          https://github.com/apache/syncope/pull/18

          LGTM, please squash your three commits into one, and don't forget to mention SYNCOPE-866 in the final commit log (the very first used here will work, anyway).

          I will merge this PR at earliest next Monday 6th, after that the freeze period due to the ongoing release VOTE is over.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ilgrosso commented on the issue: https://github.com/apache/syncope/pull/18 LGTM, please squash your three commits into one, and don't forget to mention SYNCOPE-866 in the final commit log (the very first used here will work, anyway). I will merge this PR at earliest next Monday 6th, after that the freeze period due to the ongoing release VOTE is over.
          Hide
          ilgrosso Francesco Chicchiriccò added a comment - - edited

          PR merged, thank you very much Tushar Mishra!

          Show
          ilgrosso Francesco Chicchiriccò added a comment - - edited PR merged , thank you very much Tushar Mishra !
          Hide
          ilgrosso Francesco Chicchiriccò added a comment -

          Bulk close for 2.0.0-M4

          Show
          ilgrosso Francesco Chicchiriccò added a comment - Bulk close for 2.0.0-M4

            People

            • Assignee:
              ilgrosso Francesco Chicchiriccò
              Reporter:
              tusharm Tushar Mishra
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development