Skip to content

Commit

Permalink
Makes iterator compatible with Java iterator expected behavior (#1117)
Browse files Browse the repository at this point in the history
* Makes iterator compatible with Java iterator expected behavior
both next() and hasNext() should read from stream if needed. both also inspect 'current' member, next() resets it after consuming. exception is thrown when no more elements are available to return.

* Fixing CI - formatting issue
  • Loading branch information
ItamarBenjamin authored and velo committed Nov 24, 2019
1 parent 2517b51 commit 3c8f568
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 17 deletions.
33 changes: 22 additions & 11 deletions jackson/src/main/java/feign/jackson/JacksonIteratorDecoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,9 @@

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.Module;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectReader;
import com.fasterxml.jackson.databind.RuntimeJsonMappingException;
import com.fasterxml.jackson.databind.*;
import feign.Response;
import feign.Util;
import feign.codec.DecodeException;
import feign.codec.Decoder;
import java.io.BufferedReader;
Expand All @@ -32,6 +28,7 @@
import java.lang.reflect.Type;
import java.util.Collections;
import java.util.Iterator;
import java.util.NoSuchElementException;
import static feign.Util.ensureClosed;

/**
Expand Down Expand Up @@ -131,33 +128,47 @@ static final class JacksonIterator<T> implements Iterator<T>, Closeable {

@Override
public boolean hasNext() {
if (current == null) {
current = readNext();
}
return current != null;
}

private T readNext() {
try {
JsonToken jsonToken = parser.nextToken();
if (jsonToken == null) {
return false;
return null;
}

if (jsonToken == JsonToken.START_ARRAY) {
jsonToken = parser.nextToken();
}

if (jsonToken == JsonToken.END_ARRAY) {
current = null;
ensureClosed(this);
return false;
return null;
}

current = objectReader.readValue(parser);
return objectReader.readValue(parser);
} catch (IOException e) {
// Input Stream closed automatically by parser
throw new DecodeException(response.status(), e.getMessage(), response.request(), e);
}
return current != null;
}

@Override
public T next() {
return current;
if (current != null) {
T tmp = current;
current = null;
return tmp;
}
T next = readNext();
if (next == null) {
throw new NoSuchElementException();
}
return next;
}

@Override
Expand Down
39 changes: 33 additions & 6 deletions jackson/src/test/java/feign/jackson/JacksonIteratorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,27 @@
*/
package feign.jackson;

import static feign.Util.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.core.Is.isA;
import com.fasterxml.jackson.databind.ObjectMapper;
import feign.Request;
import feign.Request.HttpMethod;
import feign.Response;
import feign.Util;
import feign.codec.DecodeException;
import feign.jackson.JacksonIteratorDecoder.JacksonIterator;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.NoSuchElementException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import static feign.Util.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hamcrest.core.Is.isA;

public class JacksonIteratorTest {

Expand All @@ -43,6 +45,31 @@ public void shouldDecodePrimitiveArrays() throws IOException {
assertThat(iterator(Integer.class, "[0,1,2,3]")).containsExactly(0, 1, 2, 3);
}

@Test
public void shouldNotSkipElementsOnHasNext() throws IOException {
JacksonIterator<Integer> iterator = iterator(Integer.class, "[0]");
assertThat(iterator.hasNext()).isTrue();
assertThat(iterator.hasNext()).isTrue();
assertThat(iterator.next()).isEqualTo(0);
assertThat(iterator.hasNext()).isFalse();
}

@Test
public void hasNextIsNotMandatory() throws IOException {
JacksonIterator<Integer> iterator = iterator(Integer.class, "[0]");
assertThat(iterator.next()).isEqualTo(0);
assertThat(iterator.hasNext()).isFalse();
}

@Test
public void expectExceptionOnNoElements() throws IOException {
JacksonIterator<Integer> iterator = iterator(Integer.class, "[0]");
assertThat(iterator.next()).isEqualTo(0);
assertThatThrownBy(() -> iterator.next())
.hasMessage(null)
.isInstanceOf(NoSuchElementException.class);
}

@Test
public void shouldDecodeObjects() throws IOException {
assertThat(iterator(User.class, "[{\"login\":\"bob\"},{\"login\":\"joe\"}]"))
Expand Down

0 comments on commit 3c8f568

Please sign in to comment.