Skip to content

Commit c0f1293

Browse files
anthony-swirldslabsanthony-swirldslabs
authored
feat: make parsed lists read-only (#682)
Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
1 parent 79e8707 commit c0f1293

File tree

5 files changed

+240
-6
lines changed

5 files changed

+240
-6
lines changed

‎pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,10 +1171,11 @@ public static final class Builder {
11711171
// NOTE: for performance reasons, repeated fields are initialized with unmodifiable lists. Similarly,
11721172
// Builder.repeatedField(Type... values) methods would wrap the values into unmodifiable lists.
11731173
// If an application needs to dynamically modify repeated fields in builders, then it must first call
1174-
// the Builder.repeatedField(List<Type> list) method and supply a modifiable list, such as an ArrayList
1175-
// instance. After that, the application must only use the getter method to access the list.
1176-
// If the application wants to finalize the list after the building is finished, then it may call
1177-
// Builder.repeatedField(builder.repeatedField().toArray(new Type[0])) as one last final step.
1174+
// the Builder.repeatedField(List<Type> list) method and supply a modifiable list, such as
1175+
// an com.hedera.pbj.runtime.UnmodifiableArrayList instance.
1176+
// After that, the application must only use the getter method to access the list.
1177+
// To finalize the list once its building is finished, simply call
1178+
// ((UnmodifiableArrayList) builder.repeatedField()).makeReadOnly().
11781179
11791180
$getterMethods}"""
11801181
.replace("$fields", fields.stream().map(field ->

‎pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecParseMethodGenerator.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ static String generateParseMethod(
8888
List<UnknownField> $unknownFields = null;
8989
9090
$parseLoop
91+
$listFieldsWriteProtection
9192
if ($unknownFields != null) {
9293
Collections.sort($unknownFields);
9394
$initialSizeOfUnknownFieldsArray = Math.max($initialSizeOfUnknownFieldsArray, $unknownFields.size());
@@ -109,6 +110,11 @@ static String generateParseMethod(
109110
+ (fields.isEmpty() ? "" : ", ") + "$unknownFields"
110111
)
111112
.replace("$parseLoop", generateParseLoop(generateCaseStatements(fields, schemaClassName), "", schemaClassName))
113+
.replace("$listFieldsWriteProtection", fields.stream()
114+
.filter(Field::repeated)
115+
.map(field -> "if (temp_" + field.name() + " instanceof UnmodifiableArrayList ual) ual.makeReadOnly();")
116+
.collect(Collectors.joining("\n"))
117+
.indent(DEFAULT_INDENT * 2))
112118
.indent(DEFAULT_INDENT);
113119
// spotless:on
114120
}

‎pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/ProtoParserTools.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import java.nio.charset.CharacterCodingException;
1313
import java.nio.charset.CodingErrorAction;
1414
import java.nio.charset.StandardCharsets;
15-
import java.util.ArrayList;
1615
import java.util.Collections;
1716
import java.util.HashMap;
1817
import java.util.List;
@@ -47,7 +46,7 @@ private ProtoParserTools() {}
4746
*/
4847
public static <T> List<T> addToList(List<T> list, T newItem) {
4948
if (list == Collections.EMPTY_LIST) {
50-
list = new ArrayList<>();
49+
list = new UnmodifiableArrayList<>();
5150
}
5251
list.add(newItem);
5352
return list;

‎pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/UnmodifiableArrayList.java‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
package com.hedera.pbj.runtime;
3+
4+
import java.util.ArrayList;
5+
import java.util.Collection;
6+
import java.util.Comparator;
7+
import java.util.ListIterator;
8+
import java.util.function.Consumer;
9+
import java.util.function.Predicate;
10+
import java.util.function.UnaryOperator;
11+
12+
/**
13+
* An extension of the `java.util.ArrayList` that can be write-protected by calling the `makeReadOnly()` method.
14+
* This is designed to be equivalent to using `Collections.unmodifiableList()`, but it's more memory efficient
15+
* because this class doesn't require an extra wrapper object.
16+
* @param <E> the type of elements
17+
*/
18+
public class UnmodifiableArrayList<E> extends ArrayList<E> {
19+
private boolean isReadOnly = false;
20+
21+
/**
22+
* Mark this list as read-only.
23+
* After this call, any mutating methods will throw the `UnsupportedOperationException`.
24+
*/
25+
public void makeReadOnly() {
26+
isReadOnly = true;
27+
}
28+
29+
private void checkReadOnly() {
30+
if (isReadOnly) {
31+
// Note that technically, this should be an IllegalStateException.
32+
// However, we throw UnsupportedOperationException for consistency with JDK unmodifiable collections.
33+
throw new UnsupportedOperationException("This list is read-only");
34+
}
35+
}
36+
37+
@Override
38+
public E set(int index, E element) {
39+
checkReadOnly();
40+
return super.set(index, element);
41+
}
42+
43+
@Override
44+
public boolean add(E o) {
45+
checkReadOnly();
46+
return super.add(o);
47+
}
48+
49+
@Override
50+
public void add(int index, E element) {
51+
checkReadOnly();
52+
super.add(index, element);
53+
}
54+
55+
@Override
56+
public void addFirst(E element) {
57+
checkReadOnly();
58+
super.addFirst(element);
59+
}
60+
61+
@Override
62+
public void addLast(E element) {
63+
checkReadOnly();
64+
super.addLast(element);
65+
}
66+
67+
@Override
68+
public E remove(int index) {
69+
checkReadOnly();
70+
return super.remove(index);
71+
}
72+
73+
@Override
74+
public E removeFirst() {
75+
checkReadOnly();
76+
return super.removeFirst();
77+
}
78+
79+
@Override
80+
public E removeLast() {
81+
checkReadOnly();
82+
return super.removeLast();
83+
}
84+
85+
@Override
86+
public boolean remove(Object o) {
87+
checkReadOnly();
88+
return super.remove(o);
89+
}
90+
91+
@Override
92+
public void clear() {
93+
checkReadOnly();
94+
super.clear();
95+
}
96+
97+
@Override
98+
public boolean addAll(Collection c) {
99+
checkReadOnly();
100+
return super.addAll(c);
101+
}
102+
103+
@Override
104+
public boolean addAll(int index, Collection c) {
105+
checkReadOnly();
106+
return super.addAll(index, c);
107+
}
108+
109+
@Override
110+
protected void removeRange(int fromIndex, int toIndex) {
111+
checkReadOnly();
112+
super.removeRange(fromIndex, toIndex);
113+
}
114+
115+
@Override
116+
public boolean removeAll(Collection c) {
117+
checkReadOnly();
118+
return super.removeAll(c);
119+
}
120+
121+
@Override
122+
public boolean retainAll(Collection c) {
123+
checkReadOnly();
124+
return super.retainAll(c);
125+
}
126+
127+
@Override
128+
public boolean removeIf(Predicate filter) {
129+
checkReadOnly();
130+
return super.removeIf(filter);
131+
}
132+
133+
@Override
134+
public void replaceAll(UnaryOperator operator) {
135+
checkReadOnly();
136+
super.replaceAll(operator);
137+
}
138+
139+
@Override
140+
public void sort(Comparator c) {
141+
checkReadOnly();
142+
super.sort(c);
143+
}
144+
145+
@Override
146+
public ListIterator<E> listIterator() {
147+
return listIterator(0);
148+
}
149+
150+
@Override
151+
public ListIterator<E> listIterator(final int index) {
152+
return new ListIterator<>() {
153+
private final ListIterator<E> i = UnmodifiableArrayList.super.listIterator(index);
154+
155+
public boolean hasNext() {
156+
return i.hasNext();
157+
}
158+
159+
public E next() {
160+
return i.next();
161+
}
162+
163+
public boolean hasPrevious() {
164+
return i.hasPrevious();
165+
}
166+
167+
public E previous() {
168+
return i.previous();
169+
}
170+
171+
public int nextIndex() {
172+
return i.nextIndex();
173+
}
174+
175+
public int previousIndex() {
176+
return i.previousIndex();
177+
}
178+
179+
public void remove() {
180+
checkReadOnly();
181+
i.remove();
182+
}
183+
184+
public void set(E e) {
185+
checkReadOnly();
186+
i.set(e);
187+
}
188+
189+
public void add(E e) {
190+
checkReadOnly();
191+
i.add(e);
192+
}
193+
194+
@Override
195+
public void forEachRemaining(Consumer<? super E> action) {
196+
i.forEachRemaining(action);
197+
}
198+
};
199+
}
200+
}

‎pbj-integration-tests/src/test/java/com/hedera/pbj/integration/test/UnmodifiableListsTest.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
package com.hedera.pbj.integration.test;
3+
4+
import static org.junit.jupiter.api.Assertions.assertEquals;
5+
import static org.junit.jupiter.api.Assertions.assertThrows;
6+
7+
import com.hedera.pbj.runtime.io.buffer.Bytes;
8+
import com.hedera.pbj.test.proto.pbj.Everything;
9+
import java.util.List;
10+
import org.junit.jupiter.api.Test;
11+
12+
public class UnmodifiableListsTest {
13+
@Test
14+
public void testUnmodifiableList() throws Exception {
15+
// This list is unmodifiable, but can be modifiable technically if one were to supply an ArrayList instance
16+
// here.
17+
Everything obj =
18+
Everything.newBuilder().sfixed32NumberList(List.of(666)).build();
19+
final Bytes bytes = Everything.PROTOBUF.toBytes(obj);
20+
21+
// However, a list in a parsed object is guaranteed to be unmodifiable.
22+
final Everything parsedObj = Everything.PROTOBUF.parse(bytes);
23+
assertEquals(List.of(666), parsedObj.sfixed32NumberList());
24+
assertThrows(
25+
UnsupportedOperationException.class,
26+
() -> parsedObj.sfixed32NumberList().add(777));
27+
}
28+
}

0 commit comments

Comments
 (0)