Skip to content

[#120179893] Make sure compression and gzip header go together #27

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
ericlemes wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[#120179893] Make sure compression and gzip header go together #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4d217f2
[#120179893] Make sure compression and gzip header go together
ericlemes Nov 9, 2016
12b0568
[#120179893] Merge from master
ericlemes Nov 9, 2016
ad502fe
[#120179893] Adding tests and checking if gzip header is present befo…
ericlemes Nov 10, 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
117 changes: 102 additions & 15 deletions reyna-test/src/com/b2msolutions/reyna/DispatcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@
import android.content.Context;
import android.content.Intent;
import android.content.SharedPreferences;
import android.icu.util.Output;
import android.net.ConnectivityManager;
import android.net.NetworkInfo;
import com.b2msolutions.reyna.Dispatcher.Result;
import com.b2msolutions.reyna.blackout.Time;
import com.b2msolutions.reyna.blackout.TimeRange;
import com.b2msolutions.reyna.http.HttpPost;
import com.b2msolutions.reyna.http.OutputStreamFactory;
import com.b2msolutions.reyna.shadows.ShadowAndroidHttpClient;
import com.b2msolutions.reyna.system.Header;
import com.b2msolutions.reyna.system.Clock;
import com.b2msolutions.reyna.system.Message;
import com.b2msolutions.reyna.system.Preferences;
Expand All @@ -25,8 +28,11 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Matchers;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config;
Expand All @@ -47,7 +53,9 @@
import java.util.GregorianCalendar;
import java.util.zip.GZIPOutputStream;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertNull;
import static org.junit.Assert.*;
import static org.mockito.Mockito.*;
import static org.robolectric.Shadows.shadowOf;
Expand All @@ -60,6 +68,7 @@ public class DispatcherTest {
private Intent batteryStatus;
@Mock NetworkInfo networkInfo;
@Mock Date now;
@Mock OutputStreamFactory outputStreamFactory;

@Before
public void setup() {
Expand Down Expand Up @@ -87,7 +96,7 @@ public void sendMessageHappyPathShouldSetExecuteCorrectHttpPostAndReturnOK() thr

Time time = mock(Time.class);

assertEquals(Result.OK, new Dispatcher().sendMessage(message, httpPost, httpClient, this.context));
assertEquals(Result.OK, new Dispatcher(this.outputStreamFactory).sendMessage(message, httpPost, httpClient, this.context));

this.verifyHttpPost(message, httpPost);

Expand All @@ -112,7 +121,7 @@ public void sendMessageHappyPathWithChineseCharactersShouldSetExecuteCorrectHttp
when(httpClient.execute(httpPost)).thenReturn(httpResponse);

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher();
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
dispatcher.clock = clock;

assertEquals(Result.OK, dispatcher.sendMessage(message, httpPost, httpClient, this.context));
Expand Down Expand Up @@ -140,7 +149,7 @@ public void sendMessageHappyPathWithPortShouldSetPort() throws URISyntaxExceptio
when(httpClient.execute(httpPost)).thenReturn(httpResponse);

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher();
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
dispatcher.clock = clock;

assertEquals(Result.OK, dispatcher.sendMessage(message, httpPost, httpClient, this.context));
Expand All @@ -162,7 +171,7 @@ public void sendMessageShouldReturnBlackoutWhenInBlackout() {
new Preferences(this.context).saveCellularDataBlackout(range);

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher();
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
dispatcher.clock = clock;

assertEquals(Result.BLACKOUT, dispatcher.sendMessage(null, null, null, this.context));
Expand All @@ -173,7 +182,7 @@ public void sendMessageShouldReturnNotConnectedWhenNotConnected() {
when(this.networkInfo.isConnectedOrConnecting()).thenReturn(false);

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher();
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
dispatcher.clock = clock;

assertEquals(Result.NOTCONNECTED, dispatcher.sendMessage(null, null, null, this.context));
Expand All @@ -188,7 +197,7 @@ public void whenExecuteThrowsReturnTemporaryError() throws URISyntaxException, I
when(httpClient.execute(httpPost)).thenThrow(new RuntimeException(""));

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher();
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
dispatcher.clock = clock;

assertEquals(Result.TEMPORARY_ERROR, dispatcher.sendMessage(message, httpPost, httpClient, this.context));
Expand All @@ -209,6 +218,7 @@ public void getResultShouldReturnExpected() {
@Test
public void sendMessageWithGzipAndContentIsLessThanMinGzipLengthShouldRemoveGzipHeaderAndSendMessageAsString() throws Exception {
Message message = RepositoryTest.getMessageWithGzipHeaders("body");
message.addHeader(new Header("Content-Encoding", " gzip "));

StatusLine statusLine = mock(StatusLine.class);
when(statusLine.getStatusCode()).thenReturn(200);
Expand All @@ -221,24 +231,24 @@ public void sendMessageWithGzipAndContentIsLessThanMinGzipLengthShouldRemoveGzip
when(httpClient.execute(httpPost)).thenReturn(httpResponse);

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher();
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
dispatcher.clock = clock;

assertEquals(Result.OK, dispatcher.sendMessage(message, httpPost, httpClient, this.context));

this.verifyHttpPost(message, httpPost);

ArgumentCaptor<ByteArrayEntity> byteArrayEntityCaptor = ArgumentCaptor.forClass(ByteArrayEntity.class);
verify(httpPost).setEntity(byteArrayEntityCaptor.capture());
ByteArrayEntity entity = byteArrayEntityCaptor.getValue();
ArgumentCaptor<StringEntity> stringEntityArgumentCaptor = ArgumentCaptor.forClass(StringEntity.class);
verify(httpPost).setEntity(stringEntityArgumentCaptor.capture());
AbstractHttpEntity entity = stringEntityArgumentCaptor.getValue();
assertNull(entity.getContentEncoding());
assertEquals(EntityUtils.toString(entity, "utf-8"), "body");
}

@Test
public void sendMessageWithGzipHeaderShouldCompressContentAndReturnOK() throws URISyntaxException, IOException, KeyManagementException, UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException, CertificateException {
Message message = RepositoryTest.getMessageWithGzipHeaders("this any message body more than 10 bytes length");
byte[] data = "this any message body more than 10 bytes length".getBytes("utf-8");
public void sendMessageWithGzipHeaderAndMoreThan20BytesAndStreamCompressDataShouldSetCorrectContentEncoding() throws URISyntaxException, IOException, KeyManagementException, UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException, CertificateException {
Message message = RepositoryTest.getMessageWithGzipHeaders("this any message body more than 20 bytes length");
byte[] data = "this any message body more than 20 bytes length".getBytes("utf-8");

StatusLine statusLine = mock(StatusLine.class);
when(statusLine.getStatusCode()).thenReturn(200);
Expand All @@ -249,8 +259,15 @@ public void sendMessageWithGzipHeaderShouldCompressContentAndReturnOK() throws U
HttpClient httpClient = mock(HttpClient.class);
when(httpClient.execute(httpPost)).thenReturn(httpResponse);

ArgumentCaptor<OutputStream> outputStreamCaptor = ArgumentCaptor.forClass(OutputStream.class);
doAnswer(new Answer(){
public Object answer(InvocationOnMock invocation) throws IOException {
OutputStream os = (OutputStream)invocation.getArguments()[0];
return new GZIPOutputStream(os);
}}).when(this.outputStreamFactory).createGzipOutputStream(any(OutputStream.class));

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher();
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
dispatcher.clock = clock;

Result actual = dispatcher.sendMessage(message, httpPost, httpClient, this.context);
Expand All @@ -268,6 +285,76 @@ public void sendMessageWithGzipHeaderShouldCompressContentAndReturnOK() throws U
assertArrayEquals(EntityUtils.toByteArray(byteArrayEntity), expected);
}

@Test
public void sendMessageWithGzipHeaderAndMoreThan20BytesAndStreamDoNotCompressDataShouldSetCorrectContentEncoding() throws URISyntaxException, IOException, KeyManagementException, UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException, CertificateException {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 100; i++){
sb.append("This should be a big message body. ");
}

Message message = RepositoryTest.getMessageWithGzipHeaders(sb.toString());
byte[] data = sb.toString().getBytes("UTF-8");

StatusLine statusLine = mock(StatusLine.class);
when(statusLine.getStatusCode()).thenReturn(200);
HttpResponse httpResponse = mock(HttpResponse.class);
when(httpResponse.getStatusLine()).thenReturn(statusLine);

HttpPost httpPost = mock(HttpPost.class);
HttpClient httpClient = mock(HttpClient.class);
when(httpClient.execute(httpPost)).thenReturn(httpResponse);

when(this.outputStreamFactory.createGzipOutputStream(any(OutputStream.class))).thenReturn(new ByteArrayOutputStream());

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
dispatcher.clock = clock;

Result actual = dispatcher.sendMessage(message, httpPost, httpClient, this.context);

assertEquals(Result.OK, actual);

this.verifyHttpPost(message, httpPost);

ArgumentCaptor<ByteArrayEntity> entityCaptor = ArgumentCaptor.forClass(ByteArrayEntity.class);
verify(httpPost).setEntity(entityCaptor.capture());
AbstractHttpEntity byteArrayEntity = entityCaptor.getValue();
assertEquals(byteArrayEntity.getContentEncoding().getValue(), "");

assertArrayEquals(EntityUtils.toByteArray(byteArrayEntity), data);
}

@Test
public void sendMessageWithoutGzipHeaderAndBigContentShouldNotCompressAndReturnOk() throws URISyntaxException, IOException, KeyManagementException, UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException, CertificateException {
Message message = RepositoryTest.getMessageWithHeaders("this any message body more than 20 bytes length. Really. Should be really bigger then 20.");

StatusLine statusLine = mock(StatusLine.class);
when(statusLine.getStatusCode()).thenReturn(200);
HttpResponse httpResponse = mock(HttpResponse.class);
when(httpResponse.getStatusLine()).thenReturn(statusLine);

HttpPost httpPost = mock(HttpPost.class);
HttpClient httpClient = mock(HttpClient.class);
when(httpClient.execute(httpPost)).thenReturn(httpResponse);

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
dispatcher.clock = clock;

Result actual = dispatcher.sendMessage(message, httpPost, httpClient, this.context);

assertEquals(Result.OK, actual);

this.verifyHttpPost(message, httpPost);

ArgumentCaptor<ByteArrayEntity> entityCaptor = ArgumentCaptor.forClass(ByteArrayEntity.class);
verify(httpPost).setEntity(entityCaptor.capture());
AbstractHttpEntity entity = entityCaptor.getValue();
assertNull(entity.getContentEncoding());
String data = EntityUtils.toString(entity);
assertEquals(message.getBody(), data);
}

@Test
public void canSendShouldReturnNOTCONNECTEDIfNoActiveNetwork() {
ConnectivityManager connectivityManager = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
Expand Down Expand Up @@ -795,7 +882,7 @@ public void whenCallingSendMessageShouldAddSubmittedTimestamp() throws IOExcepti
when(httpClient.execute(httpPost)).thenReturn(httpResponse);

Clock clock = mock(Clock.class);
Dispatcher dispatcher = new Dispatcher();
Dispatcher dispatcher = new Dispatcher(this.outputStreamFactory);
when(clock.getCurrentTimeMillis()).thenReturn(42L);
dispatcher.clock = clock;

Expand Down
29 changes: 29 additions & 0 deletions reyna-test/src/com/b2msolutions/reyna/http/OutputStreamFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.b2msolutions.reyna.http;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.util.zip.GZIPOutputStream;

import static junit.framework.Assert.*;

@RunWith(RobolectricTestRunner.class)
public class OutputStreamFactoryTest {

@Test
public void whenConstructingShouldNotThrow(){
assertNotNull(new OutputStreamFactory());
}

@Test
public void whenCreateGzipOutputStreamShouldReturnRightType() throws IOException {
OutputStreamFactory factory = new OutputStreamFactory();
OutputStream outputStream = factory.createGzipOutputStream(new ByteArrayOutputStream());
assertTrue(outputStream instanceof GZIPOutputStream);
}

}
66 changes: 50 additions & 16 deletions reyna/src/com/b2msolutions/reyna/Dispatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,36 @@
import com.b2msolutions.reyna.blackout.TimeRange;
import com.b2msolutions.reyna.http.HttpPost;
import com.b2msolutions.reyna.blackout.BlackoutTime;
import com.b2msolutions.reyna.http.OutputStreamFactory;
import com.b2msolutions.reyna.system.*;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
import org.apache.http.entity.AbstractHttpEntity;
import org.apache.http.entity.ByteArrayEntity;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.AbstractHttpClient;
import org.apache.http.protocol.HTTP;

import java.io.ByteArrayOutputStream;
import java.io.OutputStream;
import java.net.URI;
import java.text.ParseException;
import java.util.ArrayList;
import java.util.GregorianCalendar;
import java.util.Locale;
import java.util.zip.GZIPOutputStream;

public class Dispatcher {

private static final String TAG = "com.b2msolutions.reyna.Dispatcher";

private OutputStreamFactory outputStreamFactory;

protected Clock clock;

public Dispatcher() {
public Dispatcher(OutputStreamFactory outputStreamFactory) {
Copy link
Contributor

@CharlesGarth CharlesGarth Nov 10, 2016

Choose a reason for hiding this comment

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

can you leave the other constructor in as well? I think we have a few usages of Dispatcher() without args in Elemez and I can't be bothered to update them :D.

Copy link
Contributor

@CharlesGarth CharlesGarth Nov 10, 2016

Choose a reason for hiding this comment

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

Or you could follow the dependency "injection" anti-pattern we have in some of the other classes where we instantiate the dep in constructor and inject mock to the protected member... :P

this.clock = new Clock();
this.outputStreamFactory = outputStreamFactory;
}

public enum Result {
Expand Down Expand Up @@ -179,7 +188,7 @@ private Result parseHttpPost(Message message, HttpPost httpPost, Context context
URI uri = message.getURI();
httpPost.setURI(uri);

AbstractHttpEntity entity = Dispatcher.getEntity(message, context);
AbstractHttpEntity entity = this.getEntity(message, context);
httpPost.setEntity(entity);

this.setHeaders(httpPost, message.getHeaders());
Expand Down Expand Up @@ -232,8 +241,7 @@ private static Header[] removeGzipEncodingHeader(Header[] headers) {
ArrayList<Header> filteredHeaders = new ArrayList<Header>();

for (Header header : headers) {
if (header.getKey().equalsIgnoreCase("content-encoding")
&& header.getValue().equalsIgnoreCase("gzip")) {
if (header.getKey().equalsIgnoreCase("content-encoding")) {
Copy link
Contributor

@CharlesGarth CharlesGarth Nov 9, 2016

Choose a reason for hiding this comment

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

this shouldGzip() method isn't being used anymore and can be removed

Copy link
Author

@ericlemes ericlemes Nov 10, 2016

Choose a reason for hiding this comment

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

This is a very good spot. Thanks. Actually shouldGzip should be used. If we don't receive the header from elemez (or any app using reyna lib) we should not compress at all. Fixing it.

continue;
}

Expand All @@ -244,9 +252,44 @@ private static Header[] removeGzipEncodingHeader(Header[] headers) {
return filteredHeaders.toArray(returnedHeaders);
}

private static AbstractHttpEntity getCompressedEntity(String content, Context context) throws Exception {
byte[] data = content.getBytes();
return AndroidHttpClient.getCompressedEntity(data, context.getContentResolver());
private AbstractHttpEntity getEntity(Message message, Context context) throws Exception {
String content = message.getBody();

byte[] data = content.getBytes("UTF-8");
if (!shouldGzip(message.getHeaders()) || data.length <= (AndroidHttpClient.getMinGzipSize(context.getContentResolver()) * 2)){
return new StringEntity(content, HTTP.UTF_8);
}
else {
return getCompressedEntity(data, context);
}
}

private AbstractHttpEntity getCompressedEntity(byte[] data, Context context) throws Exception {
ByteArrayOutputStream arr = new ByteArrayOutputStream();
OutputStream zipper = this.outputStreamFactory.createGzipOutputStream(arr);
zipper.write(data);
zipper.close();

byte[] compressedData = arr.toByteArray();
int end = compressedData.length > 500 ? 500 : compressedData.length;

Boolean equal = true;
for (int i = 0; i < end; i++){
if (compressedData[i] != data[i]){
equal = false;
break;
}
}
AbstractHttpEntity entity;
if (!equal){
entity = new ByteArrayEntity(compressedData);
entity.setContentEncoding("gzip");
}
else {
entity = new ByteArrayEntity(data);
Copy link
Contributor

@CharlesGarth CharlesGarth Nov 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

i think this is maybe missing a unit test case

entity.setContentEncoding("");
}
return entity;
}

private void setHeaders(HttpPost httpPost, Header[] headers) {
Expand All @@ -263,16 +306,7 @@ private String getSubmittedTimestamp() {
return String.valueOf(this.clock.getCurrentTimeMillis());
}

private static AbstractHttpEntity getEntity(Message message, Context context) throws Exception {
String content = message.getBody();
AbstractHttpEntity entity = new StringEntity(content, HTTP.UTF_8);

if (Dispatcher.shouldGzip(message.getHeaders())) {
entity = getCompressedEntity(content, context);
}

return entity;
}

public static boolean isBatteryCharging(Context context) {
Intent batteryStatus = context.registerReceiver(null, new IntentFilter(Intent.ACTION_BATTERY_CHANGED));
Expand Down
Loading