More refactoring. Moved manifest parsing into module constructor. (FELIX-851)


git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@737120 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
index 4b3ebd3..98ead5e 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
@@ -900,15 +900,22 @@
         // create an associated module for it.
         Map headerMap = m_archive.getRevision(
             m_archive.getRevisionCount() - 1).getManifestHeader();
-        ManifestParser mp = new ManifestParser(
-            getFramework().getLogger(), getFramework().getConfig(), headerMap);
 
-        // Verify that the bundle symbolic name and version is unique.
-        if (mp.getManifestVersion().equals("2"))
+        final int revision = m_archive.getRevisionCount() - 1;
+        ModuleImpl module = new ModuleImpl(
+            getFramework().getLogger(),
+            getFramework().getConfig(),
+            getFramework().getResolver(),
+            Long.toString(getBundleId()) + "." + Integer.toString(revision),
+            headerMap,
+            m_archive.getRevision(revision).getContent());
+
+        // Verify that the bundle symbolic name + version is unique.
+        if (module.getManifestVersion().equals("2"))
         {
-            Version bundleVersion = mp.getBundleVersion();
+            Version bundleVersion = module.getVersion();
             bundleVersion = (bundleVersion == null) ? Version.emptyVersion : bundleVersion;
-            String symName = mp.getSymbolicName();
+            String symName = module.getSymbolicName();
 
             Bundle[] bundles = getFramework().getBundles();
             for (int i = 0; (bundles != null) && (i < bundles.length); i++)
@@ -927,24 +934,8 @@
             }
         }
 
-        // Now that we have parsed and verified the module metadata, we
-        // can actually create the module. Note, if this is an extension
-        // bundle it's exports are removed, aince they will be added to
-        // the system bundle directly later on.
-        final int revision = m_archive.getRevisionCount() - 1;
-        IModule module = new ModuleImpl(
-            getFramework().getLogger(),
-            getFramework().getConfig(),
-            getFramework().getResolver(),
-            Long.toString(getBundleId()) + "." + Integer.toString(revision),
-            m_archive.getRevision(revision).getContent(),
-            headerMap,
-            (ExtensionManager.isExtensionBundle(headerMap)) ? null : mp.getCapabilities(),
-            mp.getRequirements(),
-            mp.getDynamicRequirements(),
-            mp.getLibraries());
-
-        // Set the content loader's URL policy.
+        // Set the module's URL policy.
+// TODO: REFACTOR - Pass into constructor?
         module.setURLPolicy(
 // TODO: REFACTOR - SUCKS NEEDING URL POLICY PER MODULE.
             new URLPolicyImpl(
@@ -952,28 +943,6 @@
                 getFramework().getBundleStreamHandler(),
                 module));
 
-        // Verify that all native libraries exist in advance; this will
-        // throw an exception if the native library does not exist.
-        // TODO: CACHE - It would be nice if this check could be done
-        //               some place else in the module, perhaps.
-        R4Library[] libs = module.getNativeLibraries();
-        for (int i = 0; (libs != null) && (i < libs.length); i++)
-        {
-            String entryName = libs[i].getEntryName();
-            if (entryName != null)
-            {
-                if (module.getContent().getEntryAsNativeLibrary(entryName) == null)
-                {
-                    throw new BundleException("Native library does not exist: " + entryName);
-// TODO: REFACTOR - We have a memory leak here since we added a module above
-//                  and then don't remove it in case of an error; this may also
-//                  be a general issue for installing/updating bundles, so check.
-//                  This will likely go away when we refactor out the module
-//                  factory, but we will track it under FELIX-835 until then.
-                }
-            }
-        }
-
         return module;
     }
 
diff --git a/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java b/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
index 63d01c0..4cf3367 100644
--- a/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
+++ b/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
@@ -139,7 +139,7 @@
      * @param config the configuration to read properties from.
      * @param systemBundleInfo the info to change if we need to add exports.
      */
-    ExtensionManager(Logger logger, Map configMap)
+    ExtensionManager(Logger logger, Map configMap) throws BundleException
     {
         m_module = new ExtensionManagerModule();
         m_extensions = null;
@@ -243,15 +243,6 @@
     }
 
     /**
-     * Check whether the given manifest headers are from an extension bundle.
-     */
-    static boolean isExtensionBundle(Map headers)
-    {
-        return (ManifestParser.parseExtensionBundleHeader((String)
-                headers.get(Constants.FRAGMENT_HOST)) != null);
-    }
-
-    /**
      * Add an extension bundle. The bundle will be added to the parent classloader
      * and it's exported packages will be added to the module definition
      * exports of this instance. Subsequently, they are available form the
@@ -622,9 +613,9 @@
 
     class ExtensionManagerModule extends ModuleImpl
     {
-        ExtensionManagerModule()
+        ExtensionManagerModule() throws BundleException
         {
-            super(m_logger, null, null, "0", null, null, null, null, null, null);
+            super(m_logger, null, null, "0", null, null);
         }
 
         public Map getHeaders()
diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java
index ad3a82e..65561d9 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -1600,7 +1600,7 @@
                     // an extension bundle then don't allow it to be resolved
                     // again as per spec.
                     if (!bundle.isExtension() &&
-                        ExtensionManager.isExtensionBundle(
+                        Util.isExtensionBundle(
                             bundle.getCurrentModule().getHeaders()))
                     {
                         addSecurity(bundle);
diff --git a/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java b/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java
index 552fab2..f3fd48b 100644
--- a/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java
@@ -41,22 +41,30 @@
 import org.apache.felix.framework.util.manifestparser.ManifestParser;
 import org.apache.felix.framework.util.manifestparser.R4Library;
 import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleException;
 import org.osgi.framework.Constants;
+import org.osgi.framework.Version;
 
 public class ModuleImpl implements IModule
 {
     private final Logger m_logger;
-
-    private Bundle m_bundle = null;
-
+    private final Map m_configMap;
+    private final FelixResolver m_resolver;
+    private final String m_id;
+    private final IContent m_content;
     private final Map m_headerMap;
+
+    private final String m_manifestVersion;
+    private final Version m_version;
+    private final String m_symbolicName;
+
     private final ICapability[] m_capabilities;
     private final IRequirement[] m_requirements;
     private final IRequirement[] m_dynamicRequirements;
     private final R4Library[] m_nativeLibraries;
 
-    private final String m_id;
-    private volatile String m_symbolicName = null;
+    private Bundle m_bundle = null;
+
     private IModule[] m_fragments = null;
     private IWire[] m_wires = null;
     private IModule[] m_dependentHosts = new IModule[0];
@@ -64,10 +72,6 @@
     private IModule[] m_dependentRequirers = new IModule[0];
     private volatile boolean m_isResolved = false;
 
-    // TODO: REFACTOR - Fields below are from ContentLoaderImpl
-    private final Map m_configMap;
-    private final FelixResolver m_resolver;
-    private final IContent m_content;
     private IContent[] m_contentPath;
     private IContent[] m_fragmentContents = null;
     private IURLPolicy m_urlPolicy = null;
@@ -82,22 +86,78 @@
     // Re-usable security manager for accessing class context.
     private static SecurityManagerEx m_sm = new SecurityManagerEx();
 
-// TODO: REFACTOR - Should the module constructor parse the manifest?
     public ModuleImpl(Logger logger, Map configMap, FelixResolver resolver,
-        String id, IContent content, Map headerMap,
-        ICapability[] caps, IRequirement[] reqs, IRequirement[] dynReqs,
-        R4Library[] nativeLibs)
+        String id, Map headerMap, IContent content) throws BundleException
     {
         m_logger = logger;
         m_configMap = configMap;
         m_resolver = resolver;
         m_id = id;
-        m_content = content;
         m_headerMap = headerMap;
-        m_capabilities = caps;
-        m_requirements = reqs;
-        m_dynamicRequirements = dynReqs;
-        m_nativeLibraries = nativeLibs;
+        m_content = content;
+
+        // We need to special case the system bundle module, which does not
+        // have a content.
+        if (m_content != null)
+        {
+            ManifestParser mp = new ManifestParser(m_logger, m_configMap, m_headerMap);
+
+            // Record some of the parsed metadata. Note, if this is an extension
+            // bundle it's exports are removed, since they will be added to the
+            // system bundle directly later on.
+            m_manifestVersion = mp.getManifestVersion();
+            m_version = mp.getBundleVersion();
+            m_capabilities = (Util.isExtensionBundle(m_headerMap))
+                ? null : mp.getCapabilities();
+            m_requirements = mp.getRequirements();
+            m_dynamicRequirements = mp.getDynamicRequirements();
+            m_nativeLibraries = mp.getLibraries();
+
+            // Get symbolic name.
+            String symName = null;
+            for (int capIdx = 0;
+                (m_capabilities != null) && (capIdx < m_capabilities.length);
+                capIdx++)
+            {
+                if (m_capabilities[capIdx].getNamespace().equals(ICapability.MODULE_NAMESPACE))
+                {
+                    symName = (String) m_capabilities[capIdx].getProperties().get(
+                        Constants.BUNDLE_SYMBOLICNAME_ATTRIBUTE);
+                    break;
+                }
+            }
+            m_symbolicName = symName;
+
+            // Verify that all native libraries exist in advance; this will
+            // throw an exception if the native library does not exist.
+            for (int i = 0; (m_nativeLibraries != null) && (i < m_nativeLibraries.length); i++)
+            {
+                String entryName = m_nativeLibraries[i].getEntryName();
+                if (entryName != null)
+                {
+                    if (m_content.getEntryAsNativeLibrary(entryName) == null)
+                    {
+                        throw new BundleException("Native library does not exist: " + entryName);
+// TODO: REFACTOR - Do we still have a memory leak here?
+//                  We have a memory leak here since we added a module above
+//                  and then don't remove it in case of an error; this may also
+//                  be a general issue for installing/updating bundles, so check.
+//                  This will likely go away when we refactor out the module
+//                  factory, but we will track it under FELIX-835 until then.
+                    }
+                }
+            }
+        }
+        else
+        {
+            m_manifestVersion = null;
+            m_version = null;
+            m_capabilities = null;
+            m_requirements = null;
+            m_dynamicRequirements = null;
+            m_nativeLibraries = null;
+            m_symbolicName = null;
+        }
 
         // Read the boot delegation property and parse it.
 // TODO: REFACTOR - This used to be per framework, now it is per module
@@ -121,6 +181,16 @@
         }
    }
 
+    Logger getLogger()
+    {
+        return m_logger;
+    }
+
+    FelixResolver getResolver()
+    {
+        return m_resolver;
+    }
+
     public synchronized Bundle getBundle()
     {
         return m_bundle;
@@ -139,21 +209,18 @@
         return m_id;
     }
 
+    public String getManifestVersion()
+    {
+        return m_manifestVersion;
+    }
+
+    public Version getVersion()
+    {
+        return m_version;
+    }
+
     public String getSymbolicName()
     {
-        if (m_symbolicName == null)
-        {
-            for (int capIdx = 0;
-                (m_symbolicName == null) && (m_capabilities != null) && (capIdx < m_capabilities.length);
-                capIdx++)
-            {
-                if (m_capabilities[capIdx].getNamespace().equals(ICapability.MODULE_NAMESPACE))
-                {
-                    m_symbolicName = (String) m_capabilities[capIdx].getProperties().get(
-                        Constants.BUNDLE_SYMBOLICNAME_ATTRIBUTE);
-                }
-            }
-        }
         return m_symbolicName;
     }
 
@@ -598,18 +665,6 @@
         return tmp;
     }
 
-    // TODO: REFACTOR - Below are from ContentLoaderImpl
-
-    Logger getLogger()
-    {
-        return m_logger;
-    }
-
-    FelixResolver getResolver()
-    {
-        return m_resolver;
-    }
-
     public synchronized void close()
     {
         m_content.close();
diff --git a/framework/src/main/java/org/apache/felix/framework/util/Util.java b/framework/src/main/java/org/apache/felix/framework/util/Util.java
index 685ddba..1372157 100644
--- a/framework/src/main/java/org/apache/felix/framework/util/Util.java
+++ b/framework/src/main/java/org/apache/felix/framework/util/Util.java
@@ -27,6 +27,7 @@
 import java.util.Properties;
 
 import org.apache.felix.framework.util.manifestparser.Capability;
+import org.apache.felix.framework.util.manifestparser.ManifestParser;
 import org.apache.felix.moduleloader.*;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.Constants;
@@ -35,6 +36,15 @@
 public class Util
 {
     /**
+     * Check whether the given manifest headers are from an extension bundle.
+     */
+    public static boolean isExtensionBundle(Map headers)
+    {
+        return (ManifestParser.parseExtensionBundleHeader((String)
+                headers.get(Constants.FRAGMENT_HOST)) != null);
+    }
+
+    /**
      * Converts a module identifier to a bundle identifier. Module IDs
      * are typically <tt>&lt;bundle-id&gt;.&lt;revision&gt;</tt>; this
      * method returns only the portion corresponding to the bundle ID.