Skip to content

Commit 9f06d46

Browse files
ashwardashward
committed
Fixes paging in UserItemsGet
1 parent 4c967f7 commit 9f06d46

10 files changed

Lines changed: 346 additions & 44 deletions

File tree

‎src/main/java/org/buddycloud/channelserver/db/jdbc/JDBCNodeStore.java‎

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
import org.xmpp.resultsetmanagement.ResultSetImpl;
3939

4040
public class JDBCNodeStore implements NodeStore {
41-
41+
4242
private Logger logger = Logger.getLogger(JDBCNodeStore.class);
4343
private final Connection conn;
4444
private final NodeStoreSQLDialect dialect;
@@ -918,20 +918,19 @@ public NodeItem convertRow(java.sql.ResultSet rs)
918918

919919
java.sql.ResultSet rs = stmt.executeQuery();
920920

921-
LinkedList<NodeItem> results = new LinkedList<NodeItem>();
922-
923-
while (rs.next()) {
924-
results.push(new NodeItemImpl(rs.getString(1), rs
925-
.getString(2), rs.getTimestamp(3), rs.getString(4),
926-
rs.getString(5)));
927-
}
928-
929-
return new ClosableIteratorImpl<NodeItem>(results.iterator());
921+
return new ResultSetIterator<NodeItem>(rs,
922+
new ResultSetIterator.RowConverter<NodeItem>() {
923+
@Override
924+
public NodeItem convertRow(java.sql.ResultSet rs)
925+
throws SQLException {
926+
return new NodeItemImpl(rs.getString(1),
927+
rs.getString(2), rs.getTimestamp(3),
928+
rs.getString(4), rs.getString(5));
929+
}
930+
});
930931
}
931932
} catch (SQLException e) {
932933
throw new NodeStoreException(e);
933-
} finally {
934-
close(stmt); // Will implicitly close the resultset if required
935934
}
936935
}
937936

@@ -966,10 +965,8 @@ public CloseableIterator<NodeItem> getFirehose(int limit,
966965
stmt.setString(
967966
4,
968967
"%@"
969-
+ Configuration
970-
.getInstance()
971-
.getProperty(
972-
Configuration.CONFIGURATION_SERVER_DOMAIN)
968+
+ Configuration.getInstance().getProperty(
969+
Configuration.CONFIGURATION_SERVER_DOMAIN)
973970
+ "%");
974971
stmt.setString(
975972
5,
@@ -1010,16 +1007,20 @@ public int getFirehoseItemCount(boolean isAdmin) throws NodeStoreException {
10101007
stmt = conn.prepareStatement(dialect.countItemsForLocalNodes());
10111008
stmt.setString(1, Conf.ACCESS_MODEL);
10121009
stmt.setString(2, accessModel);
1013-
stmt.setString(3, "%@"
1014-
+ Configuration
1015-
.getInstance()
1016-
.getProperty(
1017-
Configuration.CONFIGURATION_SERVER_DOMAIN) + "%");
1018-
stmt.setString(4, "%@"
1019-
+ Configuration
1020-
.getInstance()
1021-
.getProperty(
1022-
Configuration.CONFIGURATION_SERVER_TOPICS_DOMAIN) + "%");
1010+
stmt.setString(
1011+
3,
1012+
"%@"
1013+
+ Configuration.getInstance().getProperty(
1014+
Configuration.CONFIGURATION_SERVER_DOMAIN)
1015+
+ "%");
1016+
stmt.setString(
1017+
4,
1018+
"%@"
1019+
+ Configuration
1020+
.getInstance()
1021+
.getProperty(
1022+
Configuration.CONFIGURATION_SERVER_TOPICS_DOMAIN)
1023+
+ "%");
10231024
java.sql.ResultSet rs = stmt.executeQuery();
10241025

10251026
if (rs.next()) {
@@ -1133,19 +1134,17 @@ public CloseableIterator<NodeItem> getRecentItems(JID user, Date since,
11331134
subscription.getNodeId().length()
11341135
- node.length()).equals(node))
11351136
continue;
1136-
queryParts.add(
1137-
dialect.selectRecentItemParts()
1138-
.replace("%counter%", String.valueOf(counter))
1139-
);
1137+
queryParts.add(dialect.selectRecentItemParts().replace(
1138+
"%counter%", String.valueOf(counter)));
11401139
parameters.add(subscription.getNodeId());
11411140
parameters.add(new java.sql.Timestamp(since.getTime()));
11421141
parameters.add(maxPerNode);
11431142
++counter;
11441143
}
1145-
stmt = conn.prepareStatement(
1146-
"SELECT * FROM (" +
1147-
StringUtils.join(queryParts,
1148-
" UNION ALL ") + ") AS recentItemsQuery ORDER BY \"updated\" DESC LIMIT ?;");
1144+
stmt = conn
1145+
.prepareStatement("SELECT * FROM ("
1146+
+ StringUtils.join(queryParts, " UNION ALL ")
1147+
+ ") AS recentItemsQuery ORDER BY \"updated\" DESC LIMIT ?;");
11491148
int index = 1;
11501149
for (Object parameter : parameters) {
11511150
stmt.setObject(index, parameter);
@@ -1300,7 +1299,7 @@ public ClosableIteratorImpl<NodeItem> getNodeItemThread(String nodeId,
13001299
close(stmt); // Will implicitly close the resultset if required
13011300
}
13021301
}
1303-
1302+
13041303
@Override
13051304
public int getCountNodeThread(String nodeId, String itemId)
13061305
throws NodeStoreException {
@@ -1325,7 +1324,7 @@ public int getCountNodeThread(String nodeId, String itemId)
13251324
close(stmt); // Will implicitly close the resultset if required
13261325
}
13271326
}
1328-
1327+
13291328
@Override
13301329
public CloseableIterator<NodeItem> getNewNodeItemsForUser(JID user,
13311330
Date startDate, Date endDate) throws NodeStoreException {
@@ -1769,7 +1768,7 @@ public interface NodeStoreSQLDialect {
17691768
String selectItemThread();
17701769

17711770
String selectCountItemThread();
1772-
1771+
17731772
String insertItem();
17741773

17751774
String updateItem();

‎src/main/java/org/buddycloud/channelserver/db/jdbc/ResultSetIterator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* when the iterator returns the last item.
1818
* <p>
1919
* <b>NOTE: If the calling code does not reach the end of the iterator then
20-
* {@link #close()} should be called explicitly in order to free up database
20+
* {@link #close()} MUST be called explicitly in order to free up database
2121
* resources</b>
2222
*
2323
* @param <T>

‎src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/UserItemsGet.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
import org.buddycloud.channelserver.packetprocessor.iq.namespace.pubsub.PubSubGet;
1515
import org.buddycloud.channelserver.pubsub.accessmodel.AccessModels;
1616
import org.buddycloud.channelserver.pubsub.affiliation.Affiliations;
17+
import org.buddycloud.channelserver.pubsub.model.GlobalItemID;
1718
import org.buddycloud.channelserver.pubsub.model.NodeAffiliation;
1819
import org.buddycloud.channelserver.pubsub.model.NodeItem;
1920
import org.buddycloud.channelserver.pubsub.model.NodeSubscription;
21+
import org.buddycloud.channelserver.pubsub.model.impl.GlobalItemIDImpl;
2022
import org.buddycloud.channelserver.pubsub.subscription.Subscriptions;
2123
import org.buddycloud.channelserver.utils.node.NodeAclRefuseReason;
2224
import org.buddycloud.channelserver.utils.node.NodeViewAcl;
@@ -180,7 +182,17 @@ private void getItems() throws Exception {
180182
}
181183
Element after = resultSetManagement.element("after");
182184
if (after != null) {
183-
afterItemId = after.getTextTrim();
185+
try {
186+
GlobalItemID afterGlobalItemID = GlobalItemIDImpl.fromString(after.getTextTrim());
187+
afterItemId = afterGlobalItemID.getItemID();
188+
189+
if(!afterGlobalItemID.getNodeID().equals(node)) {
190+
createExtendedErrorReply(Type.modify, Condition.item_not_found, "RSM 'after' specifies an unexpected NodeID: " + afterGlobalItemID.getNodeID());
191+
}
192+
} catch(IllegalArgumentException e) {
193+
createExtendedErrorReply(Type.modify, Condition.bad_request, "Could not parse the 'after' id: " + after);
194+
return;
195+
}
184196
}
185197
}
186198

‎src/main/java/org/buddycloud/channelserver/pubsub/model/GlobalItemID.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package org.buddycloud.channelserver.pubsub.model;
2+
3+
import org.xmpp.packet.JID;
4+
5+
/**
6+
* Pointer to a globally identifiable node item, using a service JID, node ID and itemID.
7+
* The Item is unique within the Node
8+
* The Node is unique within the Service
9+
* The Service is unique within the network.
10+
*/
11+
public interface GlobalItemID {
12+
/**
13+
* Gets the JID of service which hosts the node (and item)
14+
* @return the JID of the service.
15+
*/
16+
public JID getService();
17+
18+
/**
19+
* Gets the NodeID of the node containing the item
20+
* @return the NodeID of the node.
21+
*/
22+
public String getNodeID();
23+
24+
/**
25+
* Gets the ItemID of the item within the Node
26+
* @return the ItemID if the item.
27+
*/
28+
public String getItemID();
29+
}

‎src/main/java/org/buddycloud/channelserver/pubsub/model/impl/GlobalItemIDImpl.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package org.buddycloud.channelserver.pubsub.model.impl;
2+
3+
import java.util.regex.Matcher;
4+
import java.util.regex.Pattern;
5+
6+
import org.buddycloud.channelserver.pubsub.model.GlobalItemID;
7+
import org.xmpp.packet.JID;
8+
9+
/**
10+
* Basic implementation of {@link GlobalItemID}
11+
*/
12+
public class GlobalItemIDImpl implements GlobalItemID {
13+
14+
private static final Pattern STRING_PATTERN = Pattern.compile("^tag:([^,]+),([^,]+),([^,]+)$");
15+
16+
/**
17+
* Creates a new global item ID
18+
* @param service the JID of the pubsub service hosting the node
19+
* @param nodeID the id of the node containing the item
20+
* @param itemID the id of the item
21+
*/
22+
public GlobalItemIDImpl(JID service, String nodeID, String itemID) {
23+
this.service = service;
24+
this.nodeID = nodeID;
25+
this.itemID = itemID;
26+
}
27+
28+
private JID service;
29+
private String nodeID;
30+
private String itemID;
31+
32+
@Override
33+
public JID getService() {
34+
return service;
35+
}
36+
37+
@Override
38+
public String getNodeID() {
39+
return nodeID;
40+
}
41+
42+
@Override
43+
public String getItemID() {
44+
return itemID;
45+
}
46+
47+
@Override
48+
public String toString() {
49+
StringBuilder builder = new StringBuilder();
50+
51+
builder.append("tag:");
52+
builder.append(service.toString());
53+
builder.append(",");
54+
builder.append(nodeID);
55+
builder.append(",");
56+
builder.append(itemID);
57+
58+
return builder.toString();
59+
}
60+
61+
/**
62+
* Creates a new {@link GlobalItemID} from a tag string which looks a bit like:
63+
* <blockquote><code>tag:pubsub.server.com,a/node/id,an/item/id</code></blockquote>
64+
* @param str The tag string
65+
* @return the {@link GlobalItemID}
66+
*/
67+
public static GlobalItemID fromString(final String str) {
68+
Matcher matcher = STRING_PATTERN.matcher(str);
69+
70+
if(!matcher.matches()) {
71+
throw new IllegalArgumentException(str + " is not a valid GlobalItemID String. Expected something like 'tag:example.com,node,item'.");
72+
}
73+
74+
JID service = new JID(matcher.group(1));
75+
76+
// Handle an issue with certain clients
77+
if((service.getNode() != null) && service.getNode().equals("null")) {
78+
service = new JID(null, service.getDomain(), service.getResource());
79+
}
80+
81+
return new GlobalItemIDImpl(service, matcher.group(2), matcher.group(3));
82+
}
83+
}

‎src/main/java/org/buddycloud/channelserver/pubsub/model/impl/NodeItemImpl.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,42 @@ public String getUID() {
5353
public String getInReplyTo() {
5454
return inReplyTo;
5555
}
56+
57+
@Override
58+
public String toString() {
59+
return "NodeItemImpl [nodeId=" + nodeId + ", id=" + id + ", payload="
60+
+ payload + ", updated=" + updated + ", inReplyTo=" + inReplyTo
61+
+ "]";
62+
}
63+
64+
@Override
65+
public int hashCode() {
66+
final int prime = 31;
67+
int result = 1;
68+
result = prime * result + ((id == null) ? 0 : id.hashCode());
69+
result = prime * result + ((nodeId == null) ? 0 : nodeId.hashCode());
70+
return result;
71+
}
72+
73+
@Override
74+
public boolean equals(Object obj) {
75+
if (this == obj)
76+
return true;
77+
if (obj == null)
78+
return false;
79+
if (!(obj instanceof NodeItem))
80+
return false;
81+
NodeItem other = (NodeItem) obj;
82+
if (id == null) {
83+
if (other.getId() != null)
84+
return false;
85+
} else if (!id.equals(other.getId()))
86+
return false;
87+
if (nodeId == null) {
88+
if (other.getNodeId() != null)
89+
return false;
90+
} else if (!nodeId.equals(other.getNodeId()))
91+
return false;
92+
return true;
93+
}
5694
}

‎src/test/java/org/buddycloud/channelserver/db/jdbc/JDBCNodeStoreTest.java‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,11 +1289,11 @@ public void testGetNodeItemsWithLimits() throws Exception {
12891289
Iterator<NodeItem> result = store.getNodeItems(TEST_SERVER1_NODE1_ID,
12901290
TEST_SERVER1_NODE1_ITEM4_ID, 4);
12911291

1292-
String[] expectedNodeIds = { TEST_SERVER1_NODE1_ITEM1_ID,
1293-
TEST_SERVER1_NODE1_ITEM2_ID, TEST_SERVER1_NODE1_ITEM3_ID };
1294-
String[] expectedEntryContent = { TEST_SERVER1_NODE1_ITEM1_CONTENT,
1292+
String[] expectedNodeIds = { TEST_SERVER1_NODE1_ITEM3_ID,
1293+
TEST_SERVER1_NODE1_ITEM2_ID, TEST_SERVER1_NODE1_ITEM1_ID };
1294+
String[] expectedEntryContent = { TEST_SERVER1_NODE1_ITEM3_CONTENT,
12951295
TEST_SERVER1_NODE1_ITEM2_CONTENT,
1296-
TEST_SERVER1_NODE1_ITEM3_CONTENT };
1296+
TEST_SERVER1_NODE1_ITEM1_CONTENT };
12971297

12981298
int i = 0;
12991299

@@ -1315,6 +1315,27 @@ public void testGetNodeItemsWithLimits() throws Exception {
13151315
assertFalse("Too many items were returned", result.hasNext());
13161316
}
13171317

1318+
@Test
1319+
public void testGetNodeItemsWithPaging() throws Exception {
1320+
dbTester.loadData("node_1");
1321+
1322+
long start = System.currentTimeMillis();
1323+
1324+
NodeItem[] items = new NodeItem[20];
1325+
1326+
for (int i = 0; i < 20; i++) {
1327+
items[i] = new NodeItemImpl(TEST_SERVER1_NODE1_ID, String
1328+
.valueOf(i), new Date(start + i * 10), "payload" + String.valueOf(i));
1329+
store.addNodeItem(items[i]);
1330+
}
1331+
1332+
CloseableIterator<NodeItem> result = store.getNodeItems(TEST_SERVER1_NODE1_ID, "15", 3);
1333+
1334+
assertEquals("Incorrect node item returned", items[14], result.next());
1335+
assertEquals("Incorrect node item returned", items[13], result.next());
1336+
assertEquals("Incorrect node item returned", items[12], result.next());
1337+
}
1338+
13181339
@Test
13191340
public void testGetNodeItemsWithNegativeOneCountReturnsAllItems()
13201341
throws Exception {

0 commit comments

Comments
 (0)