Skip to content
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.

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

Resolves issue 323 #324

Open
wants to merge 3 commits into
base: JDK-8
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>2.3.2</version>
<configuration>
<source>1.7</source>
<target>1.7</target>
<source>1.8</source>
<target>1.8</target>
<!--<encoding>UTF-8</encoding>-->
<debug>true</debug>
<debuglevel>lines,vars,source</debuglevel> <!-- required to make structs work -->
Expand Down Expand Up @@ -78,7 +78,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<configuration>
<additionalparam>-Xdoclint:none</additionalparam>
Copy link
Author

Choose a reason for hiding this comment

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

I know Gradle a lot better than Maven. I'm happy to remove these changes if you don't want them.

<doclint>none</doclint>
</configuration>
<executions>
<execution>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ public void clear() {
mKeys = new int[size];
mValues = new Object[size];
mNumberOfElements = 0;
next = null;
Copy link
Author

Choose a reason for hiding this comment

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

This is the most important line that is needed and which fixes the issue.

} else {
FSTUtil.clear(mKeys);
FSTUtil.clear(mValues);
Expand Down
119 changes: 119 additions & 0 deletions src/test/deserialization_cache_bug/DeserializationCacheTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package deserialization_cache_bug;

import static org.junit.Assert.fail;

import java.util.ArrayList;
import java.util.List;

import org.junit.Test;
import org.nustaq.serialization.FSTConfiguration;

import deserialization_cache_bug.model.Call;
import deserialization_cache_bug.model.CallEvent;
import deserialization_cache_bug.model.CallRecording;
import deserialization_cache_bug.model.CallV2;
import deserialization_cache_bug.model.Note;
import deserialization_cache_bug.model.RecordingStatus;
import deserialization_cache_bug.model.Timeframe;
import deserialization_cache_bug.random.DeterministicRandomGenerator;
import deserialization_cache_bug.random.RandomGenerator;

public class DeserializationCacheTest {
Copy link
Author

Choose a reason for hiding this comment

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

This test is pretty complex, but I had a really hard time reproducing the issue and tried to mimic some of the things about our complex model that might make it more likely to happen.

private RandomGenerator random= new DeterministicRandomGenerator();

@Test
public void testDeserializationCacheIssue() {
int testCount = 30;
int mutationCountPerTest = 15;
int callCountPerTest = 100;

FSTConfiguration serializer = FSTConfiguration.createDefaultConfiguration();

for(int i = 0; i < testCount; i++) {
try {
testSerializationAndMutationOfTestCalls(i, mutationCountPerTest, callCountPerTest, serializer);
} catch (Exception e) {
e.printStackTrace();
fail();
}
}
}

private void testSerializationAndMutationOfTestCalls(int testIndex, int mutationCountPerTest, int callCountPerTest, FSTConfiguration serializer) throws Exception {
ArrayList<byte[]> currentCallsAsBytes = generateTestCallsAsBytes(callCountPerTest, serializer);
ArrayList<byte[]> mutatedCallsAsBytes = new ArrayList<>();

for(int i = 0; i < mutationCountPerTest; i++) {
for(int j = 0; j < currentCallsAsBytes.size(); j++) {
System.out.println(testIndex + "-" + i + "-" + j);
Call originalCall = (Call) serializer.asObject(currentCallsAsBytes.get(j));

ObjectValidation.validateFieldValueTypesForObject(originalCall);

mutateCall(originalCall);

byte[] mutatedBytes = serializer.asByteArray(originalCall);

mutatedCallsAsBytes.add(mutatedBytes);
}

currentCallsAsBytes = mutatedCallsAsBytes;
mutatedCallsAsBytes = new ArrayList<>();
}
}

private ArrayList<byte[]> generateTestCallsAsBytes(int callCountPerTest, FSTConfiguration serializer) throws Exception {
ArrayList<byte[]> testCallsAsBytes = new ArrayList<>();
for(int i = 0; i < callCountPerTest; i++) {
Call call = CallV2.generateBasicTestCall(random);
testCallsAsBytes.add(serializer.asByteArray(call));
}
return testCallsAsBytes;
}

private void mutateCall(Call call) {
switch(random.nextInt(4)) {
case 0:
addNote(call);
break;
case 1:
addRecording(call);
break;
case 2:
addAccountCode(call);
break;
case 3:
setRecordingAsSaved(call);
break;
}
}

private void addNote(Call call) {
Note note = new Note(random.nextUUID(), call.getStart(), "Derek Johnson(210)", "This is a rad note!", System.currentTimeMillis());
call.addNote(note);
}

private void addRecording(Call call) {
CallEvent firstEvent = call.getEvents().get(0);

List<Timeframe> times = new ArrayList<>();
times.add(new Timeframe(firstEvent.getStart(), firstEvent.getEnd()));

CallRecording recording = new CallRecording(random.nextUUID(), call.getId(), firstEvent.getEventId(), "Stream " + random.nextInt(), RecordingStatus.PENDING, "SIP-ID" + random.nextInt(), call.getStart(), call.getEnd() - call.getStart(), random.nextUUID(), 1_000_000, times);
call.addRecording(firstEvent.getEventId(), recording);
}

private void addAccountCode(Call call) {
List<String> accountCodes = call.getAccountCodes();
if(accountCodes == null) {
accountCodes = new ArrayList<>();
}
accountCodes.add("My new account code" + random.nextInt(1000));
call.setAccountCodes(accountCodes);
}

private void setRecordingAsSaved(Call call) {
long eventId = call.getEvents().get(0).getEventId();
call.setRecordingsOnEventAsSaved(eventId);
}
}
156 changes: 156 additions & 0 deletions src/test/deserialization_cache_bug/ObjectValidation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package deserialization_cache_bug;

import java.lang.reflect.Array;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.Collection;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;

public class ObjectValidation {

protected ObjectValidation() {}

public static void validateFieldValueTypesForObject(Object obj) throws Exception {
try {
validateFieldValueTypesForObject(obj, Collections.newSetFromMap(new IdentityHashMap<>()));
} catch (Throwable t) {
throw new Exception("Invalid Object Identified", t);
}
}

private static void validateFieldValueTypesForObject(Object obj, Set<Object> previouslyValidatedObjects) throws IllegalArgumentException, IllegalAccessException, Exception {
if(obj == null) {
return;
}

previouslyValidatedObjects.add(obj);

if(obj.getClass().isArray()) {
handleArray(obj, previouslyValidatedObjects);
} else if(obj instanceof Iterable) {
handleIterable((Iterable<?>)obj, previouslyValidatedObjects);
} else if(obj instanceof Map<?, ?>) {
handleMap((Map<?, ?>)obj, previouslyValidatedObjects);
} else {
handleObject(obj, previouslyValidatedObjects, true);
}
}

private static void handleArray(Object objArray, Set<Object> previouslyValidatedObjects) throws IllegalArgumentException, IllegalAccessException, Exception {
for(int i = 0; i < Array.getLength(objArray); i++) {
Object obj = Array.get(objArray, i);
if(obj != null && !previouslyValidatedObjects.contains(obj)) {
validateFieldValueTypesForObject(obj, previouslyValidatedObjects);
}
}
}

private static void handleIterable(Iterable<?> iterable, Set<Object> previouslyValidatedObjects) throws IllegalArgumentException, IllegalAccessException, Exception {
Iterator<?> it = iterable.iterator();
while(it.hasNext()) {
Object obj = it.next();
if(obj != null && !previouslyValidatedObjects.contains(obj)) {
validateFieldValueTypesForObject(obj, previouslyValidatedObjects);
}
}
handleObject(iterable, previouslyValidatedObjects, false); //Make sure that primitives are valid
}

private static void handleMap(Map<?,?> map, Set<Object> previouslyValidatedObjects) throws IllegalArgumentException, IllegalAccessException, Exception {
Iterator<?> it = map.entrySet().iterator();
while(it.hasNext()) {
Entry<?, ?> entry = (Entry<?, ?>) it.next();
if(entry != null) {
if(entry.getKey() != null && !previouslyValidatedObjects.contains(entry.getKey())) {
validateFieldValueTypesForObject(entry.getKey(), previouslyValidatedObjects);
}
if(entry.getValue() != null && !previouslyValidatedObjects.contains(entry.getValue())) {
validateFieldValueTypesForObject(entry.getValue(), previouslyValidatedObjects);
}
}
}
handleObject(map, previouslyValidatedObjects, false); //Make sure that primitives are valid
}

private static void handleObject(Object obj, Set<Object> previouslyValidatedObjects, boolean delveIntoObjects) throws IllegalArgumentException, IllegalAccessException, Exception {
Class<?> currentClass = obj.getClass();
while(currentClass != Object.class && currentClass != null) {
Field[] fields = currentClass.getDeclaredFields();
if(fields != null) {
for(Field field : fields) {
if(!Modifier.isStatic(field.getModifiers())) {
field.setAccessible(true);
Object fieldValue = field.get(obj);
if(fieldValue != null) {
validateFieldValueType(field, fieldValue);

if(delveIntoObjects && shouldDelveIntoObject(field, fieldValue, previouslyValidatedObjects)) {
validateFieldValueTypesForObject(fieldValue, previouslyValidatedObjects);
}
}
}
}
}
currentClass = currentClass.getSuperclass();
}
}

protected static void validateFieldValueType(Field field, Object value) throws Exception {
Class<?> fieldType = field.getType();
Class<? extends Object> valueType = value.getClass();

if(fieldType.equals(boolean.class) && valueType.equals(Boolean.class)) {
return;
}
if(fieldType.equals(int.class) && valueType.equals(Integer.class)) {
return;
}
if(fieldType.equals(long.class) && valueType.equals(Long.class)) {
return;
}
if(fieldType.equals(float.class) && valueType.equals(Float.class)) {
return;
}
if(fieldType.equals(double.class) && valueType.equals(Double.class)) {
return;
}
if(fieldType.equals(byte.class) && valueType.equals(Byte.class)) {
return;
}
if(fieldType.equals(char.class) && valueType.equals(Character.class)) {
return;
}
if(fieldType.equals(short.class) && valueType.equals(Short.class)) {
return;
}
if(!fieldType.isAssignableFrom(valueType)) {
throw new Exception("Field " + field + " cannot hold a value of type " + valueType);
}
}

private static boolean shouldDelveIntoObject(Field field, Object fieldValue, Set<Object> previouslyValidatedObjects) {
return
!field.getType().isPrimitive() &&
field.getType() != String.class &&
!field.getType().isEnum() &&
isNotEmptyCollection(fieldValue) &&
!isByteArray(field) &&
!previouslyValidatedObjects.contains(fieldValue);
}

private static boolean isNotEmptyCollection(Object obj) {
if(obj instanceof Collection<?>) {
return !((Collection<?>)obj).isEmpty();
}
return true;
}

private static boolean isByteArray(Field field) {
return field.getType().equals(byte[].class) || field.getType().equals(Byte[].class);
}
}
Loading