svn commit: r1355714 - in /incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl: SolrFieldMapper.java SolrYard.java

[rw...@apache.org]

Author: rwesten
Date: Sat Jun 30 11:56:50 2012
New Revision: 1355714

URL: http://svn.apache.org/viewvc?rev=1355714&view=rev
Log:
Fixes for STANBOL-669 and STANBOL-668. See issues for details

Modified:
    incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrFieldMapper.java
    incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrYard.java

Modified: incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrFieldMapper.java
URL: http://svn.apache.org/viewvc/incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrFieldMapper.java?rev=1355714&r1=1355713&r2=1355714&view=diff
==============================================================================
--- incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrFieldMapper.java (original)
+++ incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrFieldMapper.java Sat Jun 30 11:56:50 2012
@@ -125,12 +125,16 @@ public class SolrFieldMapper implements 
      * 
      * @see LinkedHashMap#
      */
-    private final LRU<IndexField,Collection<String>> indexFieldMappings = new LRU<IndexField,Collection<String>>();
+    private final Map<IndexField,Collection<String>> indexFieldMappings = 
+            //STANBOL-669: LRU chaches MUST BE synchronized!
+            Collections.synchronizedMap(new LRU<IndexField,Collection<String>>());
     /**
      * The assumption is, that only a handful of fields appear in index documents. So it makes sense to keep
      * some mappings within a cache rather than calculating them again and again.
      */
-    private final LRU<String,IndexField> fieldMappings = new LRU<String,IndexField>();
+    private final Map<String,IndexField> fieldMappings = 
+            //STANBOL-669: LRU chaches MUST BE synchronized!
+            Collections.synchronizedMap(new LRU<String,IndexField>());
 
     public SolrFieldMapper(SolrServer server) {
         if (server == null) {
@@ -599,7 +603,9 @@ public class SolrFieldMapper implements 
      */
     private Map<String,String> getNamespaceMap() {
         if (__namespaceMap == null) {
-            loadNamespaceConfig();
+            synchronized (prefixNamespaceMappingsLock) {
+                loadNamespaceConfig();
+            }
         }
         return __namespaceMap;
     }
@@ -608,6 +614,11 @@ public class SolrFieldMapper implements 
      * Do never access this Map directly! Use {@link #getPrefixMap()}!
      */
     private Map<String,String> __prefixMap = null;
+    /**
+     * used as lock during loading of the namespace <-> prefix mappings
+     * (fixes STANBOL-668)
+     */
+    private Object prefixNamespaceMappingsLock = new Object();
 
     /**
      * Getter for the prefix to namespace mappings
@@ -616,7 +627,9 @@ public class SolrFieldMapper implements 
      */
     private Map<String,String> getPrefixMap() {
         if (__prefixMap == null) {
-            loadNamespaceConfig();
+            synchronized (prefixNamespaceMappingsLock) {
+                loadNamespaceConfig();
+            }
         }
         return __prefixMap;
     }
@@ -729,8 +742,10 @@ public class SolrFieldMapper implements 
     }
 
     private void addNamespaceMapping(String prefix, String namespace) {
-        getPrefixMap().put(prefix, namespace);
-        getNamespaceMap().put(namespace, prefix);
+        synchronized (prefixNamespaceMappingsLock) { 
+            getPrefixMap().put(prefix, namespace);
+            getNamespaceMap().put(namespace, prefix);
+        }
     }
 
     /**
@@ -738,8 +753,8 @@ public class SolrFieldMapper implements 
      * the prefix &lt;-&gt; namespace mappings
      */
     private void loadNamespaceConfig() {
-        __prefixMap = new HashMap<String,String>();
-        __namespaceMap = new HashMap<String,String>();
+        HashMap<String,String> prefixMap = new HashMap<String,String>();
+        HashMap<String,String> namespaceMap = new HashMap<String,String>();
         SolrDocument config = null;
         try {
             config = getSolrDocument(FieldMapper.URI);
@@ -758,14 +773,14 @@ public class SolrFieldMapper implements 
                         String prefix = configFieldElements[1];
                         Object value = config.getFirstValue(fieldName);
                         if (value != null) {
-                            if (__namespaceMap.containsKey(value.toString())) {
-                                log.error("found two prefixes (" + __namespaceMap.get(value.toString())
+                            if (namespaceMap.containsKey(value.toString())) {
+                                log.error("found two prefixes (" + namespaceMap.get(value.toString())
                                           + " and " + prefix + ") for Namespace " + value.toString()
                                           + " keep the first one");
                             } else {
                                 log.debug(" > prefix: " + prefix + " value: " + value);
-                                __prefixMap.put(prefix, value.toString());
-                                __namespaceMap.put(value.toString(), prefix);
+                                prefixMap.put(prefix, value.toString());
+                                namespaceMap.put(value.toString(), prefix);
                                 // check for default NS
                                 if (prefix.startsWith(DEFAULT_NS_PREFIX_STRING)) {
                                     String prefixNumber = prefix.substring(DEFAULT_NS_PREFIX_STRING.length());
@@ -792,6 +807,11 @@ public class SolrFieldMapper implements 
                 }
             }
         }
+        //only store complete mappings to the member variables (STANBOL-668)
+        synchronized (prefixNamespaceMappingsLock) {
+            __prefixMap = prefixMap;
+            __namespaceMap = namespaceMap;
+        }
     }
 
     private String getConfigFieldName(String configName) {

Modified: incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrYard.java
URL: http://svn.apache.org/viewvc/incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrYard.java?rev=1355714&r1=1355713&r2=1355714&view=diff
==============================================================================
--- incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrYard.java (original)
+++ incubator/stanbol/trunk/entityhub/yard/solr/src/main/java/org/apache/stanbol/entityhub/yard/solr/impl/SolrYard.java Sat Jun 30 11:56:50 2012
@@ -28,6 +28,9 @@ import java.util.Map;
 import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.Map.Entry;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
@@ -49,6 +52,7 @@ import org.apache.solr.client.solrj.requ
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.core.SolrCore;
 import org.apache.stanbol.commons.solr.IndexReference;
 import org.apache.stanbol.commons.solr.RegisteredSolrServerTracker;
 import org.apache.stanbol.commons.solr.managed.IndexMetadata;
@@ -76,10 +80,8 @@ import org.apache.stanbol.entityhub.yard
 import org.apache.stanbol.entityhub.yard.solr.model.IndexValueFactory;
 import org.apache.stanbol.entityhub.yard.solr.query.IndexConstraintTypeEnum;
 import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceReference;
 import org.osgi.service.cm.ConfigurationException;
 import org.osgi.service.component.ComponentContext;
-import org.osgi.util.tracker.ServiceTrackerCustomizer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xml.sax.SAXException;
@@ -239,6 +241,17 @@ public class SolrYard extends AbstractYa
      * {@link #activate(ComponentContext)}.
      */
     private SolrServer _server;
+    
+    /**
+     * In case the {@link SolrServer} changes during normal operation the
+     * {@link SolrFieldMapper} needs to be reinitialised for the new Core.
+     * This lock is used to avoid NPE when setting the {@link #_fieldMapper}
+     * variable back to <code>null</code> in those cases. While this will
+     * trigger automatic re-initialisation on the next call to 
+     * {@link #getFieldMapper()} there would be the possibility that calls to
+     * {@link #getFieldMapper()} return <code>null</code>.
+     */
+    private final ReadWriteLock fieldMapperLock = new ReentrantReadWriteLock();
     /**
      * The {@link FieldMapper} is responsible for converting fields of {@link Representation} to fields in the
      * {@link SolrInputDocument} and vice versa
@@ -502,10 +515,16 @@ public class SolrYard extends AbstractYa
                     } catch (InterruptedException e) {}
                 }
             }
-            if(server == null || !server.equals(this._server)){
+            if(server != null && !server.equals(this._server)){
                 //reset the fieldMapper so that it is reinitialised for the new one
                 //STANBOL-519
-                _fieldMapper = null; 
+                Lock writeLock = fieldMapperLock.writeLock();
+                writeLock.lock();
+                try {
+                    _fieldMapper = null;
+                }finally{
+                    writeLock.unlock();
+                }
             }
         } else {
             //for remove servers and when running outside OSGI
@@ -515,8 +534,16 @@ public class SolrYard extends AbstractYa
         if(server != null){
             return server;
         } else {
+            Lock writeLock = fieldMapperLock.writeLock();
+            writeLock.lock();
+            try {
+                _fieldMapper = null;
+            }finally{
+                writeLock.unlock();
+            }
             throw new YardException(String.format("The SolrIndex '%s' for SolrYard '%s' is currently not active!",
                 ((SolrYardConfig)getConfig()).getSolrServerLocation(),getName()));
+            
         }
     }
 
@@ -687,11 +714,24 @@ public class SolrYard extends AbstractYa
         }
     }
     private SolrFieldMapper getFieldMapper() throws YardException {
-        if(_fieldMapper == null){
+        Lock readLock = fieldMapperLock.readLock();
+        readLock.lock();
+        try {
+            if(_fieldMapper != null){
+                return _fieldMapper;
+            }
+        } finally {
+            readLock.unlock();
+        }
+        Lock writeLock = fieldMapperLock.writeLock();
+        writeLock.lock();
+        try {
             // the fieldMapper need the Server to store it's namespace prefix configuration
             _fieldMapper = new SolrFieldMapper(getServer());
+            return _fieldMapper;
+        } finally {
+            writeLock.unlock();
         }
-        return _fieldMapper;
     }
     
     /**
@@ -889,6 +929,12 @@ public class SolrYard extends AbstractYa
      * @return the Representation
      */
     protected final Representation createRepresentation(FieldMapper fieldMapper, SolrDocument doc, Set<String> fields) {
+        if(fieldMapper == null){
+            throw new IllegalArgumentException("The parsed FieldMapper MUST NOT be NULL!");
+        }
+        if(doc == null){
+            throw new IllegalArgumentException("The parsed SolrDocument MUST NOT be NULL!");
+        }
         Object id = doc.getFirstValue(fieldMapper.getDocumentIdField());
         if (id == null) {
             throw new IllegalStateException(String.format(