Return-Path: X-Original-To: apmail-sling-commits-archive@www.apache.org Delivered-To: apmail-sling-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 578216F2B for ; Mon, 4 Jul 2011 06:28:09 +0000 (UTC) Received: (qmail 43733 invoked by uid 500); 4 Jul 2011 06:28:08 -0000 Delivered-To: apmail-sling-commits-archive@sling.apache.org Received: (qmail 43662 invoked by uid 500); 4 Jul 2011 06:27:58 -0000 Mailing-List: contact commits-help@sling.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@sling.apache.org Delivered-To: mailing list commits@sling.apache.org Received: (qmail 43648 invoked by uid 99); 4 Jul 2011 06:27:52 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 04 Jul 2011 06:27:52 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 04 Jul 2011 06:27:48 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 10FF32388994; Mon, 4 Jul 2011 06:27:27 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1142562 - in /sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal: ContentLoaderService.java Loader.java Date: Mon, 04 Jul 2011 06:27:27 -0000 To: commits@sling.apache.org From: cziegeler@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20110704062727.10FF32388994@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: cziegeler Date: Mon Jul 4 06:27:26 2011 New Revision: 1142562 URL: http://svn.apache.org/viewvc?rev=1142562&view=rev Log: SLING-2114 : Content Loader is not thread safe Modified: sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentLoaderService.java sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/Loader.java Modified: sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentLoaderService.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentLoaderService.java?rev=1142562&r1=1142561&r2=1142562&view=diff ============================================================================== --- sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentLoaderService.java (original) +++ sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/ContentLoaderService.java Mon Jul 4 06:27:26 2011 @@ -35,7 +35,6 @@ import javax.jcr.Session; import javax.jcr.Value; import javax.jcr.lock.LockException; -import org.apache.jackrabbit.JcrConstants; import org.apache.sling.commons.mime.MimeTypeService; import org.apache.sling.engine.SlingSettingsService; import org.apache.sling.jcr.api.SlingRepository; @@ -144,7 +143,10 @@ public class ContentLoaderService implem // we can safely add the content at this point. try { session = this.getSession(); - final boolean isUpdate = this.updatedBundles.remove(bundle.getSymbolicName()); + final boolean isUpdate; + synchronized ( this.updatedBundles ) { + isUpdate = this.updatedBundles.remove(bundle.getSymbolicName()); + } initialContentLoader.registerBundle(session, bundle, isUpdate); } catch (Throwable t) { log.error( @@ -158,7 +160,9 @@ public class ContentLoaderService implem case BundleEvent.UPDATED: // we just add the symbolic name to the list of updated bundles // we will use this info when the new start event is triggered - this.updatedBundles.add(bundle.getSymbolicName()); + synchronized ( this.updatedBundles ) { + this.updatedBundles.add(bundle.getSymbolicName()); + } break; case BundleEvent.UNINSTALLED: try { Modified: sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/Loader.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/Loader.java?rev=1142562&r1=1142561&r2=1142562&view=diff ============================================================================== --- sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/Loader.java (original) +++ sling/trunk/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/Loader.java Mon Jul 4 06:27:26 2011 @@ -60,15 +60,12 @@ public class Loader extends BaseImportLo private ContentLoaderService contentLoaderService; - private final DefaultContentCreator contentCreator; - // bundles whose registration failed and should be retried private List delayedBundles; public Loader(ContentLoaderService contentLoaderService) { super(); this.contentLoaderService = contentLoaderService; - this.contentCreator = new DefaultContentCreator(contentLoaderService); this.delayedBundles = new LinkedList(); } @@ -255,6 +252,7 @@ public class Loader extends BaseImportLo log.debug("Installing initial content from bundle {}", bundle.getSymbolicName()); + final DefaultContentCreator contentCreator = new DefaultContentCreator(this.contentLoaderService); try { while (pathIter.hasNext()) { @@ -277,7 +275,7 @@ public class Loader extends BaseImportLo if (targetNode != null) { installFromPath(bundle, entry.getPath(), entry, targetNode, - entry.isUninstall() ? createdNodes : null); + entry.isUninstall() ? createdNodes : null, contentCreator); } } } @@ -307,7 +305,7 @@ public class Loader extends BaseImportLo } // finally checkin versionable nodes - for (final Node versionable : this.contentCreator.getVersionables()) { + for (final Node versionable : contentCreator.getVersionables()) { versionable.checkin(); } @@ -326,7 +324,7 @@ public class Loader extends BaseImportLo "Failure to rollback partial initial content for bundle {}", bundle.getSymbolicName(), re); } - this.contentCreator.clear(); + contentCreator.clear(); for (Session session : createdSessions.values()) { session.logout(); } @@ -351,10 +349,11 @@ public class Loader extends BaseImportLo final String path, final PathEntry configuration, final Node parent, - final List createdNodes) + final List createdNodes, + final DefaultContentCreator contentCreator) throws RepositoryException { // init content creator - this.contentCreator.init(configuration, this.defaultImportProviders, createdNodes, null); + contentCreator.init(configuration, this.defaultImportProviders, createdNodes, null); final Map processedEntries = new HashMap(); @@ -368,17 +367,17 @@ public class Loader extends BaseImportLo return; } // we have a single file content, let's check if this has an import provider extension - for (String ext : this.contentCreator.getImportProviders().keySet()) { + for (String ext : contentCreator.getImportProviders().keySet()) { if ( path.endsWith(ext) ) { } } - handleFile(path, bundle, processedEntries, configuration, parent, createdNodes); + handleFile(path, bundle, processedEntries, configuration, parent, createdNodes, contentCreator); return; } // potential parent node import/extension - URL parentNodeDescriptor = importParentNode(parent.getSession(), bundle, path, parent); + URL parentNodeDescriptor = importParentNode(parent.getSession(), bundle, path, parent, contentCreator); if (parentNodeDescriptor != null) { processedEntries.put(parentNodeDescriptor, parent); } @@ -392,7 +391,7 @@ public class Loader extends BaseImportLo final String base = entry.substring(0, entry.length() - 1); URL nodeDescriptor = null; - for (String ext : this.contentCreator.getImportProviders().keySet()) { + for (String ext : contentCreator.getImportProviders().keySet()) { nodeDescriptor = bundle.getEntry(base + ext); if (nodeDescriptor != null) { break; @@ -408,7 +407,7 @@ public class Loader extends BaseImportLo node = processedEntries.get(nodeDescriptor); if (node == null) { node = createNode(parent, name, nodeDescriptor, - configuration); + configuration, contentCreator); processedEntries.put(nodeDescriptor, node); } } else { @@ -417,12 +416,12 @@ public class Loader extends BaseImportLo // walk down the line if (node != null) { - installFromPath(bundle, entry, configuration, node, createdNodes); + installFromPath(bundle, entry, configuration, node, createdNodes, contentCreator); } } else { // file => create file - handleFile(entry, bundle, processedEntries, configuration, parent, createdNodes); + handleFile(entry, bundle, processedEntries, configuration, parent, createdNodes, contentCreator); } } } @@ -443,7 +442,8 @@ public class Loader extends BaseImportLo final Map processedEntries, final PathEntry configuration, final Node parent, - final List createdNodes) + final List createdNodes, + final DefaultContentCreator contentCreator) throws RepositoryException { final URL file = bundle.getEntry(entry); final String name = getName(entry); @@ -455,7 +455,7 @@ public class Loader extends BaseImportLo // check for node descriptor URL nodeDescriptor = null; - for (String ext : this.contentCreator.getImportProviders().keySet()) { + for (String ext : contentCreator.getImportProviders().keySet()) { nodeDescriptor = bundle.getEntry(entry + ext); if (nodeDescriptor != null) { break; @@ -463,11 +463,11 @@ public class Loader extends BaseImportLo } // install if it is a descriptor - boolean foundProvider = this.contentCreator.getImportProvider(entry) != null; + boolean foundProvider = contentCreator.getImportProvider(entry) != null; Node node = null; if (foundProvider) { - if ((node = createNode(parent, name, file, configuration)) != null) { + if ((node = createNode(parent, name, file, configuration, contentCreator)) != null) { log.debug("Created Node as {} {} ",node.getPath(),name); processedEntries.put(file, node); } else { @@ -480,7 +480,7 @@ public class Loader extends BaseImportLo // otherwise just place as file if ( node == null ) { try { - createFile(configuration, parent, file, createdNodes); + createFile(configuration, parent, file, createdNodes, contentCreator); node = parent.getNode(name); } catch (IOException ioe) { log.warn("Cannot create file node for {}", file, ioe); @@ -490,12 +490,12 @@ public class Loader extends BaseImportLo // process it if (nodeDescriptor != null && processedEntries.get(nodeDescriptor) == null ) { try { - this.contentCreator.setIgnoreOverwriteFlag(true); + contentCreator.setIgnoreOverwriteFlag(true); node = createNode(parent, name, nodeDescriptor, - configuration); + configuration, contentCreator); processedEntries.put(nodeDescriptor, node); } finally { - this.contentCreator.setIgnoreOverwriteFlag(false); + contentCreator.setIgnoreOverwriteFlag(false); } } } catch ( RepositoryException e ) { @@ -518,7 +518,8 @@ public class Loader extends BaseImportLo private Node createNode(Node parent, String name, URL resourceUrl, - PathEntry configuration) + PathEntry configuration, + final DefaultContentCreator contentCreator) throws RepositoryException { final String resourcePath = resourceUrl.getPath().toLowerCase(); try { @@ -528,7 +529,7 @@ public class Loader extends BaseImportLo } // get the node reader for this resource - final ImportProvider ip = this.contentCreator.getImportProvider(resourcePath); + final ImportProvider ip = contentCreator.getImportProvider(resourcePath); if ( ip == null ) { return null; } @@ -539,10 +540,10 @@ public class Loader extends BaseImportLo return null; } - this.contentCreator.prepareParsing(parent, toPlainName(name)); - nodeReader.parse(resourceUrl, this.contentCreator); + contentCreator.prepareParsing(parent, toPlainName(name, contentCreator)); + nodeReader.parse(resourceUrl, contentCreator); - return this.contentCreator.getCreatedRootNode(); + return contentCreator.getCreatedRootNode(); } catch (RepositoryException re) { throw re; } catch (Throwable t) { @@ -580,7 +581,7 @@ public class Loader extends BaseImportLo * @throws IOException * @throws RepositoryException */ - private void createFile(PathEntry configuration, Node parent, URL source, List createdNodes) + private void createFile(PathEntry configuration, Node parent, URL source, List createdNodes, final DefaultContentCreator contentCreator) throws IOException, RepositoryException { final String srcPath = source.getPath(); int pos = srcPath.lastIndexOf("/"); @@ -592,15 +593,15 @@ public class Loader extends BaseImportLo path = srcPath.substring(0, pos + 1) + name; } - this.contentCreator.init(configuration, defaultImportProviders, createdNodes, null); - this.contentCreator.prepareParsing(parent, name); + contentCreator.init(configuration, defaultImportProviders, createdNodes, null); + contentCreator.prepareParsing(parent, name); final URLConnection conn = source.openConnection(); final long lastModified = Math.min(conn.getLastModified(), configuration.getLastModified()); final String type = conn.getContentType(); final InputStream data = conn.getInputStream(); - this.contentCreator.createFileAndResourceNode(path, data, type, lastModified); - this.contentCreator.finishNode(); - this.contentCreator.finishNode(); + contentCreator.createFileAndResourceNode(path, data, type, lastModified); + contentCreator.finishNode(); + contentCreator.finishNode(); } /** @@ -808,10 +809,11 @@ public class Loader extends BaseImportLo * Return the root node descriptor. */ private Descriptor getRootNodeDescriptor(final Bundle bundle, - final String path) { + final String path, + final DefaultContentCreator contentCreator) { URL rootNodeDescriptor = null; - for (Map.Entry e : this.contentCreator.getImportProviders().entrySet()) { + for (Map.Entry e : contentCreator.getImportProviders().entrySet()) { if (e.getValue() != null) { rootNodeDescriptor = bundle.getEntry(path + ROOT_DESCRIPTOR + e.getKey()); @@ -836,17 +838,17 @@ public class Loader extends BaseImportLo * Imports mixin nodes and properties (and optionally child nodes) of the * parent node. */ - private URL importParentNode(Session session, Bundle bundle, String path, Node parent) + private URL importParentNode(Session session, Bundle bundle, String path, Node parent, final DefaultContentCreator contentCreator) throws RepositoryException { - final Descriptor descriptor = getRootNodeDescriptor(bundle, path); + final Descriptor descriptor = getRootNodeDescriptor(bundle, path, contentCreator); // no root descriptor found if (descriptor == null) { return null; } try { - this.contentCreator.prepareParsing(parent, null); - descriptor.nodeReader.parse(descriptor.rootNodeDescriptor, this.contentCreator); + contentCreator.prepareParsing(parent, null); + descriptor.nodeReader.parse(descriptor.rootNodeDescriptor, contentCreator); return descriptor.rootNodeDescriptor; } catch (RepositoryException re) { @@ -857,8 +859,8 @@ public class Loader extends BaseImportLo } - private String toPlainName(String name) { - final String providerExt = this.contentCreator.getImportProviderExtension(name); + private String toPlainName(final String name, final DefaultContentCreator contentCreator) { + final String providerExt = contentCreator.getImportProviderExtension(name); if (providerExt != null) { return name.substring(0, name.length() - providerExt.length()); }