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
>>
>>
>