[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Dan Allen updated ARQ-322: -------------------------- Summary: Implement a GlassFish 3.1 managed container adapter (was: Implement a GlassFish 3 managed container) Affects Version/s: glassfish_1.0.0.CR2 (was: 1.0.0.Alpha4) Priority: Major (was: Minor) Description: To round out the GlassFish support, we need a managed container adapter (see the JBoss AS managed container). (was: To round out the GlassFish support, we need a managed container (see the JBoss AS managed container).) I've implemented a working prototype of the container adapter. It's functional and validated by tests. There is still work remaining to make it release ready. * review code and clean it up (feel free to refactor anything) * ear deployment does not return context path in ProtocolMetaData (see ear test) * add support for JVM flags (use property javaJvmArguments) * add support for custom debug port (use property debugPort) * add support for deploying glassfish-resources.xml (requires custom cleanup, may need to file separate issue) It's also worrisome how much code is copy-paste from the 3.1 remote container. Consider creating a common module that is shared between them. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Dan Allen commented on ARQ-322: ------------------------------- Btw, if you want to get more fancy w/ the ServerControl, check out this Java wrapper for asadmin. I used jar -jar whereas this code uses the asadmin command directly, but both work. https://github.com/Codeartisans/asadmin/tree/master/asadmin-java -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Vineet Reynolds commented on ARQ-322: ------------------------------------- There's a cleaned up version of Dan Allen's prototype here: https://github.com/VineetReynolds/arquillian-container-glassfish/tree/ARQ... I'll comment on the points noted above, with regard to the original prototype, that are still not addressed and might not addressed right away. * add support for JVM flags (use property javaJvmArguments) - GlassFish does not allow for new JVM flags and arguments to be specified for the GlassFish process via an asadmin command. This requires changes to be made to the domain configuration via the asadmin create-jvm-options command (and a rollback via the asadmin delete-jvm-options command), and a possible domain restart. We could raise this feature request in the GlassFish JIRA. * add support for custom debug port (use property debugPort) - Again, GlassFish does not allow for the debug port to be changed at startup through an asadmin commmand. The domain configuration file must be modified for a custom debug port to be used. We could raise this feature request in the GlassFish JIRA. * add support for deploying glassfish-resources.xml - This could be done via the `asadmin add-resources` command. But it would incur a cost at the moment, for the creation of resources cannot be rolled back easily. Aslak has already filed a feature request against GlassFish JIRA (http://java.net/jira/browse/GLASSFISH-16240) for this purpose. I'm of the opinion that we should wait for this feature to appear in GlassFish instead of building in the logic to parse the sun-resources/glassfish-resources.xml files to manage each individual resource. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Vineet Reynolds commented on ARQ-322: ------------------------------------- A FIXME is also pending from the original prototype. In GlassFishManagedContainerConfiguration, a FIXME states that the check for the admin-cli.jar file should be more robust. I'd like some inputs here, for this appears to be sufficient. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Dan Allen commented on ARQ-322: ------------------------------- In follow-up to your other commands: ---- *deploying glassfish-resources.xml* I agree that ideally we should wait on the fix in GlassFish. If that stalls, I think we could probably hit most of the use cases by parsing the XML file ourselves and issuing the individual delete-* commands in reverse. Let's open a separate tracking issue for that. *setting the debug port* I also agree that the start-domain command should accept a debug port. I did, however, look into what it would take to change the port in the container adapter. In a nutshell, you can only change the debug port when the DAS is running (or you have to edit the domain.xml file manually). Here's the command: {code} asadmin set server.java-config.debug-options=-Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=9009 {code} There is another issue at hand here. In order for the developer to make use of debugging w/ the managed container, it must be possible to set the suspend flag on the jdwp option. So --debug alone isn't sufficient (again, probably something start-domain should support). Here's the code that would be required to do this today: {code:title=GlassFishServerControl#start()} if (config.isDebug()) { result = executeAdminCommand("Setting debug options", "set", Arrays.asList(config.getTarget() + ".java-config.debug-options=" + "-Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=" + config.getDebugSocketPort())); if (result > 0) { executeAdminDomainCommand("Stopping container", "stop-domain", new ArrayList<String>()); throw new LifecycleException("Could not set debug options"); } // NOTE we can't use restart-domain with suspend=y because connecting the debugger // does not resume startup result = executeAdminDomainCommand("Stopping container", "stop-domain", new ArrayList<String>()); if (result > 0) { throw new LifecycleException("Could not stop container to restart in debug mode"); } result = executeAdminDomainCommand("Starting container in debug mode", "start-domain", Arrays.asList("--debug")); if (result > 0) { throw new LifecycleException("Could not restart container in debug mode"); } } {code} {code:title=GlassFishServerControl#stop()} if (config.isDebug()) { result = executeAdminCommand("Restoring debug options", "set", Arrays.asList(config.getTarget() + ".java-config.debug-options=" + "-Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=" + config.getDebugSocketPort())); if (result > 0) { logger.warning("Could not reset debug options, you may need to change them manually"); } } {code} and of course the configuration property: {code:title=GlassFishManagedContainerConfiguration} /** * Socket port to use for debugging */ private int debugSocketPort = 9009; public int getDebugSocketPort() { return debugSocketPort; } public void setDebugSocketPort(int debugSocketPort) { this.debugSocketPort = debugSocketPort; } {code} *add support for JVM flags* If setting the debug port wasn't complex enough, the JVM arguments are just crazy. Again, JVM options can be set with the DAS is running. However, it only supports deleting or appending, not replacing. To do this reliably, you have to read in the JVM options, figure out if the ones you want to add overlap, etc, etc. Here's the asadmin command set to set an option: {code} asadmin --echo create-jvm-options -Xmx512m {code} This is really where I think manually editing the domain.xml would make sense, because working with the available commands is like having your hands tied. ---- There is one other option to all of this. We could support creating a domain configuration to set custom properties such as the debug port and jvm options, then restart into that configuration. The configuration is created using the copy-config command [1]. The benefit to this approach is that it would also side step the issue of having to revert any changes we make for the test run (such as the debug port). [1] http://docs.oracle.com/cd/E18930_01/html/821-2433/copy-config-1.html#scro... -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Vineet Reynolds commented on ARQ-322: ------------------------------------- Yes, we're almost ready for pushing this into master. I've just noticed that we do not incorporate a check to verify if the GlassFish DAS is already running, just like it is done for the managed JBoss and Tomcat containers. I'll include this check and request a pull request. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Dan Allen commented on ARQ-322: ------------------------------- Also, I think we can get rid of the ShrinkWrapUtil in the remote and managed containers (since you are using streams for deployments now). I'm pushing one additional minor fix that will remove an annoyance for me w/ embedded...a missing deleteOnExit() for the temporary directory. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Dan Allen commented on ARQ-322: ------------------------------- Aha!!! The REST API no longer allows you to request XML as a response type. If you change the "accepts" from MediaType.APPLICATION_XML to MediaType.APPLICATION_JSON, you'll see that it's trying to return this string: {"message":"","command":"server","exit_code":"SUCCESS","extraProperties":{"commands":[],"methods":[{"name":"GET"},{}],"childResources":{"server":"http:\/\/localhost:4848\/management\/domain\/servers\/server\/server"}}} Can you gut out the XML parsing and replace with JSON parsing? Then we'll be in good shape. Btw, I'm pretty sure the admin REST API has always support JSON responses. For some reason, Jason picked XML when we wrote the first prototype. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Dan Allen commented on ARQ-322: ------------------------------- Oh, rats. That wasn't it. I think we still want to change to JSON because it should be faster to parse. Getting a decent JSON parsing library in there would clean up a lot of that response code. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Dan Allen commented on ARQ-322: ------------------------------- {color:green}*Green bar!!*{color} Just add the following line in GlassFishClientUtil#prepareClient() {code} client.addFilter(new CsrfProtectionFilter()); {code} Make sure it's the client filter (com.sun.jersey.api.client.filter.CsrfProtectionFilter). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Vineet Reynolds edited comment on ARQ-322 at 3/12/12 2:09 AM: -------------------------------------------------------------- Yes, adding in the CSRF protection header, allows the deployment to GlassFish to proceed with the deployment. Here's my change for now. {code:java} public Map POSTMultiPartRequest(String additionalResourceUrl, FormDataMultiPart form) { ClientResponse response = prepareClient(additionalResourceUrl).type(MediaType.MULTIPART_FORM_DATA_TYPE) .header("X-Requested-By", "Arquillian") .post(ClientResponse.class, form); Map responseMap = getResponseMap(response); return responseMap; } {code} Edit: I'll add the client-side CsrfProtectionFilter, since the above isn't something exposed by the Jersey API. was (Author: vineet.reynolds): Yes, adding in the CSRF protection header, allows the deployment to GlassFish to proceed with the deployment. Here's my change for now. {code:java} public Map POSTMultiPartRequest(String additionalResourceUrl, FormDataMultiPart form) { ClientResponse response = prepareClient(additionalResourceUrl).type(MediaType.MULTIPART_FORM_DATA_TYPE) .header("X-Requested-By", "Arquillian") .post(ClientResponse.class, form); Map responseMap = getResponseMap(response); return responseMap; } {code} -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Vineet Reynolds updated ARQ-322: -------------------------------- Status: Pull Request Sent (was: Open) Git Pull Request: https://github.com/arquillian/arquillian-container-glassfish/pull/17 Pull request sent. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Dan Allen commented on ARQ-322: ------------------------------- Ah, right, I should have guessed there was another issue based on the fact that we've discussed it in the past. Thanks for the pointer. As for XML vs JSON, I wonder which would be faster. I would imagine that the developer would prefer speed over one less dependency, since parse speed affects every test execution. I could see the extra dep as a problem if it had to be packaged, but in this case it's purely for client-side usage. Should we at least do an analysis of the speed? If so, I can open a separate issue. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Aslak Knutsen updated ARQ-322: ------------------------------ Fix Version/s: glassfish_1.0.0.Final Git Pull Request: https://github.com/arquillian/arquillian-container-glassfish/pull/17 (was: https://github.com/arquillian/arquillian-container-glassfish/pull/17) -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/ARQ-322?page=com.atlassian.jira.plugin.sy... ] Aslak Knutsen updated ARQ-322: ------------------------------ Fix Version/s: glassfish_1.0.0.CR3 (was: glassfish_1.0.0.Final) -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira