Commit 0d3b3a38 authored by Gugli's avatar Gugli Committed by Guus der Kinderen

OF-1206: PrivacyLists on first run (#656)

* PrivacyLists on first run
There are 2 instances of PrivacyListProvider (one in PrivacyListManager and the other in IQPrivacyHandler)

The issue was : both instances have separate privacyListCount variables, which are filled during PrivacyListProvider constructor (== server startup).
Thus, if at server startup the privacyListCount is 0, and one of the instances adds a PrivacyList, the other will never be informed and discard all changes and reads.

Fix : turn PrivacyListProvider into a singleton.

* PrivacyLists on first run
There are 2 instances of PrivacyListProvider (one in PrivacyListManager and the other in IQPrivacyHandler)

The issue was : both instances have separate privacyListCount variables, which are filled during PrivacyListProvider constructor (== server startup).
Thus, if at server startup the privacyListCount is 0, and one of the instances adds a PrivacyList, the other will never be informed and discard all changes and reads.

Fix : turn PrivacyListProvider into a singleton.
parent 2638e753
...@@ -51,7 +51,7 @@ public class IQPrivacyHandler extends IQHandler ...@@ -51,7 +51,7 @@ public class IQPrivacyHandler extends IQHandler
private IQHandlerInfo info; private IQHandlerInfo info;
private PrivacyListManager manager = PrivacyListManager.getInstance(); private PrivacyListManager manager = PrivacyListManager.getInstance();
private PrivacyListProvider provider = new PrivacyListProvider(); private PrivacyListProvider provider = PrivacyListProvider.getInstance();
public IQPrivacyHandler() { public IQPrivacyHandler() {
super("Blocking Communication Handler"); super("Blocking Communication Handler");
......
...@@ -34,7 +34,7 @@ public class PrivacyListManager { ...@@ -34,7 +34,7 @@ public class PrivacyListManager {
private static final PrivacyListManager instance = new PrivacyListManager(); private static final PrivacyListManager instance = new PrivacyListManager();
private static Cache<String, PrivacyList> listsCache; private static Cache<String, PrivacyList> listsCache;
private PrivacyListProvider provider = new PrivacyListProvider(); private PrivacyListProvider provider = PrivacyListProvider.getInstance();
private List<PrivacyListEventListener> listeners = new CopyOnWriteArrayList<>(); private List<PrivacyListEventListener> listeners = new CopyOnWriteArrayList<>();
......
...@@ -29,7 +29,7 @@ import java.util.HashMap; ...@@ -29,7 +29,7 @@ import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.concurrent.BlockingQueue; import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicBoolean;
import org.dom4j.Element; import org.dom4j.Element;
import org.dom4j.io.SAXReader; import org.dom4j.io.SAXReader;
...@@ -45,6 +45,8 @@ import org.slf4j.LoggerFactory; ...@@ -45,6 +45,8 @@ import org.slf4j.LoggerFactory;
*/ */
public class PrivacyListProvider { public class PrivacyListProvider {
private static final PrivacyListProvider instance = new PrivacyListProvider();
private static final Logger Log = LoggerFactory.getLogger(PrivacyListProvider.class); private static final Logger Log = LoggerFactory.getLogger(PrivacyListProvider.class);
private static final String PRIVACY_LIST_COUNT = private static final String PRIVACY_LIST_COUNT =
...@@ -71,11 +73,20 @@ public class PrivacyListProvider { ...@@ -71,11 +73,20 @@ public class PrivacyListProvider {
private BlockingQueue<SAXReader> xmlReaders = new LinkedBlockingQueue<>(POOL_SIZE); private BlockingQueue<SAXReader> xmlReaders = new LinkedBlockingQueue<>(POOL_SIZE);
/** /**
* Stores the total number of privacy lists. * Boolean used to optimize getters when the database is empty
*/ */
private AtomicInteger privacyListCount; private AtomicBoolean databaseContainsPrivacyLists;
public PrivacyListProvider() { /**
* Returns the unique instance of this class.
*
* @return the unique instance of this class.
*/
public static PrivacyListProvider getInstance() {
return instance;
}
private PrivacyListProvider() {
super(); super();
// Initialize the pool of sax readers // Initialize the pool of sax readers
for (int i=0; i<POOL_SIZE; i++) { for (int i=0; i<POOL_SIZE; i++) {
...@@ -84,13 +95,10 @@ public class PrivacyListProvider { ...@@ -84,13 +95,10 @@ public class PrivacyListProvider {
xmlReaders.add(xmlReader); xmlReaders.add(xmlReader);
} }
// Load the total number of privacy lists in the database. We're looking // Checks if the PrivacyLists database is empty.
// for the (very common) special case that there are no privacy lists stored. // In that case, we can optimize away many database calls.
// In that case, we can optimize away many database calls. In the future, a databaseContainsPrivacyLists = new AtomicBoolean(false);
// better general-case solution may be to cache all privacy lists defined loadDatabaseContainsPrivacyLists();
// if there are less than, say, 500.
privacyListCount = new AtomicInteger(0);
loadPrivacyListCount();
} }
/** /**
...@@ -102,7 +110,7 @@ public class PrivacyListProvider { ...@@ -102,7 +110,7 @@ public class PrivacyListProvider {
*/ */
public Map<String, Boolean> getPrivacyLists(String username) { public Map<String, Boolean> getPrivacyLists(String username) {
// If there are no privacy lists stored, this method is a no-op. // If there are no privacy lists stored, this method is a no-op.
if (privacyListCount.get() == 0) { if (!databaseContainsPrivacyLists.get()) {
return Collections.emptyMap(); return Collections.emptyMap();
} }
...@@ -139,7 +147,7 @@ public class PrivacyListProvider { ...@@ -139,7 +147,7 @@ public class PrivacyListProvider {
*/ */
public PrivacyList loadPrivacyList(String username, String listName) { public PrivacyList loadPrivacyList(String username, String listName) {
// If there are no privacy lists stored, this method is a no-op. // If there are no privacy lists stored, this method is a no-op.
if (privacyListCount.get() == 0) { if (!databaseContainsPrivacyLists.get()) {
return null; return null;
} }
...@@ -203,7 +211,7 @@ public class PrivacyListProvider { ...@@ -203,7 +211,7 @@ public class PrivacyListProvider {
*/ */
public PrivacyList loadDefaultPrivacyList(String username) { public PrivacyList loadDefaultPrivacyList(String username) {
// If there are no privacy lists stored, this method is a no-op. // If there are no privacy lists stored, this method is a no-op.
if (privacyListCount.get() == 0) { if (!databaseContainsPrivacyLists.get()) {
return null; return null;
} }
...@@ -280,9 +288,7 @@ public class PrivacyListProvider { ...@@ -280,9 +288,7 @@ public class PrivacyListProvider {
finally { finally {
DbConnectionManager.closeConnection(pstmt, con); DbConnectionManager.closeConnection(pstmt, con);
} }
// Set the privacy list count to -1. We don't know how many privacy lists there databaseContainsPrivacyLists.set(true);
// are, but it's not "0", which is the case we care about.
privacyListCount.set(-1);
} }
/** /**
...@@ -310,6 +316,7 @@ public class PrivacyListProvider { ...@@ -310,6 +316,7 @@ public class PrivacyListProvider {
finally { finally {
DbConnectionManager.closeConnection(pstmt, con); DbConnectionManager.closeConnection(pstmt, con);
} }
databaseContainsPrivacyLists.set(true);
} }
/** /**
...@@ -320,7 +327,7 @@ public class PrivacyListProvider { ...@@ -320,7 +327,7 @@ public class PrivacyListProvider {
*/ */
public void deletePrivacyList(String username, String listName) { public void deletePrivacyList(String username, String listName) {
// If there are no privacy lists stored, this method is a no-op. // If there are no privacy lists stored, this method is a no-op.
if (privacyListCount.get() == 0) { if (!databaseContainsPrivacyLists.get()) {
return; return;
} }
Connection con = null; Connection con = null;
...@@ -338,9 +345,7 @@ public class PrivacyListProvider { ...@@ -338,9 +345,7 @@ public class PrivacyListProvider {
finally { finally {
DbConnectionManager.closeConnection(pstmt, con); DbConnectionManager.closeConnection(pstmt, con);
} }
// Set the privacy list count to -1. We don't know how many privacy lists there databaseContainsPrivacyLists.set(true);
// are, but it's probably not "0", which is the case we care about.
privacyListCount.set(-1);
} }
/** /**
...@@ -350,7 +355,7 @@ public class PrivacyListProvider { ...@@ -350,7 +355,7 @@ public class PrivacyListProvider {
*/ */
public void deletePrivacyLists(String username) { public void deletePrivacyLists(String username) {
// If there are no privacy lists stored, this method is a no-op. // If there are no privacy lists stored, this method is a no-op.
if (privacyListCount.get() == 0) { if (!databaseContainsPrivacyLists.get()) {
return; return;
} }
Connection con = null; Connection con = null;
...@@ -367,15 +372,13 @@ public class PrivacyListProvider { ...@@ -367,15 +372,13 @@ public class PrivacyListProvider {
finally { finally {
DbConnectionManager.closeConnection(pstmt, con); DbConnectionManager.closeConnection(pstmt, con);
} }
// Set the privacy list count to -1. We don't know how many privacy lists there databaseContainsPrivacyLists.set(true);
// are, but it's probably not "0", which is the case we care about.
privacyListCount.set(-1);
} }
/** /**
* Loads the total number of privacy lists stored in the database. * Loads the total number of privacy lists stored in the database to know if we must use them.
*/ */
private void loadPrivacyListCount() { private void loadDatabaseContainsPrivacyLists() {
Connection con = null; Connection con = null;
PreparedStatement pstmt = null; PreparedStatement pstmt = null;
ResultSet rs = null; ResultSet rs = null;
...@@ -384,7 +387,7 @@ public class PrivacyListProvider { ...@@ -384,7 +387,7 @@ public class PrivacyListProvider {
pstmt = con.prepareStatement(PRIVACY_LIST_COUNT); pstmt = con.prepareStatement(PRIVACY_LIST_COUNT);
rs = pstmt.executeQuery(); rs = pstmt.executeQuery();
rs.next(); rs.next();
privacyListCount.set(rs.getInt(1)); databaseContainsPrivacyLists.set(rs.getInt(1) != 0);
} }
catch (Exception e) { catch (Exception e) {
Log.error(e.getMessage(), e); Log.error(e.getMessage(), e);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment