Il 13/10/2015 15:13, Francesco Chicchiriccò ha scritto:
On 13/10/2015 14:52, Massimiliano Perrone wrote:
Il 13/10/2015 13:03, Francesco Chicchiriccò ha scritto:
On 13/10/2015 12:50, Massimiliano Perrone wrote:
Hi all,
I found this "strange" behavior working with ConfigurationService class.

When I try to delete a configuration I get always a valid response also when the configuration key doesn't exist (while I was expecting a NotFound error). Reading the code I found below difference from (1) ConfigurationLogic and, for instance, (2) SchemaLogic classes:

(1)
@PreAuthorize("hasRole('" + Entitlement.CONFIGURATION_DELETE + "')")
    public void delete(final String schema) {
        confDAO.delete(schema);
    }

(2)
@PreAuthorize("hasRole('" + Entitlement.SCHEMA_DELETE + "')")
public void delete(final SchemaType schemaType, final String schemaName) {
        if (!doesSchemaExist(schemaType, schemaName)) {
throw new NotFoundException(schemaType + "/" + schemaName);
        }

        switch (schemaType) {
            case VIRTUAL:
                virSchemaDAO.delete(schemaName);
                break;

            case DERIVED:
                derSchemaDAO.delete(schemaName);
                break;

            case PLAIN:
            default:
                plainSchemaDAO.delete(schemaName);
        }
    }

As you can read the second class has a control on schema existence, the first one hasn't.
It is the right behavior?

Nice catch: please open an issue - and possibly provide a fix as well if you can; essentially, the method above should become similar to [1] or [2].

Hi Francesco,
following your suggestion I added the check before the delete operation, it seems to work but...

But developing the test for it I found that the exception isn't the expected one. As you can read below to catch the exception I added the generic Exception class (3) so, printed the class name, I found that it is a javax.xml.ws.WebServiceException exception (4).

Furthermore during the CLI development I had to always catch the WebServiceException exception but, until now, I thought it was the right exception to catch, but now I have some doubts.


(3)
try {
            configurationService.delete("nonExistent");
            fail("NOT POSSIBLE");
        } catch (SyncopeClientException e) {
assertEquals(Response.Status.NOT_FOUND, e.getType().getResponseStatus());
        } catch (Exception e) {
            e.printStackTrace();
            fail(e.getMessage() + " C " + e.getClass());
        }

(4)
java.lang.AssertionError: Remote exception with status code: NOT_FOUND C class javax.xml.ws.WebServiceException

When throwing NotFoundException form ConfigurationLogic, such exception is converted into an HTTP message by RestServiceExceptionMapper [3]; moreover, if using SyncopeClient (as you're doing), the RestClientExceptionMapper [4] attempts to convert the HTTP message back into a Java exception.

The exception you should received from the DELETE invocation (once fixed) is exactly the same that you receive when trying to GET an non-existing configuration attribute, e.g.

curl -X GET --header "Accept: application/json" --header "Authorization: Basic YWRtaW46cGFzc3dvcmQ=" "http://localhost:9080/syncope/rest/configurations/non-existing";

returns

< HTTP/1.1 404 Not Found
< Date: Tue, 13 Oct 2015 13:06:53 GMT
* Server Apache-Coyote/1.1 is not blacklisted
< Server: Apache-Coyote/1.1
< X-Application-Error-Code: NotFound
< X-Application-Error-Info: NotFound:NotFoundException: Configuration schema caz
< X-Syncope-Domain: Master
< Content-Type: application/json;charset=UTF-8
< Transfer-Encoding: chunked
<
* Connection #1 to host syncopecore2-tirasa.rhcloud.com left intact
{"status":404,"type":"NotFound","elements":["NotFoundException: Configuration schema caz"]}

which looks fine to me.

Using SyncopeClient:

        try {
            new SyncopeClientFactoryBean().
                    setAddress(ADDRESS).
                    create(ADMIN_UNAME, ADMIN_PWD).
                    getService(ConfigurationService.class).
                    get("non-existing");
            fail();
        } catch (SyncopeClientException e) {
            assertEquals(ClientExceptionType.NotFound, e.getType());
        }

I don't understand why your code is not working in a similar fashion, then.

Incidentally, I've noticed that the test

org.apache.syncope.fit.core.reference.ConfigurationITCase.delete()

needs to be adjusted as part of SYNCOPE-707.

Recompiling the code it seems to work as expected. Thanks for the support, I'm going to push and close the issue.


Regards.

[1] https://github.com/apache/syncope/blob/master/core/logic/src/main/java/org/apache/syncope/core/logic/RoleLogic.java#L88-L100 [2] https://github.com/apache/syncope/blob/master/core/logic/src/main/java/org/apache/syncope/core/logic/RealmLogic.java#L85-L96
[3] https://github.com/apache/syncope/blob/master/core/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/RestServiceExceptionMapper.java#L214-L216 [4] https://github.com/apache/syncope/blob/master/client/lib/src/main/java/org/apache/syncope/client/lib/RestClientExceptionMapper.java


--
Massimiliano Perrone
Tel +39 393 9121310

Tirasa S.r.l.
Viale D'Annunzio 267 - 65127 Pescara
Tel +39 0859116307 / FAX +39 0859111173
http://www.tirasa.net

"L'apprendere molte cose non insegna l'intelligenza"
(Eraclito)

Reply via email to