Skip to content

Commit 9dcb398

Browse files
chris-melmanchris-melman
committed
fix review
1 parent ba645d1 commit 9dcb398

10 files changed

Lines changed: 74 additions & 46 deletions

File tree

‎dbus-java/src/main/java/org/freedesktop/dbus/messages/EmptyCollectionHelper.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ final class EmptyCollectionHelper {
2424
* @param currentOffset the current offset within the signature
2525
* @return the index of the last element of the collection (subtype)
2626
*/
27-
static int determineSignatureOffsetEmptyCollection(byte[] sigb, int currentOffset) {
27+
static int determineSignatureOffset(byte[] sigb, int currentOffset) {
2828
byte[] restSigbytes = Arrays.copyOfRange(sigb, currentOffset, sigb.length);
2929
String sigSubString = new String(restSigbytes);
3030

@@ -72,10 +72,10 @@ static int determineSignatureOffsetEmptyCollection(byte[] sigb, int currentOffse
7272
}
7373
break;
7474
case ARRAY:
75-
//array is a strange type since it has uses not brackets but the next type is part of the array
76-
// for example aa(ii) means array of arrays of struct of two ints.
77-
return determineSignatureOffsetEmptyCollection(sigb, currentOffset + i + 1);
78-
case PRIMATIVE:
75+
// array is a strange type it has on starting/ending character. It just uses the next full type.
76+
// For example aa(ii) means an array of arrays of struct with two Integer.
77+
return determineSignatureOffset(sigb, currentOffset + i + 1);
78+
case PRIMITIVE:
7979
default:
8080
return currentOffset + i ;
8181
}
@@ -99,7 +99,7 @@ private static ECollectionSubType determineCollectionSubType(char sig) {
9999
return ECollectionSubType.ARRAY;
100100
default:
101101
// of course there can be other types but those shouldn't be allowed in this part of the signature
102-
return ECollectionSubType.PRIMATIVE;
102+
return ECollectionSubType.PRIMITIVE;
103103
}
104104
}
105105

@@ -111,6 +111,6 @@ private enum ECollectionSubType {
111111
STRUCT,
112112
DICT,
113113
ARRAY,
114-
PRIMATIVE
114+
PRIMITIVE
115115
}
116116
}

‎dbus-java/src/main/java/org/freedesktop/dbus/messages/Message.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,7 @@ private int appendone(byte[] sigb, int sigofs, Object data) throws DBusException
662662
diff = appendone(sigb, i, o);
663663
}
664664
if (contents.length == 0) {
665-
diff = EmptyCollectionHelper.determineSignatureOffsetEmptyCollection(sigb, diff);
665+
diff = EmptyCollectionHelper.determineSignatureOffset(sigb, diff);
666666
}
667667
i = diff;
668668
} else if (data instanceof Map) {
@@ -673,7 +673,7 @@ private int appendone(byte[] sigb, int sigofs, Object data) throws DBusException
673673
diff = appendone(sigb, i, o);
674674
}
675675
if (map.size() == 0) {
676-
diff = EmptyCollectionHelper.determineSignatureOffsetEmptyCollection(sigb, diff);
676+
diff = EmptyCollectionHelper.determineSignatureOffset(sigb, diff);
677677
}
678678
i = diff;
679679
} else {
@@ -684,7 +684,7 @@ private int appendone(byte[] sigb, int sigofs, Object data) throws DBusException
684684
diff = appendone(sigb, i, o);
685685
}
686686
if (contents.length == 0) {
687-
diff = EmptyCollectionHelper.determineSignatureOffsetEmptyCollection(sigb, diff);
687+
diff = EmptyCollectionHelper.determineSignatureOffset(sigb, diff);
688688
}
689689
i = diff;
690690
}

‎dbus-java/src/test/java/org/freedesktop/dbus/test/TestAll.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.text.Collator;
2424
import java.util.ArrayList;
2525
import java.util.Arrays;
26-
import java.util.Collections;
2726
import java.util.HashMap;
2827
import java.util.List;
2928
import java.util.Map;
@@ -387,8 +386,8 @@ public void testListOfStruct() throws DBusException {
387386
SampleRemoteInterface tri = (SampleRemoteInterface) clientconn.getPeerRemoteObject("foo.bar.Test", TEST_OBJECT_PATH);
388387

389388
IntStruct elem1 = new IntStruct(3, 7);
390-
IntStruct elem2 = new IntStruct(9, 14);
391-
List<IntStruct> list = Arrays.asList(elem1, elem2);
389+
IntStruct elem2 = new IntStruct(9, 14);
390+
List<IntStruct> list = Arrays.asList(elem1, elem2);
392391
SampleStruct4 param = new SampleStruct4(list);
393392
int[][] out = tri.testListstruct(param);
394393
if (out.length != 2) {
@@ -398,7 +397,6 @@ public void testListOfStruct() throws DBusException {
398397
assertEquals(elem1.getValue2(), out[0][1]);
399398
assertEquals(elem2.getValue1(), out[1][0]);
400399
assertEquals(elem2.getValue2(), out[1][1]);
401-
402400
}
403401

404402
public void testFrob() throws DBusException {

‎dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/ISampleCollectionInterface.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,35 +13,35 @@
1313

1414
import org.freedesktop.dbus.interfaces.DBusInterface;
1515
import org.freedesktop.dbus.test.collections.empty.structs.ArrayStructIntStruct;
16-
import org.freedesktop.dbus.test.collections.empty.structs.ArrayStructPrimative;
16+
import org.freedesktop.dbus.test.collections.empty.structs.ArrayStructPrimitive;
1717
import org.freedesktop.dbus.test.collections.empty.structs.DeepArrayStruct;
1818
import org.freedesktop.dbus.test.collections.empty.structs.DeepListStruct;
1919
import org.freedesktop.dbus.test.collections.empty.structs.DeepMapStruct;
2020
import org.freedesktop.dbus.test.collections.empty.structs.ListMapStruct;
21-
import org.freedesktop.dbus.test.collections.empty.structs.ListStructPrimative;
21+
import org.freedesktop.dbus.test.collections.empty.structs.ListStructPrimitive;
2222
import org.freedesktop.dbus.test.collections.empty.structs.ListStructStruct;
2323
import org.freedesktop.dbus.test.collections.empty.structs.MapArrayStruct;
2424
import org.freedesktop.dbus.test.collections.empty.structs.MapStructIntStruct;
25-
import org.freedesktop.dbus.test.collections.empty.structs.MapStructPrimative;
25+
import org.freedesktop.dbus.test.collections.empty.structs.MapStructPrimitive;
2626

2727
/**
2828
* A sample remote interface which implements same function for all of the structs
2929
*/
3030
public interface ISampleCollectionInterface extends DBusInterface {
3131

32-
String testListPrimative(ListStructPrimative param);
32+
String testListPrimitive(ListStructPrimitive param);
3333

3434
String testListIntStruct(ListStructStruct param);
3535

3636
String testDeepList(DeepListStruct param);
3737

38-
String testArrayPrimative(ArrayStructPrimative param);
38+
String testArrayPrimitive(ArrayStructPrimitive param);
3939

4040
String testArrayIntStruct(ArrayStructIntStruct param);
4141

4242
String testDeepArray(DeepArrayStruct param);
4343

44-
String testMapPrimative(MapStructPrimative param);
44+
String testMapPrimitive(MapStructPrimitive param);
4545

4646
String testMapIntStruct(MapStructIntStruct param);
4747

‎dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/SampleCollectionImpl.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,22 @@
1111
package org.freedesktop.dbus.test.collections.empty;
1212

1313
import org.freedesktop.dbus.test.collections.empty.structs.ArrayStructIntStruct;
14-
import org.freedesktop.dbus.test.collections.empty.structs.ArrayStructPrimative;
14+
import org.freedesktop.dbus.test.collections.empty.structs.ArrayStructPrimitive;
1515
import org.freedesktop.dbus.test.collections.empty.structs.DeepArrayStruct;
1616
import org.freedesktop.dbus.test.collections.empty.structs.DeepListStruct;
1717
import org.freedesktop.dbus.test.collections.empty.structs.DeepMapStruct;
1818
import org.freedesktop.dbus.test.collections.empty.structs.IEmptyCollectionStruct;
1919
import org.freedesktop.dbus.test.collections.empty.structs.ListMapStruct;
20-
import org.freedesktop.dbus.test.collections.empty.structs.ListStructPrimative;
20+
import org.freedesktop.dbus.test.collections.empty.structs.ListStructPrimitive;
2121
import org.freedesktop.dbus.test.collections.empty.structs.ListStructStruct;
2222
import org.freedesktop.dbus.test.collections.empty.structs.MapArrayStruct;
2323
import org.freedesktop.dbus.test.collections.empty.structs.MapStructIntStruct;
24-
import org.freedesktop.dbus.test.collections.empty.structs.MapStructPrimative;
24+
import org.freedesktop.dbus.test.collections.empty.structs.MapStructPrimitive;
2525

2626
public class SampleCollectionImpl implements ISampleCollectionInterface {
2727

2828
@Override
29-
public String testListPrimative(ListStructPrimative param) {
29+
public String testListPrimitive(ListStructPrimitive param) {
3030
return testValue(param);
3131
}
3232

@@ -41,7 +41,7 @@ public String testDeepList(DeepListStruct param) {
4141
}
4242

4343
@Override
44-
public String testArrayPrimative(ArrayStructPrimative param) {
44+
public String testArrayPrimitive(ArrayStructPrimitive param) {
4545
return testValue(param);
4646

4747
}
@@ -57,7 +57,7 @@ public String testDeepArray(DeepArrayStruct param) {
5757
}
5858

5959
@Override
60-
public String testMapPrimative(MapStructPrimative param) {
60+
public String testMapPrimitive(MapStructPrimitive param) {
6161
return testValue(param);
6262
}
6363

‎dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/TestEmptyCollections.java‎

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,17 @@
2626
import org.freedesktop.dbus.exceptions.DBusException;
2727
import org.freedesktop.dbus.exceptions.DBusExecutionException;
2828
import org.freedesktop.dbus.test.collections.empty.structs.ArrayStructIntStruct;
29-
import org.freedesktop.dbus.test.collections.empty.structs.ArrayStructPrimative;
29+
import org.freedesktop.dbus.test.collections.empty.structs.ArrayStructPrimitive;
3030
import org.freedesktop.dbus.test.collections.empty.structs.DeepArrayStruct;
3131
import org.freedesktop.dbus.test.collections.empty.structs.DeepListStruct;
3232
import org.freedesktop.dbus.test.collections.empty.structs.DeepMapStruct;
3333
import org.freedesktop.dbus.test.collections.empty.structs.IEmptyCollectionStruct;
3434
import org.freedesktop.dbus.test.collections.empty.structs.ListMapStruct;
35-
import org.freedesktop.dbus.test.collections.empty.structs.ListStructPrimative;
35+
import org.freedesktop.dbus.test.collections.empty.structs.ListStructPrimitive;
3636
import org.freedesktop.dbus.test.collections.empty.structs.ListStructStruct;
3737
import org.freedesktop.dbus.test.collections.empty.structs.MapArrayStruct;
3838
import org.freedesktop.dbus.test.collections.empty.structs.MapStructIntStruct;
39-
import org.freedesktop.dbus.test.collections.empty.structs.MapStructPrimative;
39+
import org.freedesktop.dbus.test.collections.empty.structs.MapStructPrimitive;
4040
import org.freedesktop.dbus.test.helper.structs.IntStruct;
4141
import org.junit.jupiter.api.AfterEach;
4242
import org.junit.jupiter.api.BeforeEach;
@@ -45,6 +45,27 @@
4545
import org.junit.jupiter.params.provider.Arguments;
4646
import org.junit.jupiter.params.provider.MethodSource;
4747

48+
/**
49+
*
50+
* The test structure is a bit of a complex constructions. However the goal is very simple
51+
*
52+
* The class tests all structs implementing IEmptyCollectionStruct
53+
*
54+
* There are two tests:
55+
*
56+
* Empty test:
57+
*
58+
* The test creates an empty object with emptyFactory function and the testString and sends the object to the {@link ISampleCollectionInterface}.
59+
* This interface returns the {@link IEmptyCollectionStruct#getValidationValue()} that value can be
60+
* used to determine whether (de)serialization of object with an empty collection is executed correctly.
61+
*
62+
* Non Empty test:
63+
*
64+
* The test creates an non empty object with nonEmptyFactory function and the testString and sends the object to the {@link ISampleCollectionInterface}.
65+
* This interface returns the {@link IEmptyCollectionStruct#getStringTestValue()} that value can be
66+
* used to determine whether (de)serialization of non empty collection is executed correctly.
67+
*
68+
*/
4869
class TestEmptyCollections {
4970

5071
private DBusConnection serverconn;
@@ -117,24 +138,32 @@ <T extends IEmptyCollectionStruct<?>> void testNonEmpty(ArgumentObj<T> arguments
117138
assertEquals(validationValue, result);
118139
}
119140

141+
/**
142+
* List of arguments for each scenario:
143+
* 1: Interface function to use for to test this struct
144+
* 2: Factory to create an empty structure
145+
* 3: Factory to create an non empty structure
146+
* 4: Name, which is also used for validating empty collections
147+
* 5: Expected to string value for non empty collection test
148+
*/
120149
static Stream<Arguments> scenarios() {
121150
return Stream.of(
122-
Arguments.of(new ArgumentObj<>(ISampleCollectionInterface::testListPrimative,
123-
s -> new ListStructPrimative(Collections.emptyList(), s),
124-
s -> new ListStructPrimative(Arrays.asList(1, 2), s)), "ListPrimative", "1,2"),
151+
Arguments.of(new ArgumentObj<>(ISampleCollectionInterface::testListPrimitive,
152+
s -> new ListStructPrimitive(Collections.emptyList(), s),
153+
s -> new ListStructPrimitive(Arrays.asList(1, 2), s)), "ListPrimative", "1,2"),
125154
Arguments.of(new ArgumentObj<>(ISampleCollectionInterface::testListIntStruct,
126155
s -> new ListStructStruct(Collections.emptyList(), s),
127156
s -> new ListStructStruct(Arrays.asList(new IntStruct(5, 6)), s)), "ListStruct", "(5,6)"),
128-
Arguments.of(new ArgumentObj<>(ISampleCollectionInterface::testArrayPrimative,
129-
s -> new ArrayStructPrimative(new int[0], s),
130-
s -> new ArrayStructPrimative(new int[] {4,5}, s)), "ArrayPrimative", "4,5"),
157+
Arguments.of(new ArgumentObj<>(ISampleCollectionInterface::testArrayPrimitive,
158+
s -> new ArrayStructPrimitive(new int[0], s),
159+
s -> new ArrayStructPrimitive(new int[] {4,5}, s)), "ArrayPrimative", "4,5"),
131160
Arguments.of(new ArgumentObj<>(ISampleCollectionInterface::testArrayIntStruct,
132161
s -> new ArrayStructIntStruct(new IntStruct[0], s),
133162
s -> new ArrayStructIntStruct(new IntStruct[] { new IntStruct(9, 12)}, s)),
134163
"ArrayIntStruct", "(9,12)"),
135-
Arguments.of(new ArgumentObj<>(ISampleCollectionInterface::testMapPrimative,
136-
s -> new MapStructPrimative(Collections.emptyMap(), s),
137-
s -> new MapStructPrimative(getIntHashMap(), s)), "MapPrimative", "{test:8}"),
164+
Arguments.of(new ArgumentObj<>(ISampleCollectionInterface::testMapPrimitive,
165+
s -> new MapStructPrimitive(Collections.emptyMap(), s),
166+
s -> new MapStructPrimitive(getIntHashMap(), s)), "MapPrimative", "{test:8}"),
138167
Arguments.of(new ArgumentObj<>(ISampleCollectionInterface::testMapIntStruct,
139168
s -> new MapStructIntStruct(Collections.emptyMap(), s),
140169
s -> new MapStructIntStruct(getIntStructHashMap(), s)), "MapIntStruct", "{other:(12,17)}"),
@@ -209,8 +238,6 @@ public ArgumentObj(BiFunction<ISampleCollectionInterface, T, String> function, F
209238
this.function = function;
210239
this.factoryEmpty = factoryEmpty;
211240
this.factoryNonEmpty = factoryNonEmpty;
212-
213241
}
214242
}
215-
216243
}

dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/structs/ArrayStructPrimative.java renamed to dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/structs/ArrayStructPrimitive.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@
1717
import org.freedesktop.dbus.Struct;
1818
import org.freedesktop.dbus.annotations.Position;
1919

20-
public final class ArrayStructPrimative extends Struct implements IEmptyCollectionStruct<int[]> {
20+
public final class ArrayStructPrimitive extends Struct implements IEmptyCollectionStruct<int[]> {
2121

2222
@Position(0)
2323
private final int[] list;
2424

2525
@Position(1)
2626
private final String validationValue;
2727

28-
public ArrayStructPrimative(int[] list, String validationValue) {
28+
public ArrayStructPrimitive(int[] list, String validationValue) {
2929
this.list = list.clone();
3030
this.validationValue = validationValue;
3131
}

‎dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/structs/IEmptyCollectionStruct.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package org.freedesktop.dbus.test.collections.empty.structs;
22

3+
/**
4+
* Interface is used to make the TestEmptyCollection usable for parameterized testing
5+
*/
36
public interface IEmptyCollectionStruct<T> {
47

58
/**

dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/structs/ListStructPrimative.java renamed to dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/structs/ListStructPrimitive.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@
1717
import org.freedesktop.dbus.Struct;
1818
import org.freedesktop.dbus.annotations.Position;
1919

20-
public final class ListStructPrimative extends Struct implements IEmptyCollectionStruct<List<Integer>> {
20+
public final class ListStructPrimitive extends Struct implements IEmptyCollectionStruct<List<Integer>> {
2121

2222
@Position(0)
2323
private final List<Integer> list;
2424

2525
@Position(1)
2626
private final String validationValue;
2727

28-
public ListStructPrimative(List<Integer> list, String validationValue) {
28+
public ListStructPrimitive(List<Integer> list, String validationValue) {
2929
this.list = list;
3030
this.validationValue = validationValue;
3131
}

dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/structs/MapStructPrimative.java renamed to dbus-java/src/test/java/org/freedesktop/dbus/test/collections/empty/structs/MapStructPrimitive.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,15 @@
1818
import org.freedesktop.dbus.annotations.Position;
1919

2020

21-
public final class MapStructPrimative extends Struct implements IEmptyCollectionStruct<Map<String, Integer>> {
21+
public final class MapStructPrimitive extends Struct implements IEmptyCollectionStruct<Map<String, Integer>> {
2222

2323
@Position(0)
2424
private final Map<String, Integer> map;
2525

2626
@Position(1)
2727
private final String validationValue;
2828

29-
public MapStructPrimative(Map<String, Integer> map, String validationValue) {
29+
public MapStructPrimitive(Map<String, Integer> map, String validationValue) {
3030
this.map = map;
3131
this.validationValue = validationValue;
3232
}

0 commit comments

Comments
 (0)