Now
On Fri, Aug 28, 2009 at 4:28 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).
>
> if
> (DeploymentFileData.isServiceArchiveFile(file.getName())) {
> addFileToDeploy(file,
> deploymentEngine.getServiceDeployer(),
> - WSInfo.TYPE_SERVICE);
> + WSInfo.TYPE_SERVICE);
> + noServicesXML = false;
>
> noServicesXML = false; --> Does't this mean; foundServiceXML = true?
>
> 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];
Yeah, there may be issues in this implementation, but that does not mean
that you simply reject it because you think it is ugly. We can review the
implementation & provide valuable feedback and suggestions on how this could
be properly done, so that it does not break anything & also see how we can
make REST work.
>
>
>
> > 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
>
>
--
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
