Skip to content

Feature/marker pagination fixes #298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Feature/marker pagination fixes #298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1adc4ce
test: Fix wrong assert in PersistedEntryTest
reftel Sep 9, 2016
24c3606
test: Assert response code in FeedTagTest
reftel Sep 9, 2016
c03a5d7
test: align AtomHopperJettyServerBuilder impls
reftel Oct 5, 2016
d072422
test: Programs for verifying following by marker
reftel Sep 27, 2016
db207d9
hibernate: Use dateLastUpdated for pagination
reftel Sep 9, 2016
b0a93f9
Use Java 8
reftel Aug 17, 2016
68552a6
Use Hibernate 5.1
reftel Aug 17, 2016
eac408d
Use higher precision time stamps internally
reftel Aug 17, 2016
59d016c
hibernate: Use database timestamps for entries
reftel Sep 9, 2016
3091020
hibernate: Lock the feed when adding entries
reftel Sep 9, 2016
77393d0
hibernate: Mark timesetamps unique
reftel Sep 9, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions adapters/hibernate/src/main/java/org/atomhopper/hibernate/HibernateFeedRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.atomhopper.hibernate;

import java.sql.Timestamp;
import java.time.Instant;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
Expand All @@ -18,10 +20,13 @@
import org.atomhopper.hibernate.actions.SimpleSessionAction;
import org.atomhopper.hibernate.query.CategoryCriteriaGenerator;
import org.hibernate.Criteria;
import org.hibernate.LockMode;
import org.hibernate.Query;
import org.hibernate.Session;
import org.hibernate.Transaction;
import org.hibernate.criterion.Order;
import org.hibernate.criterion.Restrictions;
import org.hibernate.type.TimestampType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -162,13 +167,13 @@ public List<PersistedEntry> perform(Session liveSession) {

switch (direction) {
case FORWARD:
criteria.add(Restrictions.gt(DATE_LAST_UPDATED, markerEntry.getCreationDate())).addOrder(Order.asc(DATE_LAST_UPDATED));
criteria.add(Restrictions.gt(DATE_LAST_UPDATED, markerEntry.getDateLastUpdated())).addOrder(Order.asc(DATE_LAST_UPDATED));
feedPage.addAll(criteria.list());
Collections.reverse(feedPage);
break;

case BACKWARD:
criteria.add(Restrictions.le(DATE_LAST_UPDATED, markerEntry.getCreationDate())).addOrder(Order.desc(DATE_LAST_UPDATED));
criteria.add(Restrictions.le(DATE_LAST_UPDATED, markerEntry.getDateLastUpdated())).addOrder(Order.desc(DATE_LAST_UPDATED));
feedPage.addAll(criteria.list());
break;
}
Expand Down Expand Up @@ -254,6 +259,20 @@ public void perform(Session liveSession) {

if (feed == null) {
feed = entry.getFeed();
} else {
liveSession.lock(feed, LockMode.PESSIMISTIC_WRITE);
}

if (entry.getCreationDate() == null || entry.getDateLastUpdated() == null) {
final Instant now = ((Timestamp)liveSession.createQuery(
"select coalesce(max(current_timestamp()), current_timestamp()) from PersistedFeed"
).uniqueResult()).toInstant();
if (entry.getCreationDate() == null) {
entry.setCreationDate(now);
}
if (entry.getDateLastUpdated() == null) {
entry.setDateLastUpdated(now);
}
}

liveSession.saveOrUpdate(feed);
Expand Down
10 changes: 5 additions & 5 deletions ...ters/hibernate/src/main/java/org/atomhopper/hibernate/adapter/HibernateFeedPublisher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ public AdapterResponse<Entry> postEntry(PostEntryRequest postEntryRequest) {
Date updated = abderaParsedEntry.getUpdated();

if (updated != null) {
persistedEntry.setDateLastUpdated(updated);
persistedEntry.setCreationDate(updated);
persistedEntry.setDateLastUpdated(updated.toInstant());
persistedEntry.setCreationDate(updated.toInstant());
}
}

Expand All @@ -106,11 +106,11 @@ public AdapterResponse<Entry> postEntry(PostEntryRequest postEntryRequest) {
persistedEntry.setFeed(feedRef);
persistedEntry.setEntryBody(entryToString(abderaParsedEntry));

abderaParsedEntry.setUpdated(persistedEntry.getDateLastUpdated());
abderaParsedEntry.setPublished(persistedEntry.getCreationDate());

feedRepository.saveEntry(persistedEntry);

abderaParsedEntry.setUpdated(Date.from(persistedEntry.getDateLastUpdated()));
abderaParsedEntry.setPublished(Date.from(persistedEntry.getCreationDate()));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you flip the order on this?

incrementCounterForFeed(postEntryRequest.getFeedName());

return ResponseBuilder.created(abderaParsedEntry);
Expand Down
5 changes: 3 additions & 2 deletions adapters/hibernate/src/main/java/org/atomhopper/hibernate/adapter/HibernateFeedSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.io.UnsupportedEncodingException;
import java.net.URL;
import java.net.URLEncoder;
import java.sql.Date;
import java.util.List;
import java.util.Map;
import java.util.UUID;
Expand Down Expand Up @@ -163,8 +164,8 @@ private Entry hydrateEntry(PersistedEntry persistedEntry, Abdera abderaReference
if (hydratedEntryDocument != null) {
entry = hydratedEntryDocument.getRoot();

entry.setUpdated(persistedEntry.getDateLastUpdated());
entry.setPublished(persistedEntry.getCreationDate());
entry.setUpdated(Date.from(persistedEntry.getDateLastUpdated()));
entry.setPublished(Date.from(persistedEntry.getCreationDate()));
}

return entry;
Expand Down
205 changes: 166 additions & 39 deletions adapters/hibernate/src/test/java/org/atomhopper/hibernate/HibernateFeedRepositoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package org.atomhopper.hibernate;

import static org.mockito.Mockito.mock;
import static org.junit.Assert.assertFalse;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertTrue;

import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -20,11 +21,12 @@
import org.atomhopper.adapter.jpa.PersistedCategory;
import org.atomhopper.adapter.jpa.PersistedEntry;
import org.atomhopper.adapter.jpa.PersistedFeed;
import org.atomhopper.dbal.AtomDatabaseException;
import org.atomhopper.hibernate.actions.ComplexSessionAction;
import org.atomhopper.dbal.PageDirection;
import org.atomhopper.hibernate.actions.SimpleSessionAction;
import org.atomhopper.hibernate.query.SimpleCategoryCriteriaGenerator;
import org.hibernate.Session;
import org.hibernate.criterion.Restrictions;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
Expand All @@ -39,11 +41,8 @@
@RunWith(Enclosed.class)
public class HibernateFeedRepositoryTest {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing the WhenPerformingSimpleAction and the WhenPerformingComplexAction test classes?

Should WhenCreatingMisconfiguredFeedRepository be an additional class rather than replacing?


public static class WhenPerformingSimpleAction {

HibernateFeedRepository feedRepository;
public static class WhenCreatingMisconfiguredFeedRepository{
Map<String, String> parameters;
SimpleSessionAction simpleSessionAction;

@Before
public void setup() throws Exception {
Expand All @@ -54,40 +53,11 @@ public void setup() throws Exception {
parameters.put("hibernate.connection.username", "sa");
parameters.put("hibernate.connection.password", "");
parameters.put("hibernate.hbm2ddl.auto", "update");

feedRepository = new HibernateFeedRepository(parameters);
simpleSessionAction = mock(SimpleSessionAction.class);
}

@Test(expected=AtomDatabaseException.class)
public void shouldThrowAtomDatabaseException() throws Exception {
feedRepository.performSimpleAction(simpleSessionAction);
}
}

public static class WhenPerformingComplexAction {

HibernateFeedRepository feedRepository;
Map<String, String> parameters;
ComplexSessionAction complexSessionAction;

@Before
public void setup() throws Exception {
parameters = new HashMap<String, String>();
parameters.put("hibernate.connection.driver_class", "org.h2.Driver");
parameters.put("hibernate.dialect", "org.hibernate.dialect.H2Dialect");
parameters.put("hibernate.connection.username", "sa");
parameters.put("hibernate.connection.password", "");
parameters.put("hibernate.hbm2ddl.auto", "update");

feedRepository = new HibernateFeedRepository(parameters);
complexSessionAction = mock(ComplexSessionAction.class);
}

/*This should throw the error because */
@Test(expected=AtomDatabaseException.class)
@Test(expected=UnsupportedOperationException.class)
public void shouldThrowAtomDatabaseException() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change the name of this test as its throwing a different exception?

feedRepository.performComplexAction(complexSessionAction);
new HibernateFeedRepository(parameters);
}
}

Expand All @@ -99,7 +69,7 @@ public static void setup() throws Exception {
Map<String, String> parameters;
parameters = new HashMap<String, String>();
parameters.put("hibernate.connection.driver_class", "org.h2.Driver");
parameters.put("hibernate.connection.url", "jdbc:h2:mem:WhenCreatingFeed");
parameters.put("hibernate.connection.url", "jdbc:h2:mem:WhenCreatingEntry");
parameters.put("hibernate.dialect", "org.hibernate.dialect.H2Dialect");
parameters.put("hibernate.connection.username", "sa");
parameters.put("hibernate.connection.password", "");
Expand Down Expand Up @@ -158,6 +128,110 @@ public void perform(Session liveSession) {
}
});
}

@Test
public void pagingForwardsShouldNotFindTheEntry() {
final PersistedEntry entry = feedRepository.getEntry("entryId", "feedName");
final List<PersistedEntry> page = feedRepository.getFeedPage(
"feedName", entry, PageDirection.FORWARD, new SimpleCategoryCriteriaGenerator(""), 1
);
Assert.assertTrue(page.isEmpty());
}

@Test
public void pagingBackwardsShouldFindTheEntry() {
final PersistedEntry entry = feedRepository.getEntry("entryId", "feedName");
final List<PersistedEntry> page = feedRepository.getFeedPage(
"feedName", entry, PageDirection.BACKWARD, new SimpleCategoryCriteriaGenerator(""), 1
);
Assert.assertEquals(1, page.size());
Assert.assertEquals("entryId", page.get(0).getEntryId());
}
}

public static class WhenCreatingTwoEntries {
static HibernateFeedRepository feedRepository;

@BeforeClass
public static void setup() throws Exception {
Map<String, String> parameters;
parameters = new HashMap<String, String>();
parameters.put("hibernate.connection.driver_class", "org.h2.Driver");
parameters.put("hibernate.connection.url", "jdbc:h2:mem:WhenCreatingTwoEntries");
parameters.put("hibernate.dialect", "org.hibernate.dialect.H2Dialect");
parameters.put("hibernate.connection.username", "sa");
parameters.put("hibernate.connection.password", "");
parameters.put("hibernate.hbm2ddl.auto", "update");

feedRepository = new HibernateFeedRepository(parameters);

PersistedFeed feed = new PersistedFeed("feedName", "feedId");

PersistedEntry entry1 = new PersistedEntry("entry1");
Instant earlier = Instant.now().minusSeconds(60);
entry1.setDateLastUpdated(earlier);
entry1.setCreationDate(earlier);
entry1.setFeed(feed);

feedRepository.saveEntry(entry1);

PersistedEntry entry2 = new PersistedEntry("entry2");
Instant now = Instant.now();
entry2.setDateLastUpdated(now);
entry2.setCreationDate(now);
entry2.setFeed(feed);

feedRepository.saveEntry(entry2);
}

@Test
public void entriesShouldBeSortedByRecency() throws Exception {
final List<PersistedEntry> entries = feedRepository.getFeedHead(
"feedName", new SimpleCategoryCriteriaGenerator(""), 2
);
Assert.assertEquals(entries.get(0).getEntryId(), "entry2");
Assert.assertEquals(entries.get(1).getEntryId(), "entry1");
}

@Test
public void pagingBackwardsFromLatestShouldFindBoth() {
final PersistedEntry latest = feedRepository.getEntry("entry2", "feedName");
final List<PersistedEntry> page = feedRepository.getFeedPage(
"feedName", latest, PageDirection.BACKWARD, new SimpleCategoryCriteriaGenerator(""), 2
);
Assert.assertEquals(2, page.size());
Assert.assertEquals("entry2", page.get(0).getEntryId());
Assert.assertEquals("entry1", page.get(1).getEntryId());
}

@Test
public void pagingForwardsFromLatestShouldNotFindAnything() {
final PersistedEntry latest = feedRepository.getEntry("entry2", "feedName");
final List<PersistedEntry> page = feedRepository.getFeedPage(
"feedName", latest, PageDirection.FORWARD, new SimpleCategoryCriteriaGenerator(""), 2
);
Assert.assertTrue(page.isEmpty());
}

@Test
public void pagingBackwardsFromOldestShouldOnlyFindOldest() {
final PersistedEntry oldest = feedRepository.getEntry("entry1", "feedName");
final List<PersistedEntry> page = feedRepository.getFeedPage(
"feedName", oldest, PageDirection.BACKWARD, new SimpleCategoryCriteriaGenerator(""), 2
);
Assert.assertEquals(1, page.size());
Assert.assertEquals("entry1", page.get(0).getEntryId());
}

@Test
public void pagingForwardsFromOldestShouldFindLatest() {
final PersistedEntry oldest = feedRepository.getEntry("entry1", "feedName");
final List<PersistedEntry> page = feedRepository.getFeedPage(
"feedName", oldest, PageDirection.FORWARD, new SimpleCategoryCriteriaGenerator(""), 2
);
Assert.assertEquals(1, page.size());
Assert.assertEquals("entry2", page.get(0).getEntryId());
}
}

public static class WhenGettingCategories {
Expand Down Expand Up @@ -357,5 +431,58 @@ public Integer run() {
r1.get(3000);
r2.get(3000);
}

@Test
public void entriesShouldGetDistinctIds() throws Exception {

final PersistedEntry existing = new PersistedEntry("existing");
final PersistedFeed feed = new PersistedFeed("feed1", "feed1");
feed.setEntries(Collections.singleton(existing));
existing.setFeed(feed);
feedRepository.saveEntry(existing);

final Runner.Future<PersistedEntry> r1 = runner1.run(new Runner.Operation<PersistedEntry>() {
@Override
public PersistedEntry run() {
final PersistedEntry entry = new PersistedEntry("entry2a");
feed.setEntries(Collections.singleton(entry));
entry.setFeed(feed);
try {
barrier.await();
} catch (InterruptedException e) {
e.printStackTrace();
} catch (BrokenBarrierException e) {
e.printStackTrace();
}

feedRepository.saveEntry(entry);
return entry;
}
});
final Runner.Future<PersistedEntry> r2 = runner2.run(new Runner.Operation<PersistedEntry>() {
@Override
public PersistedEntry run() {
final PersistedEntry entry = new PersistedEntry("entry2b");
feed.setEntries(Collections.singleton(entry));
entry.setFeed(feed);
try {
barrier.await();
} catch (InterruptedException e) {
e.printStackTrace();
} catch (BrokenBarrierException e) {
e.printStackTrace();
}

feedRepository.saveEntry(entry);
return entry;
}
});
final PersistedEntry entry1 = r1.get(3000);
final PersistedEntry entry2 = r2.get(3000);

assertFalse(entry1.getEntryId().equals(entry2.getEntryId()));
assertFalse(entry1.getCreationDate().isBefore(existing.getCreationDate()));
assertFalse(entry2.getCreationDate().isBefore(existing.getCreationDate()));
}
}
}
6 changes: 5 additions & 1 deletion ...ers/hibernate/src/test/java/org/atomhopper/hibernate/adapter/HibernateFeedSourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.atomhopper.hibernate.adapter;

import java.net.URL;
import java.time.Instant;
import java.util.*;

import static junit.framework.Assert.assertEquals;
Expand Down Expand Up @@ -72,6 +73,9 @@ public void setUp() throws Exception {
persistedEntry.setFeed(persistedFeed);
persistedEntry.setEntryId(ID);
persistedEntry.setEntryBody(ENTRY_BODY);
Instant now = Instant.now();
persistedEntry.setCreationDate(now);
persistedEntry.setDateLastUpdated(now);
}

@Test(expected = UnsupportedOperationException.class)
Expand Down Expand Up @@ -170,4 +174,4 @@ public void shouldGetCurrentLinkFromArchiveFeedAndArchiveNode() throws Exception
assertTrue("'<fn:archive>' node should exist", found );
}
}
}
}
Loading