Enabling fetch of multiple things with one request

marcioos · Dec 17, 2013 · 19bece5 · 19bece5
1 parent 0543516
commit 19bece5
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 57 deletions.
diff --git a/pom.xml b/pom.xml
@@ -2,7 +2,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>co.yellowbricks</groupId>
     <artifactId>bgg-client</artifactId>
-    <version>1.2-SNAPSHOT</version>
+    <version>1.3-SNAPSHOT</version>
     <name>Board Game Geek Client</name>
 
     <properties>

diff --git a/src/main/java/co/yellowbricks/bggclient/BGG.java b/src/main/java/co/yellowbricks/bggclient/BGG.java
@@ -1,5 +1,7 @@
 package co.yellowbricks.bggclient;
 
+import java.util.Collection;
+
 import javax.xml.bind.JAXBException;
 
 import co.yellowbricks.bggclient.common.NoItemsFoundException;
@@ -32,17 +34,17 @@ public static SearchOutput search(String query, ThingType... thingTypes) throws
         }
     }
 
-    public static FetchItem fetch(int id, ThingType... thingTypes) throws FetchException, NoItemsFoundException {
+    public static Collection<FetchItem> fetch(Collection<Integer> ids, ThingType... thingTypes) throws FetchException, NoItemsFoundException {
         try {
-            FetchItemOutput items = (FetchItemOutput) FetchItemOutput.UNMARSHALLER.unmarshal(BggService.INSTANCE.fetch(id, thingTypes));
+            FetchItemOutput items = (FetchItemOutput) FetchItemOutput.UNMARSHALLER.unmarshal(BggService.INSTANCE.fetch(ids, thingTypes));
 
             if (items.getItems() != null && !items.getItems().isEmpty())
-                return items.getItems().get(0);
+                return items.getItems();
             throw new NoItemsFoundException();
         } catch (BggServiceException e) {
-            throw new FetchException("While fetching id: " + id, e);
+            throw new FetchException("While fetching ids: " + ids, e);
         } catch (JAXBException e) {
-            throw new FetchException("While fetching id: " + id, e);
+            throw new FetchException("While fetching ids: " + ids, e);
         }
     }
 

diff --git a/src/main/java/co/yellowbricks/bggclient/request/BggService.java b/src/main/java/co/yellowbricks/bggclient/request/BggService.java
@@ -1,20 +1,23 @@
 package co.yellowbricks.bggclient.request;
 
-import co.yellowbricks.bggclient.common.ThingType;
-import com.ning.http.client.AsyncHttpClient.BoundRequestBuilder;
+import java.util.Collection;
 
 import javax.xml.transform.Source;
 import javax.xml.transform.stream.StreamSource;
 
+import co.yellowbricks.bggclient.common.ThingType;
+
+import com.ning.http.client.AsyncHttpClient.BoundRequestBuilder;
+
 public enum BggService {
-	
+
 	INSTANCE;
 
 	private static final String BASE_URL = "http://www.boardgamegeek.com/xmlapi2";
 	private static final String SEARCH_URL = BASE_URL + "/search";
 	private static final String FETCH_URL = BASE_URL + "/thing";
 	private static final String COLLECTION_URL = BASE_URL + "/collection";
-	
+
 	public Source search(final String query, final ThingType... thingTypes) throws BggServiceException {
 		return new StreamSource(HttpRequester.INSTANCE.executeRequest(SEARCH_URL, new ParameterAdder() {
 			@Override
@@ -24,16 +27,16 @@ public BoundRequestBuilder addParameters(BoundRequestBuilder requestBuilder) {
 			}
 		}));
 	}
-	
-	public Source fetch(final int id, final ThingType... thingTypes) throws BggServiceException {
+
+	public Source fetch(final Collection<Integer> ids, final ThingType... thingTypes) throws BggServiceException {
 		return new StreamSource(HttpRequester.INSTANCE.executeRequest(FETCH_URL, new ParameterAdder() {
 			@Override
 			public BoundRequestBuilder addParameters(BoundRequestBuilder requestBuilder) {
-				return addThingTypesToQuery(requestBuilder, thingTypes).addQueryParameter("id", String.valueOf(id));
+				return addThingTypesToQuery(requestBuilder, thingTypes).addQueryParameter("id", getIdsAsString(ids));
 			}
 		}));
 	}
-	
+
 	public Source fetchCollection(final String ownerName) throws BggServiceException {
 		return new StreamSource(HttpRequester.INSTANCE.executeRequest(COLLECTION_URL, new ParameterAdder() {
 			@Override
@@ -42,19 +45,28 @@ public BoundRequestBuilder addParameters(BoundRequestBuilder requestBuilder) {
 			}
 		}));
 	}
-	
+
 	private BoundRequestBuilder addThingTypesToQuery(BoundRequestBuilder requestBuilder, final ThingType... thingTypes) {
 	    if (thingTypes != null)
 	        return requestBuilder.addQueryParameter("type", getTypesQueryString(thingTypes));
         return requestBuilder;
     }
 
-    private String getTypesQueryString(ThingType... thingTypes) {
+    private static String getTypesQueryString(ThingType... thingTypes) {
         StringBuilder builder = new StringBuilder();
         for (int i = 0; i < thingTypes.length; i++) {
             builder.append(thingTypes[i].getKey());
             if (i < (thingTypes.length - 1)) builder.append(",");
         }
         return builder.toString();
     }
+
+    private static String getIdsAsString(Collection<Integer> ids) {
+        StringBuilder builder = new StringBuilder();
+        for (Integer id : ids) {
+            builder.append(String.valueOf(id));
+            builder.append(',');
+        }
+        return builder.length() > 0 ? builder.substring(0, builder.length() - 1): "";
+    }
 }
diff --git a/src/test/java/co/yellowbricks/bggclient/BGGFetchTest.java b/src/test/java/co/yellowbricks/bggclient/BGGFetchTest.java
@@ -1,67 +1,83 @@
 package co.yellowbricks.bggclient;
 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+
+import org.junit.Test;
+
 import co.yellowbricks.bggclient.common.NoItemsFoundException;
 import co.yellowbricks.bggclient.common.ThingType;
 import co.yellowbricks.bggclient.fetch.FetchException;
 import co.yellowbricks.bggclient.fetch.domain.CollectionItem;
 import co.yellowbricks.bggclient.fetch.domain.FetchItem;
 import co.yellowbricks.bggclient.fetch.domain.UserCollection;
-import org.junit.Test;
-
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
 
 public class BGGFetchTest {
 
     @Test
     public void shouldFetchDieMacher() throws FetchException, NoItemsFoundException {
         int dieMacherId = 1;
-        
-        FetchItem item = BGG.fetch(dieMacherId, ThingType.BOARDGAME);
+
+        FetchItem item = BGG.fetch(Arrays.asList(dieMacherId), ThingType.BOARDGAME).iterator().next();
 
         assertThat(item.getName(), containsString("Macher"));
     }
-    
-   @Test
+
+    @Test
     public void shouldFetchAgricolaXDeck() throws FetchException, NoItemsFoundException {
         int agricolaXDeckId = 38733;
-        
-        FetchItem item = BGG.fetch(agricolaXDeckId, ThingType.BOARDGAME_EXPANSION);
+
+        FetchItem item = BGG.fetch(Arrays.asList(agricolaXDeckId), ThingType.BOARDGAME_EXPANSION).iterator().next();
 
         assertThat(item.getName(), containsString("Agricola"));
     }
-
+
+    @Test
+    public void shouldFetchAgricolaXDeckAndDieMacher() throws FetchException, NoItemsFoundException {
+        int agricolaXDeckId = 38733;
+        int dieMacherId = 1;
+
+        Collection<FetchItem> item = BGG.fetch(Arrays.asList(agricolaXDeckId, dieMacherId));
+
+        assertThat(((ArrayList<FetchItem>) item).get(0).getName(), containsString("Agricola"));
+        assertThat(((ArrayList<FetchItem>) item).get(1).getName(), containsString("Macher"));
+    }
+
     @Test(expected = NoItemsFoundException.class)
     public void shouldFindNoItems() throws FetchException, NoItemsFoundException {
-        BGG.fetch(13123123);
+        BGG.fetch(Arrays.asList(13123123, 2380182));
     }
-    
+
     @Test
     public void shouldFetchMyCollection() throws FetchException, NoItemsFoundException {
         String myName = "marcio_os";
-        
+
         UserCollection myCollection = BGG.fetchCollection(myName);
-        
+
         assertThat(myCollection.getTotalItems(), is(13));
     }
-    
+
     @Test
     public void collectionShouldContainBGGTermsOfUseUrl() throws FetchException, NoItemsFoundException {
         String myName = "marcio_os";
-        
+
         UserCollection myCollection = BGG.fetchCollection(myName);
-        
+
         assertThat(myCollection.getTermsOfUseUrl(), is("http://boardgamegeek.com/xmlapi/termsofuse"));
     }
-    
+
     @Test
     public void myCollectionShouldContainDominion() throws FetchException, NoItemsFoundException {
         String myName = "marcio_os";
-        
+
         UserCollection myCollection = BGG.fetchCollection(myName);
-        
+
         for (CollectionItem item : myCollection.getItems()) if ("Dominion".equals(item.getName())) return;
         fail("Dominion not found");
     }

diff --git a/src/test/java/co/yellowbricks/bggclient/BGGSearchTest.java b/src/test/java/co/yellowbricks/bggclient/BGGSearchTest.java
@@ -16,20 +16,20 @@ public class BGGSearchTest {
     public void shouldReturnCorrectAmountOfDominionGames() throws SearchException, NoItemsFoundException {
         SearchOutput items = BGG.search("dominion", ThingType.BOARDGAME);
 
-        Assert.assertThat(items.getTotal(), CoreMatchers.is(28));
+        Assert.assertThat(items.getTotal(), CoreMatchers.is(29));
     }
 
     @Test(expected = NoItemsFoundException.class)
     public void shouldFindNoItems() throws SearchException, NoItemsFoundException {
         BGG.search("a game that should not exist");
     }
-    
+
     @Test
     public void searchBoardgamesShouldReturnExpansion() throws SearchException, NoItemsFoundException {
         int agricolaXDeckId = 38733;
-        
+
         SearchOutput items = BGG.search("agricola", ThingType.BOARDGAME);
-        
+
         for (SearchItem item : items.getItems()) if (item.getId() == agricolaXDeckId) return;
         Assert.fail("SearchOutput does not contain Agricola X-Deck id");
     }

diff --git a/src/test/java/co/yellowbricks/bggclient/fetch/domain/FetchItemTest.java b/src/test/java/co/yellowbricks/bggclient/fetch/domain/FetchItemTest.java
@@ -17,36 +17,36 @@ public class FetchItemTest {
 
     @Test
     public void shouldGetBestNumberOfPlayersForDieMacher() throws FetchException, NoItemsFoundException {
-        FetchItem dieMacher = BGG.fetch(DIE_MACHER_ID);
-        
+        FetchItem dieMacher = BGG.fetch(Arrays.asList(DIE_MACHER_ID)).iterator().next();
+
         Assert.assertThat(dieMacher.getBestNumberOfPlayers(), CoreMatchers.equalTo("5"));
     }
-    
+
     @Test
     public void shouldGetDieMacherCategories() throws FetchException, NoItemsFoundException {
-        FetchItem dieMacher = BGG.fetch(DIE_MACHER_ID);
-        
+        FetchItem dieMacher = BGG.fetch(Arrays.asList(DIE_MACHER_ID)).iterator().next();
+
         Assert.assertThat(dieMacher.getCategories(), CoreMatchers.equalTo(Arrays.asList("Dice", "Economic", "Negotiation", "Political")));
     }
-    
+
     @Test
     public void shouldGetDieMacherMechanics() throws FetchException, NoItemsFoundException {
-        FetchItem dieMacher = BGG.fetch(DIE_MACHER_ID);
-        
+        FetchItem dieMacher = BGG.fetch(Arrays.asList(DIE_MACHER_ID)).iterator().next();
+
         Assert.assertThat(dieMacher.getMechanics(), CoreMatchers.equalTo(Arrays.asList("Area Control / Area Influence", "Auction/Bidding", "Dice Rolling", "Hand Management")));
     }
-    
+
     @Test
     public void shouldGetDieMacherDesigners() throws FetchException, NoItemsFoundException {
-        FetchItem dieMacher = BGG.fetch(DIE_MACHER_ID);
-        
+        FetchItem dieMacher = BGG.fetch(Arrays.asList(DIE_MACHER_ID)).iterator().next();
+
         Assert.assertThat(dieMacher.getDesigners(), CoreMatchers.equalTo(Arrays.asList("Karl-Heinz Schmiel")));
     }
-    
+
     @Test
     public void dominion2004ShouldHaveUnknownBestNumberOfPlayers() throws FetchException, NoItemsFoundException {
-        FetchItem dominion2004 = BGG.fetch(DOMINION_2004_ID);
-        
+        FetchItem dominion2004 = BGG.fetch(Arrays.asList(DOMINION_2004_ID)).iterator().next();
+
         Assert.assertThat(dominion2004.getBestNumberOfPlayers(), CoreMatchers.equalTo("unknown"));
     }
 }