Hi Deepal,
On Fri, Aug 28, 2009 at 9:58 PM, Deepal jayasinghe <[email protected]>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. 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).
No. I think you are mistaken. This noServiceXML flag is used it identify
directories which don't contain a META-INF/services.xml file and don't have
any sub directories. This means this folder is not deployable. I've logged
it in such cases.
if (noServicesXML) {
log.error(Messages.getMessage(DeploymentErrorMsgs.SERVICE_XML_NOT_FOUND));
}
>
>
> 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.
No. I took this scenario into account even before I implement this. See the
code in URI based operation dispatcher. It handles this scenario correctly.
> (I think he has only
> test the scenario which supposed to work, not the cases which introduces
> issues).
I'm not telling you that it's perfect. May be I've missed some scenarios.
But If you can please test it and point me out, I can fix those issues if
there are any.
Thanks,
~Isuru
> 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 <[email protected]
> > <mailto:[email protected]>> 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
> > <[email protected]
> > <mailto:[email protected]>>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/
