search

tsgrp / OpenContent

TSG's Web Services for ECM Repositories
8 stars 4 forks source link

AlfrescoEmbContentImpl unnecessary serviceRegistry validation of ticket #25

Open jrubins opened 10 years ago

jrubins commented 10 years ago

A lot of methods in AlfrescoEmbContentImpl start with the following line (or the same concept):

ServiceRegistryProxy.getServiceRegistry(ticket).getAuthenticationService().validate(ticket)

This is redundant as the call to ServiceRegistryProxy.getServiceRegistry(ticket) already validates the ticket before returning the serviceRegistry:

public static ServiceRegistry getServiceRegistry(String ticket) {
    SERVICE_REGISTRY.getAuthenticationService().validate(ticket);
    return SERVICE_REGISTRY;
}

It seems like these calls could be cleaned up.

gsteimer commented 10 years ago

There was a very specific (and important) reason we added this validation call to each implementation method.

@benallenallen @dgrumieaux - do you remember the details? Could it be that Alfresco added the validation in a later release vs. what we were using in 2012 (Alfresco 4.0.2.9). If that's the case, then should we remove the duplicated calls from OC, or leave them in there to have compatibility with older Alfresco releases?

dgrumieaux commented 10 years ago

I discussed this with Jon yesterday evening, and Jon is right - we simply are calling the .validate() twice.

@jrubins, the .validate() call within the ServiceRegistryProxy.getServiceRegistry(ticket) call is the only one we need. Please remove any subsequent calls in OC code outside of the ServiceRegistryProxy class.

gsteimer commented 10 years ago

@dgrumieaux - but was this something that wasn't present in Alfresco 4.0.2.9? If that's the case, removing the validate calls in OC will break older Alfresco repos.

gsteimer commented 10 years ago

Or, am I missing something? I'm assuming the getServiceRegistry method is part of Alfresco's code. If it's actually an OC method, then yeah, absolutely - remove the redundant calls to validate()

dgrumieaux commented 10 years ago

Exactly right, we created the ServiceRegistryProxy.getServiceRegistry(ticket) method to validate ALL calls through the serviceRegistry.

jrubins commented 10 years ago

Yeah sorry, I wasn't very clear in my description. The getServiceRegistry method is in our custom ServiceRegistryProxy class in 2.alfrescoEmb module.