Permalink
Browse files

XX-10022: Optimize phonebook retrieval using plain SQL queries instea…

…d of Hibernate

-retrieve entries using jdbcTemplate (Plain SQL queries + RowCallbackhandler)
Based on Patch from George Niculae - Thanks!
-close stream in XStreamRepresentation after writing content
  • Loading branch information...
1 parent c75485a commit a3967eacf73fab0fbf95a806426fb642305bfe2b @mirceac mirceac committed Jan 23, 2012
@@ -16,7 +16,6 @@
import static org.apache.commons.collections.CollectionUtils.select;
import static org.apache.commons.lang.StringUtils.join;
import static org.sipfoundry.sipxconfig.common.DaoUtils.checkDuplicates;
-import static org.sipfoundry.sipxconfig.common.DaoUtils.forAllUsersDo;
import static org.sipfoundry.sipxconfig.common.DaoUtils.requireOneOrZero;
import static org.springframework.dao.support.DataAccessUtils.singleResult;
@@ -32,6 +31,7 @@
import java.io.Reader;
import java.io.Writer;
import java.sql.Connection;
+import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
@@ -71,7 +71,6 @@
import org.sipfoundry.sipxconfig.bulk.BulkParser;
import org.sipfoundry.sipxconfig.bulk.csv.CsvWriter;
import org.sipfoundry.sipxconfig.bulk.vcard.VCardParserException;
-import org.sipfoundry.sipxconfig.common.Closure;
import org.sipfoundry.sipxconfig.common.CoreContext;
import org.sipfoundry.sipxconfig.common.DataCollectionUtil;
import org.sipfoundry.sipxconfig.common.SipxHibernateDaoSupport;
@@ -81,9 +80,13 @@
import org.sipfoundry.sipxconfig.setting.BeanWithSettingsDao;
import org.sipfoundry.sipxconfig.setting.Group;
import org.springframework.beans.factory.annotation.Required;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.core.RowCallbackHandler;
+
import com.glaforge.i18n.io.CharsetToolkit;
+
public class PhonebookManagerImpl extends SipxHibernateDaoSupport<Phonebook> implements PhonebookManager,
DaoEventListener {
private static final Log LOG = LogFactory.getLog(PhonebookManagerImpl.class);
@@ -94,9 +97,40 @@
private static final String PARAM_USER_ID = "userId";
private static final String AT_SIGN = "@";
+ private static final String QUERY = "SELECT u.user_id, u.user_name, u.first_name, u.last_name, "
+ + "abe.email_address, abe.alternate_email_address, "
+ + "abe.im_id, abe.im_display_name, abe.alternate_im_id, abe.job_title, abe.job_dept, "
+ + "abe.company_name, abe.assistant_name, abe.assistant_phone_number, "
+ + "abe.fax_number, abe.location, abe.home_phone_number, abe.cell_phone_number, "
+ + "ha.street as home_street, ha.city as home_city, ha.country as home_country, "
+ + "ha.state as home_state, ha.zip as home_zip, ha.office_designation as home_office_designation, "
+ + "ja.street as job_street, ja.zip as job_zip, "
+ + "ja.city as job_city, ja.country as job_country, ja.state as job_state, "
+ + "ja.office_designation as job_office_designation from Users u "
+ + "left join address_book_entry abe on abe.address_book_entry_id = u.address_book_entry_id "
+ + "left join address ha on ha.address_id = abe.home_address_id "
+ + "left join address ja on ja.address_id = abe.office_address_id "
+ + "WHERE u.user_type='C' ORDER BY u.user_id;";
+ private static final String QUERY_GROUP = "SELECT u.user_id, u.user_name, u.first_name, "
+ + "u.last_name, abe.email_address, abe.alternate_email_address, "
+ + "abe.im_id, abe.im_display_name, abe.alternate_im_id, abe.job_title, "
+ + "abe.job_dept, abe.company_name, abe.assistant_name, abe.assistant_phone_number, "
+ + "abe.fax_number, abe.location, abe.home_phone_number, "
+ + "abe.cell_phone_number, ha.street as home_street, ha.city as home_city, ha.country as home_country, "
+ + "ha.state as home_state, ha.zip as home_zip, "
+ + "ha.office_designation as home_office_designation, ja.street as job_street, ja.zip as job_zip, "
+ + "ja.city as job_city, ja.country as job_country, "
+ + "ja.state as job_state, ja.office_designation as job_office_designation from Users u "
+ + "left join address_book_entry abe "
+ + "on abe.address_book_entry_id = u.address_book_entry_id "
+ + "left join address ha on ha.address_id = abe.home_address_id left join address ja on "
+ + "ja.address_id = abe.office_address_id inner join user_group ug on u.user_id = ug.user_id "
+ + "WHERE u.user_type='C' AND ug.group_id=%d ORDER BY u.user_id;";
+
private boolean m_phonebookManagementEnabled;
private String m_externalUsersDirectory;
private CoreContext m_coreContext;
+ private JdbcTemplate m_jdbcTemplate;
private BulkParser m_csvParser;
private BulkParser m_vcardParser;
@@ -305,15 +339,54 @@ public Phonebook getPrivatePhonebookCreateIfRequired(User user) {
}
private void addEveryoneEntries(final User user, final Map<String, PhonebookEntry> entries) {
- Closure<User> closure = new Closure<User>() {
+ m_jdbcTemplate.query(QUERY, new RowCallbackHandler() {
@Override
- public void execute(User closureUser) {
- if (!closureUser.equals(user)) {
- addUserEntries(closureUser, entries);
+ public void processRow(ResultSet rs) throws SQLException {
+ int userId = rs.getInt("user_id");
+ if (userId != user.getId()) {
+ PhonebookEntry entry = extractPhonebookEntry(rs);
+ entries.put(getEntryKey(entry), entry);
}
}
- };
- forAllUsersDo(m_coreContext, closure);
+ });
+ }
+
+ private UserPhonebookEntry extractPhonebookEntry(ResultSet rs) throws SQLException {
+ String firstName = rs.getString("first_name");
+ String lastName = rs.getString("last_name");
+ String userName = rs.getString("user_name");
+ AddressBookEntry abe = new AddressBookEntry();
+ abe.setEmailAddress(rs.getString("email_address"));
+ abe.setAlternateEmailAddress(rs.getString("alternate_email_address"));
+ abe.setImId(rs.getString("im_id"));
+ abe.setImDisplayName(rs.getString("im_display_name"));
+ abe.setAlternateImId(rs.getString("alternate_im_id"));
+ abe.setJobTitle(rs.getString("job_title"));
+ abe.setJobDept(rs.getString("job_dept"));
+ abe.setCompanyName(rs.getString("company_name"));
+ abe.setAssistantName(rs.getString("assistant_name"));
+ abe.setAssistantPhoneNumber(rs.getString("assistant_phone_number"));
+ abe.setFaxNumber(rs.getString("fax_number"));
+ abe.setLocation(rs.getString("location"));
+ abe.setHomePhoneNumber(rs.getString("home_phone_number"));
+ abe.setCellPhoneNumber(rs.getString("cell_phone_number"));
+ Address homeAddress = new Address();
+ homeAddress.setStreet(rs.getString("home_street"));
+ homeAddress.setCity(rs.getString("home_city"));
+ homeAddress.setCountry(rs.getString("home_country"));
+ homeAddress.setState(rs.getString("home_state"));
+ homeAddress.setZip(rs.getString("home_zip"));
+ homeAddress.setOfficeDesignation(rs.getString("home_office_designation"));
+ abe.setHomeAddress(homeAddress);
+ Address jobAddress = new Address();
+ jobAddress.setStreet(rs.getString("job_street"));
+ jobAddress.setCity(rs.getString("job_city"));
+ jobAddress.setCountry(rs.getString("job_country"));
+ jobAddress.setState(rs.getString("job_state"));
+ jobAddress.setZip(rs.getString("job_zip"));
+ jobAddress.setOfficeDesignation(rs.getString("job_office_designation"));
+ abe.setOfficeAddress(jobAddress);
+ return new UserPhonebookEntry(firstName, lastName, userName, abe);
}
public PagedPhonebook getPagedPhonebook(Collection<Phonebook> phonebook, User user, String startRow,
@@ -347,12 +420,18 @@ public PagedPhonebook getPagedPhonebook(Collection<Phonebook> phonebook, User us
}
public Collection<PhonebookEntry> getEntries(Phonebook phonebook) {
- Map<String, PhonebookEntry> entries = new TreeMap<String, PhonebookEntry>();
+ final Map<String, PhonebookEntry> entries = new TreeMap<String, PhonebookEntry>();
Collection<Group> members = phonebook.getMembers();
if (members != null) {
for (Group group : members) {
- addUserEntries(m_coreContext.getGroupMembers(group), entries);
+ m_jdbcTemplate.query(String.format(QUERY_GROUP, group.getId()), new RowCallbackHandler() {
+ @Override
+ public void processRow(ResultSet rs) throws SQLException {
+ PhonebookEntry entry = extractPhonebookEntry(rs);
+ entries.put(getEntryKey(entry), entry);
+ }
+ });
}
}
@@ -384,30 +463,18 @@ public PagedPhonebook getPagedPhonebook(Collection<Phonebook> phonebook, User us
}
private void addEveryoneEntries(final Map<String, PhonebookEntry> entries) {
- Closure<User> closure = new Closure<User>() {
+ m_jdbcTemplate.query(QUERY, new RowCallbackHandler() {
@Override
- public void execute(User closureUser) {
+ public void processRow(ResultSet rs) throws SQLException {
Map<String, PhonebookEntry> entry = new TreeMap<String, PhonebookEntry>();
- UserPhonebookEntry userEntry = new UserPhonebookEntry(closureUser);
+ UserPhonebookEntry userEntry = extractPhonebookEntry(rs);
String entryKey = getEntryKey(userEntry);
entry.put(entryKey, userEntry);
if (!entries.containsKey(entryKey)) {
entries.put(entryKey, userEntry);
}
}
- };
- forAllUsersDo(m_coreContext, closure);
- }
-
- private void addUserEntries(Collection<User> users, Map<String, PhonebookEntry> entries) {
- for (User user : users) {
- addUserEntries(user, entries);
- }
- }
-
- private void addUserEntries(User user, Map<String, PhonebookEntry> entries) {
- PhonebookEntry entry = new UserPhonebookEntry(user);
- entries.put(getEntryKey(entry), entry);
+ });
}
private void addEntriesFromFile(Map<String, PhonebookEntry> entries, InputStream in, String encoding,
@@ -708,41 +775,36 @@ public InvalidPhonebookFormat() {
* public so Velocity doesn't reject object
*/
public static class UserPhonebookEntry extends PhonebookEntry {
- private final User m_user;
-
- UserPhonebookEntry(User user) {
- m_user = user;
+ private String m_firstName;
+ private String m_lastName;
+ private String m_number;
+ private AddressBookEntry m_abe;
+
+ UserPhonebookEntry(String first, String last, String number, AddressBookEntry abe) {
+ m_firstName = first;
+ m_lastName = last;
+ m_number = number;
+ m_abe = abe;
}
@Override
public String getFirstName() {
- return m_user.getFirstName();
+ return m_firstName;
}
@Override
public String getLastName() {
- return m_user.getLastName();
+ return m_lastName;
}
@Override
public String getNumber() {
- return StringUtils.defaultIfEmpty(m_user.getExtension(true), m_user.getUserName());
+ return m_number;
}
@Override
public AddressBookEntry getAddressBookEntry() {
- AddressBookEntry userAbe = m_user.getAddressBookEntry();
- if (userAbe != null) {
- AddressBookEntry representableAbe = new AddressBookEntry();
- representableAbe.update(userAbe);
- boolean imEnabled = (Boolean) m_user.getSettingTypedValue("im/im-account");
- if (!imEnabled) {
- representableAbe.setImId(StringUtils.EMPTY);
- }
- return representableAbe;
- }
- return null;
-
+ return m_abe;
}
}
@@ -1007,6 +1069,11 @@ public void removePrivatePhonebook(User user) {
}
}
+ public void setConfigJdbcTemplate(JdbcTemplate template) {
+ m_jdbcTemplate = template;
+ }
+
+ @Required
public void setSettingsDao(BeanWithSettingsDao<GeneralPhonebookSettings> settingsDao) {
m_settingsDao = settingsDao;
}
@@ -44,6 +44,7 @@
</property>
</bean>
</property>
+ <property name="configJdbcTemplate" ref="configJdbcTemplate" />
</bean>
<bean id="filePhonebookEntryUpgradeTask" class="org.sipfoundry.sipxconfig.phonebook.FilePhonebookEntryUpgradeTask">
@@ -57,5 +58,6 @@
<bean id="generalPhonebookSettings" class="org.sipfoundry.sipxconfig.phonebook.GeneralPhonebookSettings" parent="settingsBean">
</bean>
+
</beans>
@@ -49,11 +49,14 @@
import org.sipfoundry.sipxconfig.setting.BeanWithSettingsTestCase;
import org.sipfoundry.sipxconfig.setting.Group;
import org.sipfoundry.sipxconfig.test.PhonebookTestHelper;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.core.RowCallbackHandler;
import org.springframework.orm.hibernate3.HibernateTemplate;
public class PhonebookManagerTest extends BeanWithSettingsTestCase {
GeneralPhonebookSettings settings;
HibernateTemplate m_hibernateTemplate;
+ JdbcTemplate m_jdbcTemplate;
protected void setUp() throws Exception {
super.setUp();
@@ -66,6 +69,7 @@ protected void setUp() throws Exception {
m_hibernateTemplate.loadAll(GeneralPhonebookSettings.class);
expectLastCall().andReturn(settingsList);
replay(m_hibernateTemplate);
+ m_jdbcTemplate = createMock(JdbcTemplate.class);
}
public void testGetEmptyPhonebookRows() {
@@ -76,26 +80,18 @@ public void testGetEmptyPhonebookRows() {
public void testGetRows() {
Phonebook phonebook = new Phonebook();
- Group group = new Group();
- User user = new User();
- user.setFirstName("Tweety");
- user.setLastName("Bird");
- user.setUserName("tbird");
- phonebook.setMembers(singleton(group));
- Collection<User> users = singleton(user);
+
IMocksControl coreContextControl = EasyMock.createControl();
CoreContext coreContext = coreContextControl.createMock(CoreContext.class);
- coreContext.getGroupMembers(group);
- coreContextControl.andReturn(users);
coreContextControl.replay();
-
PhonebookManagerImpl context = new PhonebookManagerImpl();
context.setCoreContext(coreContext);
context.setHibernateTemplate(m_hibernateTemplate);
- Collection<PhonebookEntry> entries = context.getEntries(phonebook);
- assertEquals(1, entries.size());
- PhonebookEntry entry = entries.iterator().next();
- assertEquals("Tweety", entry.getFirstName());
+ context.setConfigJdbcTemplate(m_jdbcTemplate);
+
+ //This just performs DATABASE query - see PhonebookManagerTestIntegration. Here we can
+ //just test that m_jdbcTemplate is used
+ context.getEntries(phonebook);
coreContextControl.verify();
}
@@ -378,30 +374,6 @@ public void testSearch() {
out.search(singletonList(new Phonebook()), "nulluser", userPortal);
}
- public void testUserPhoneBookEntry() throws Exception {
- User user = new MockUser(Boolean.TRUE);
- AddressBookEntry abe = new AddressBookEntry();
- abe.setImId("test");
- user.setAddressBookEntry(abe);
- UserPhonebookEntry entry = new PhonebookManagerImpl.UserPhonebookEntry(user);
-
- assertEquals("test", entry.getAddressBookEntry().getImId());
-
- user.setUserName("500");
- assertEquals("500", entry.getNumber());
-
- user.setUserName("abcd");
- assertEquals("abcd", entry.getNumber());
-
- user.setAliasesString("501");
- assertEquals("501", entry.getNumber());
-
- user = new MockUser(Boolean.FALSE);
- user.setAddressBookEntry(abe);
- entry = new PhonebookManagerImpl.UserPhonebookEntry(user);
- assertEquals("", entry.getAddressBookEntry().getImId());
- }
-
private class MockUser extends User {
private Boolean m_imEnabled;
@@ -26,6 +26,7 @@
import java.io.IOException;
import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.restlet.data.MediaType;
@@ -172,5 +173,9 @@ public void write(OutputStream outputStream) throws IOException {
} else if (m_object != null) {
getXstream().toXML(m_object, outputStream);
}
+ if (outputStream != null) {
+ outputStream.flush();
+ IOUtils.closeQuietly(outputStream);
+ }
}
}

0 comments on commit a3967ea

Please sign in to comment.