You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@syncope.apache.org by tmess567 <gi...@git.apache.org> on 2016/06/03 08:04:35 UTC
[GitHub] syncope pull request #18: Added Exception for existing key (SYNCOPE-866)
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**
and **ReportTemplates**.
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 <tu...@gmail.com>
Date: 2016-06-03T07:54:02Z
Added Exception for existing key (SYNCOPE-866)
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope issue #18: Added Exception for existing key (SYNCOPE-866)
Posted by tmess567 <gi...@git.apache.org>.
Github user tmess567 commented on the issue:
https://github.com/apache/syncope/pull/18
Done. Please check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope pull request #18: Added Exception for existing key (SYNCOPE-866)
Posted by ilgrosso <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope issue #18: Added Exception for existing key (SYNCOPE-866)
Posted by ilgrosso <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope issue #18: Added Exception for existing key (SYNCOPE-866)
Posted by tmess567 <gi...@git.apache.org>.
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?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope pull request #18: Added Exception for existing key (SYNCOPE-866)
Posted by ilgrosso <gi...@git.apache.org>.
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).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope issue #18: Added Exception for existing key (SYNCOPE-866)
Posted by ilgrosso <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope issue #18: Added Exception for existing key (SYNCOPE-866)
Posted by ilgrosso <gi...@git.apache.org>.
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()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope pull request #18: Added Exception for existing key (SYNCOPE-866)
Posted by ilgrosso <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope pull request #18: Added Exception for existing key (SYNCOPE-866)
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/syncope/pull/18
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope issue #18: Added Exception for existing key (SYNCOPE-866)
Posted by tmess567 <gi...@git.apache.org>.
Github user tmess567 commented on the issue:
https://github.com/apache/syncope/pull/18
That's done. I'll commit right away.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope issue #18: Added Exception for existing key (SYNCOPE-866)
Posted by ilgrosso <gi...@git.apache.org>.
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());
}
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] syncope issue #18: Added Exception for existing key (SYNCOPE-866)
Posted by ilgrosso <gi...@git.apache.org>.
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?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---