Mailing-List: contact dev-help@geronimo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geronimo.apache.org
Received-SPF: pass (nike.apache.org: local policy)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
  s=dk20050327; d=earthlink.net;
  b=akYsauv2MaWTj8Gh3/a//YIB9CgYP7dgRWU971yhnESS8E0dCTmoXSu2fFv14Nrz;
  h=Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:References:In-Reply-To:Content-Type:Content-Transfer-Encoding:X-ELNK-Trace:X-Originating-IP;
Message-ID: <47BE5299.90108@earthlink.net>
Date: Thu, 21 Feb 2008 23:42:01 -0500
From: Joe Bohn <joe.bohn@earthlink.net>
User-Agent: Thunderbird 2.0.0.9 (Macintosh/20071031)
MIME-Version: 1.0
To: dev@geronimo.apache.org
Subject: Re: [Fwd: Re: NPE exporting a plugin ... null returned from
 getPluginMetadata()]
References: <47BE4F3D.5010903@earthlink.net>
In-Reply-To: <47BE4F3D.5010903@earthlink.net>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Joe Bohn wrote:
> forwarding messages to dev list for further discussion.
> 
> 
> -------- Original Message --------
> Subject: Re: NPE exporting a plugin ... null returned from 
> getPluginMetadata()
> Date: Thu, 21 Feb 2008 14:42:36 -0800 (PST)
> From: David Jencks <david_jencks@yahoo.com>
> To: Joe Bohn <joe.bohn@earthlink.net>
> 
> This should be on the dev list :-)  Feel free to forward my contributions.
> 
> As I understand the issue we want all applications/modules to have some 
> kind of geronimo-plugin.xml.  Lets look at where that could happen
> 
> -- deployers could construct them.  Right now I don't see quite how to 
> do this in one place, but it might let us put more of the info into the 
> plan or an auxilary plan.  I've wondered about combining the plan and 
> geronimo-plugin.xml file.
I agree this would perhaps be the best.  Combining the plan and 
geronimo-plugin.xml is an interesting idea.  I think that makes a lot of 
sense if we can provide or construct reasonable defaults so that the 
geronimo-plugin info need only be specified if the user really has 
intentions of exporting a plugin.  Would this then result in the 
configuration being considered as if it was an installed plugin and 
available for other functions such as inclusion when exporting a server?
> 
> -- config stores could insert them when you install a module.  I think 
> this might be the best plan for now if all the info is available for 
> constructing a default geronimo-plugin.xml
> 
> -- config stores or the GeronimoSourceRepository could provide a default 
> if there's nothing there when you ask.  I'm not too thrilled with this, 
> it makes me feel like the config store is lying.  The SourceRepository 
> might be ok.
I think this would be ok in the short term.  It seems that this is the 
function that was in place earlier.  The nice thing about it is that it 
is only created when necessary (and then only for transient purposes 
since it isn't persisted).  AFAIK this is only used when exporting 
plugins at the moment.
> 
> -- plugin installer could construct it if missing.  I think this is my 
> second choice at the moment.
> 
> One thing I haven't figured out is if the console lets you just edit the 
> plugin info or only includes the edited info in the export.  Do you know?
Some of the information can be edited but not all.  For example, the 
Unique ID cannot be edited but some other fields (such as the human 
readable name) can be.
> 
> thanks
> david jencks
> 
> ----- Original Message ----
> From: Joe Bohn <joe.bohn@earthlink.net>
> To: David Jencks <david_jencks@yahoo.com>
> Sent: Thursday, February 21, 2008 1:40:19 PM
> Subject: NPE exporting a plugin ... null returned from getPluginMetadata()
> 
> I'm looking into a problem where we get a NPE when attempting to export 
> a configuration as a plugin (I'm driving it from the console Plugin 
> portlet .. GERONIMO-3866 & GERONIMO-3867 - they are actually the same 
> root problem).
> 
> This code in ExportConfigHandler.renderView() is the source of the problem:
>     PluginType data = 
> ManagementHelper.getManagementHelper(request).getPluginInstaller().getPluginMetadata( 
> 
>                 Artifact.create(configId));
>     PluginArtifactType instance = data.getPluginArtifact().get(0);
> 
> The last line is getting the NPE because the 
> pluginInstaller.getPluginMetadata for the artifact is returning null. 
> I've verified that the artifact itself appears valid after being created.
> 
> Digging a bit deeper I discovered that in 
> GeronimoSourceRepository.extractPluginMetadata(File dir) we have the 
> following check:
> 
>    if (!xml.isFile() || !xml.canRead() || xml.length() == 0) {
>       return null;
>    }
> 
> So this is why we get null for the metadata and why we fail the export 
> of a plugin.  It appears that the process of extractingPluginMetadata 
> assumes it is working with a deployed plugin and judging from the code 
> in the portlet I would assume this has not always been the case.
> 
> I found slightly different code in place back when the original function 
> logic was included in PluginInstallerGBean.  When it was moved to 
> GeronimoSourceRespository (in rev. 604483) the additional check for 
> configId was lost.  The original code was this:
> 
> 
>    if (!xml.isFile() || !xml.canRead() || xml.length() == 0) {
>       if (moduleId != null) {
>           return createDefaultMetadata(moduleId);
>       } else {
>           return null;
>       }
>    }
> 
> I think this additional check would allow the export plugin to function 
> properly by creating some default metadata (I'm trying it out now).
> 
> Would it be a problem if I introduced that capability into the 
> GeornimoSourceRepository.extractPluginMetadata(File dir)?   Of course I 
> would have to pass along the configId to make it work.  I'm trying it 
> out now but didn't want to do something that would cause you grief.
> 
> Thanks,
> Joe
> 
>