On Fri, Aug 28, 2009 at 9:58 PM, Deepal jayasinghe <deep...@gmail.com>wrote:
> I just went though the patch and which has a number of assumptions which
> introduce so many issues. First as I can see it might break the
> directory based deployment, the reason is the logic differentiate
> sub-directory from a service just considering have services.xml.
Yes. If a subdirectory contains a services XML, it is sent to be deployed.
Otherwise it is sent through another iteration to find services in the sub
directory. Is it wrong? Am I missing something?
I think it is the way already existing code treats directory bases services.
It checks for the service XML and if it is not found, throws and exception
saying "no services XML found". Just look at the code in the
processServiceGroup method of the ArchiveReader class.
File file = new File(filename, SERVICES_XML);
if (!file.exists()) {
// try for meta-inf
file = new File(filename, SERVICES_XML.toLowerCase());
}
if (file.exists()) {
InputStream in = null;
try {
in = new FileInputStream(file);
axisServiceGroup.setServiceGroupName(currentFile.getName());
return buildServiceGroup(in, currentFile,
axisServiceGroup, wsdlServices, configCtx);
} catch (FileNotFoundException e) {
throw new DeploymentException(
Messages.getMessage(DeploymentErrorMsgs.FILE_NOT_FOUND,
e.getMessage()));
} catch (XMLStreamException e) {
throw new DeploymentException(
Messages.getMessage(DeploymentErrorMsgs.XML_STREAM_EXCEPTION,
e.getMessage()));
} finally {
if (in != null) {
try {
in.close();
} catch (IOException e) {
log.info
(Messages.getMessage("errorininputstreamclose"));
}
}
}
} else {
throw new DeploymentException(
Messages.getMessage(DeploymentErrorMsgs.SERVICE_XML_NOT_FOUND));
}
Thanks,
~Isuru
> And
> just look at the following code, if the service is an archive file it
> assume it does not have a serviceXML which is wrong (I might have also
> done such mistake, but I am just pointing out).
>
> if
> (DeploymentFileData.isServiceArchiveFile(file.getName())) {
> addFileToDeploy(file,
> deploymentEngine.getServiceDeployer(),
> - WSInfo.TYPE_SERVICE);
> + WSInfo.TYPE_SERVICE);
> + noServicesXML = false;
>
>
> At the runtime this make some very wrong assumptions, that is if the
> service is not found that it assume it might be hierarchical service and
> find the service from second part of the URL. I believe this break the
> REST behavior and normal operation dispatching, for example logic would
> assume operation name as the hierachical service. (I think he has only
> test the scenario which supposed to work, not the cases which introduces
> issues). You will understand what I am talking about if you just go
> through the patch.
>
> * Ex : services/echo/echoString --> values[0] = echo, values[1] =
> echoString
> + * values[2] = echo/echoString
> + */
> + String[] values = new String[3];
>
>
> > Of course this needs unit tests. The unit test should check whether
> > the existing service deployment properly works, w.r.t. deployment &
> > invocation, as well as should check the hierarchical service deployment.
> >
> > Azeez
> >
> > On Fri, Aug 28, 2009 at 3:38 PM, Davanum Srinivas <dava...@gmail.com
> > <mailto:dava...@gmail.com>> wrote:
> >
> > Azeez,
> >
> > So is this also one of the scenarios where unit tests are not
> > needed/necessary? :) Just for my education...
> >
> > -- dims
> >
> >
> > On 08/28/2009 11:16 AM, Afkham Azeez wrote:
> >
> > On Fri, Aug 28, 2009 at 3:00 PM, Andreas Veithen
> > <andreas.veit...@gmail.com
> > <mailto:andreas.veit...@gmail.com>>wrote:
> >
> > Guys,
> >
> > I share Deepal's concern about the possible impact of this
> > change. He
> > mentioned the WS-Addressing case, but I believe that this
> > change will
> > also break autogeneration of WSDLs: probably the endpoint
> > URLs in the
> > WSDLs will be wrong.
> >
> >
> >
> > Yes, it is a good thing that Isuru took the time to start a
> > discussion this
> > on the list, even though the policy is CTR, and made sure that
> > everybody's
> > concerns are heard. It is a good example to follow.
> >
> >
> >
> > Andreas
> >
> >
> >
> >
> >
> > --
> > Thanks
> > Afkham Azeez
> >
> > Blog: http://afkham.org
> > Developer Portal: http://www.wso2.org
> > WSAS Blog: http://wso2wsas.blogspot.com
> > Company: http://wso2.com
> > GPG Fingerprint: 643F C2AF EB78 F886 40C9 B2A2 4AE2 C887 665E 0760
>
>
> --
> Thank you!
>
>
> http://blogs.deepal.org
> http://deepal.org
>
>
--
Senior Software Engineer,
WSO2 Inc. http://wso2.org/
Blog : http://isurues.wordpress.com/
