dev@glassfish.java.net

Re: About the GLASSFISH-16651

From: Tang Yong <tangyong_at_cn.fujitsu.com>
Date: Thu, 12 Jul 2012 14:47:28 +0900

Dear Sahoo,

Thanks your reply very much!

> 1. I don't understand the reason behind changing WARManifestProcessor.
> That code enforces the spec requirements about what can be overridden
> and what can't be. Is it a leftover from your earlier change?
I am sorry that it is a my earlier version, in fact, nowaday,
WARManifestProcessor should not be modified.

About the point, I will firstly update gf trunk and then, implement the
contribution version based the newest source.

> You can then treat everything except uriScheme as query parameters
>while constructing the final URI. That way, you don't have to worry
>about input validation. That will be handled by the stream handler.
right. I think also.

> 3. OSGiDeployer may not be the right place to make the new URI,
>because by the OSGiDeployer is called user's jar would have been
>exploded into a directory and I don't think
>WebBundleURLStreamHandlerFactory can handle any input stream other
than >a JarInputStream.
About the point , I have known WebBundleURLStreamHandlerFactory
can not handle the directory as inputstream. And I also realised
OSGiDeployer may not be the right place.

>Did it actually work for you?
Now, the current prototype work for me because I modified the
OSGiDeployer.makeBundleLocation method and modified the bundle's
original location with context.getOriginalSource().getURI(). However,
this will cause a problem: bundle.getLocation() will not return
domain/domain1/XXXX , so, this is why OSGiDeployer may not be the right
place to make the new URI.

>Can't you just change >OSGiArchiveHandler >like you had done earlier?
On the previous thread, I have refered that :

On expand method, if using WebBundleURLStreamHandler service, will
bring coupling between osgi-container and osgi-web-container, just as
the following, the reason is at the point of getting
WebBundleURLStreamHandlerService , osgi registry has other
URLStreamHandlerService implementations besides
WebBundleURLStreamHandlerService:

for (ServiceReference r : urlhandlers) {
    urlhandler = (URLStreamHandlerService) osgicontext.getService(r);
    if (urlhandler instanceof WebBundleURLStreamHandlerService) {
                break;
    }
}

So, I selected OSGiDeployer.

--Best Regard!
--Tang


Sahoo wrote:
> Hi Tang,
>
> Here are my comments about the latest code available in your git repo:
>
> 1. I don't understand the reason behind changing WARManifestProcessor.
> That code enforces the spec requirements about what can be overridden
> and what can't be. Is it a leftover from your earlier change?
>
> 2. About passing properties in deploy command:
> Allow user to provide the properties as shown below:
>
> asadmin deploy --properties
> uriScheme=webBundle:Bundle-SymbolicName=foo:Import-Package=javax.servlet:Web-ContextPath=/foo
> /tmp/foo.war
>
> This will result in following configuration in domain.xml:
>
> <application location="${com.sun.aas.instanceRootURI}/applications/foo/"
> name="foo" object-type="user">
> <property name="Import-Package" value="javax.servlet"></property>
> <property name="uriScheme" value="webBundle"></property>
> <property name="appLocation"
> value="${com.sun.aas.instanceRootURI}/applications/__internal/foo/foo.war"></property>
>
> <property name="Web-ContextPath" value="/foo"></property>
> <property name="defaultAppName" value="foo"></property>
> <property name="Bundle-SymbolicName" value="foo"></property>
> <module name="foo">
> <engine sniffer="osgi"></engine>
> </module>
> </application>
>
> You can then treat everything except uriScheme as query parameters while
> constructing the final URI. That way, you don't have to worry about
> input validation. That will be handled by the stream handler.
>
> 3. OSGiDeployer may not be the right place to make the new URI, because
> by the OSGiDeployer is called user's jar would have been exploded into a
> directory and I don't think WebBundleURLStreamHandlerFactory can handle
> any input stream other than a JarInputStream. Did it actually work for
> you? Can't you just change OSGiArchiveHandler like you had done earlier?
>
> Have you signed the contributor agreement [1] that's required to supply
> code to GlassFish project? If not, please do that so that we can have
> more involved discussion.
>
> Thanks,
> Sahoo
>
> [1] http://glassfish.java.net/public/devindex.html
> On Wednesday 11 July 2012 01:12 PM, Tang Yong wrote:
>> Dear Sahoo, Hong
>> CC: Tom
>>
>> Now, the new prototype has been implemented and wish you to review it.
>>
>> On the new prototype, there are the following changes:
>>
>> 1 decoupling between osgi-container and osgi-web-container
>>
>> According to new investigation, Modifying OSGiArchiveHandler.expand
>> is not a good place, although it is a place to rewrite Manifest.MF to
>> glassfish\domains\domain1\applications\XXXX. Originally, for reusing
>> WebBundleURLStreamHandlerService, I added the coupling between
>> osgi-container and osgi-web-container and breaked the dependency.
>>
>> On the new prototype, by means of modifying OSGiDeployer class simply,
>> while executing "getBundleContext().installBundle", by constructing a
>> new url string, osgi-web-container module can intercept and handle the
>> url with webbundle scheme. Thus, also removing the coupling between
>> osgi-container and osgi-web-container, and I think it is a more easy
>> modify way!
>>
>> 2 about the new using way
>>
>> Now, according to sahoo's suggestion, a user can use the following way:
>>
>> 1) asadmin deploy --type=osgi --properties
>> uriScheme=wab:Web-ContextPath=/test_sample1 e:\test_sample1.war
>>
>> 2) asadmin deploy --type=osgi --properties
>> uriScheme=wab:Web-ContextPath=/test_sample1:Bundle-SymbolicName=test1
>> e:\test_sample1.war
>>
>> ...
>>
>> Note1:
>> On sahoo's suggestion, wish user to use the following way:
>>
>> asadmin deploy --type=osgi --properties uriScheme=wab
>> queryParams="Web-ContextPath=/test_sample1" e:\test_sample1.war
>>
>> However, when I implemented and tested the above using way, I found that
>> GF cannot support the case, and the following error appeared:
>>
>> Invalid property syntax, "=" in value:
>> queryParams=Web-ContextPath=/test_sample1Invalid property syntax, "=" in
>> value:
>>
>> So, I changed the using way.
>>
>> Note2:about context.getAppProps().getProperty
>> On the earlier thread, Hong has pointed that
>> context.getAppProps().getProperty should get values of options. However,
>> if executing context.getAppProps().getProperty on
>> OSGiArchiveHandler.expand, I can not get values of options, because at
>> that time, "deploymentContext.getAppProps().putAll(properties) " still
>> has not been executed. This is not a bug, and thanks Hong for letting me
>> to check and consider implementation of prototype again.
>>
>> [Problem]
>> After using the new prototype to deploy a wab, the contents of
>> Manifest.MF of glassfish\domains\domain1\applications\XXXX have not been
>> rewritten, because OSGiArchiveHandler.expand is executed before
>> OSGiDeployer.prepare. should be also rewritten although not influencing
>> wab's deploying and executing?
>>
>> --Best Regard!
>> --Tang
>>
>> Sahoo wrote:
>>> On Tuesday 10 July 2012 06:25 PM, Tang Yong wrote:
>>>> Dear Hong, sahoo,
>>>>
>>>> I have modified the prototype and removed the container-related
>>>> logic on
>>>> DeployCommand and DeploymentContextImpl.
>>>>
>>>> Now, using the following way can deploy a wab:
>>>>
>>>> 1)asadmin deploy --type=osgi --properties Web-ContextPath=/test_sample1
>>>> e:\test_sample1.war
>>>>
>>>> 2)asadmin deploy --type=osgi e:\test_sample2.war
>>>>
>>> Very cool. I browsed your code and here are some comments:
>>>
>>> OSGiArchiveHandler cannot not know about WAB - WAB support is provided
>>> by osgi-web-container, so the dependency needs to be broken. I think if
>>> you take properties like the following, you can avoid the decoupling:
>>>
>>> asadmin deploy --type osgi --properties uriScheme=wab queryParams="comma
>>> separated name=value pairs"
>>>
>>> then you can just create a new URI inside OSGiArchiveHandler.expand().
>>> You can then obtain an InputStream from the new URI and use the same.
>>>
>>>
>>>> In addition, about
>>>>> DeploymentContext.getAppProps through the deployment lifecycle
>>>> I have confirmed that when using "--properties
>>>> Web-ContextPath=/test_sample1", "DeploymentContext.getAppProps" can not
>>>> get the value of "Web-ContextPath" option. However, using
>>>> "context.getCommandParameters(DeployCommandParameters.class).properties.getProperty("Web-ContextPath")"
>>>>
>>>> can get the value of "Web-ContextPath" option. So, I think that a bug
>>>> may be exist in DeploymentContextImpl class.
>>>>
>>> You should file a bug.
>>>
>>> Sahoo
>>>
>>>
>>
>
>
>