Skip to content

Commit 20bf72b

Browse files
gaborkaszabgaborkaszab
authored
Core: Clean up TrackingStruct tests (#17041)
1) Split the TrackingBuilder tests to a separate suite 2) Replaced setters by ordinal usage with constructors calls, except in tests that deliberately exercise these setters 3) In TestTrackingStruct replaced the usage of the builder with constructor calls
1 parent d7c07d5 commit 20bf72b

2 files changed

Lines changed: 410 additions & 405 deletions

File tree

‎core/src/test/java/org/apache/iceberg/TestTrackingBuilder.java‎

Lines changed: 286 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,286 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.iceberg;
20+
21+
import static org.assertj.core.api.Assertions.assertThat;
22+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
23+
24+
import java.nio.ByteBuffer;
25+
import java.util.function.Consumer;
26+
import java.util.stream.Stream;
27+
import org.junit.jupiter.api.Test;
28+
import org.junit.jupiter.params.ParameterizedTest;
29+
import org.junit.jupiter.params.provider.Arguments;
30+
import org.junit.jupiter.params.provider.MethodSource;
31+
32+
class TestTrackingBuilder {
33+
34+
@Test
35+
void testAddedWithSameCommitDvStaysAdded() {
36+
Tracking tracking = TrackingBuilder.added(42L).dvUpdated().build();
37+
38+
assertThat(tracking.status()).isEqualTo(EntryStatus.ADDED);
39+
assertThat(tracking.snapshotId()).isEqualTo(42L);
40+
assertThat(tracking.dvSnapshotId()).isEqualTo(42L);
41+
assertThat(tracking.deletedPositions()).isNull();
42+
assertThat(tracking.replacedPositions()).isNull();
43+
// sequence numbers and firstRowId remain null; populated by inheritance
44+
assertThat(tracking.dataSequenceNumber()).isNull();
45+
assertThat(tracking.fileSequenceNumber()).isNull();
46+
assertThat(tracking.firstRowId()).isNull();
47+
}
48+
49+
@Test
50+
void testExistingBuilderPreservesSourceFields() {
51+
Tracking source = sourceTracking();
52+
53+
Tracking existing = TrackingBuilder.from(source, 1L).build();
54+
55+
assertThat(existing.status()).isEqualTo(EntryStatus.EXISTING);
56+
assertThat(existing.snapshotId()).isEqualTo(source.snapshotId());
57+
assertThat(existing.dataSequenceNumber()).isEqualTo(source.dataSequenceNumber());
58+
assertThat(existing.fileSequenceNumber()).isEqualTo(source.fileSequenceNumber());
59+
assertThat(existing.dvSnapshotId()).isEqualTo(source.dvSnapshotId());
60+
assertThat(existing.firstRowId()).isEqualTo(source.firstRowId());
61+
}
62+
63+
@Test
64+
void testDeleteUpdatesSnapshotIdAndPreservesRest() {
65+
Tracking source = sourceTracking();
66+
67+
Tracking deleted = TrackingBuilder.deleted(source, 999L);
68+
69+
assertThat(deleted.status()).isEqualTo(EntryStatus.DELETED);
70+
assertThat(deleted.snapshotId()).isEqualTo(999L);
71+
assertThat(deleted.dataSequenceNumber()).isEqualTo(source.dataSequenceNumber());
72+
assertThat(deleted.fileSequenceNumber()).isEqualTo(source.fileSequenceNumber());
73+
assertThat(deleted.dvSnapshotId()).isEqualTo(source.dvSnapshotId());
74+
assertThat(deleted.firstRowId()).isEqualTo(source.firstRowId());
75+
}
76+
77+
@Test
78+
void testReplaceUpdatesSnapshotIdAndPreservesRest() {
79+
Tracking source = sourceTracking();
80+
81+
Tracking replaced = TrackingBuilder.replaced(source, 999L);
82+
83+
assertThat(replaced.status()).isEqualTo(EntryStatus.REPLACED);
84+
assertThat(replaced.snapshotId()).isEqualTo(999L);
85+
assertThat(replaced.dataSequenceNumber()).isEqualTo(source.dataSequenceNumber());
86+
assertThat(replaced.fileSequenceNumber()).isEqualTo(source.fileSequenceNumber());
87+
assertThat(replaced.dvSnapshotId()).isEqualTo(source.dvSnapshotId());
88+
assertThat(replaced.firstRowId()).isEqualTo(source.firstRowId());
89+
}
90+
91+
@Test
92+
void testSourceDvPositionsAreNotCarriedForward() {
93+
Tracking source =
94+
new TrackingStruct(
95+
EntryStatus.ADDED, 42L, 10L, 10L, 43L, 1000L, new byte[] {1, 2}, new byte[] {3, 4});
96+
97+
Tracking existing = TrackingBuilder.from(source, 1L).build();
98+
assertThat(existing.deletedPositions()).isNull();
99+
assertThat(existing.replacedPositions()).isNull();
100+
101+
Tracking deleted = TrackingBuilder.deleted(source, 999L);
102+
assertThat(deleted.deletedPositions()).isNull();
103+
assertThat(deleted.replacedPositions()).isNull();
104+
105+
Tracking replaced = TrackingBuilder.replaced(source, 999L);
106+
assertThat(replaced.deletedPositions()).isNull();
107+
assertThat(replaced.replacedPositions()).isNull();
108+
}
109+
110+
@Test
111+
void testDvUpdatedProducesModifiedAndAdvancesDvSnapshotId() {
112+
Tracking source = sourceTracking();
113+
Tracking modified = TrackingBuilder.from(source, 999L).dvUpdated().build();
114+
115+
assertThat(modified.status()).isEqualTo(EntryStatus.MODIFIED);
116+
// the entry snapshot id is preserved so we still know when the base file was added
117+
assertThat(modified.snapshotId()).isEqualTo(source.snapshotId()).isNotEqualTo(999L);
118+
// only the DV snapshot id advances to the commit snapshot
119+
assertThat(modified.dvSnapshotId()).isEqualTo(999L);
120+
}
121+
122+
@Test
123+
void testManifestDVMutatorsRejectedOnAdded() {
124+
assertThatThrownBy(
125+
() -> TrackingBuilder.added(42L).deletedPositions(ByteBuffer.wrap(new byte[] {1})))
126+
.isInstanceOf(IllegalStateException.class)
127+
.hasMessage("Cannot set deleted positions on ADDED entry");
128+
129+
assertThatThrownBy(
130+
() -> TrackingBuilder.added(42L).replacedPositions(ByteBuffer.wrap(new byte[] {1})))
131+
.isInstanceOf(IllegalStateException.class)
132+
.hasMessage("Cannot set replaced positions on ADDED entry");
133+
}
134+
135+
@Test
136+
void testDvUpdatedRejectedWhenManifestPositionsSet() {
137+
assertThatThrownBy(
138+
() ->
139+
TrackingBuilder.from(manifestSourceTracking(), 999L)
140+
.deletedPositions(ByteBuffer.wrap(new byte[] {1}))
141+
.dvUpdated())
142+
.isInstanceOf(IllegalStateException.class)
143+
.hasMessage(
144+
"Cannot mark DV updated on a manifest entry (deleted/replaced positions are set)");
145+
146+
assertThatThrownBy(
147+
() ->
148+
TrackingBuilder.from(manifestSourceTracking(), 999L)
149+
.replacedPositions(ByteBuffer.wrap(new byte[] {1}))
150+
.dvUpdated())
151+
.isInstanceOf(IllegalStateException.class)
152+
.hasMessage(
153+
"Cannot mark DV updated on a manifest entry (deleted/replaced positions are set)");
154+
}
155+
156+
@Test
157+
void testBuilderRejectsNullSource() {
158+
assertThatThrownBy(() -> TrackingBuilder.from(null, 1L))
159+
.isInstanceOf(IllegalArgumentException.class)
160+
.hasMessage("Invalid source tracking: null");
161+
}
162+
163+
@Test
164+
void testSourceBuildersRejectSourceWithoutSequenceNumbers() {
165+
Tracking missingBoth = TrackingBuilder.added(42L).build();
166+
167+
assertThatThrownBy(() -> TrackingBuilder.from(missingBoth, 1L))
168+
.isInstanceOf(IllegalArgumentException.class)
169+
.hasMessage("Invalid tracking source: data sequence number is null");
170+
171+
assertThatThrownBy(() -> TrackingBuilder.deleted(missingBoth, 1L))
172+
.isInstanceOf(IllegalArgumentException.class)
173+
.hasMessage("Invalid tracking source: data sequence number is null");
174+
175+
assertThatThrownBy(() -> TrackingBuilder.replaced(missingBoth, 1L))
176+
.isInstanceOf(IllegalArgumentException.class)
177+
.hasMessage("Invalid tracking source: data sequence number is null");
178+
179+
TrackingStruct missingFileSeq =
180+
new TrackingStruct(EntryStatus.ADDED, 42L, 10L, null, null, null, null, null);
181+
182+
assertThatThrownBy(() -> TrackingBuilder.from(missingFileSeq, 1L))
183+
.isInstanceOf(IllegalArgumentException.class)
184+
.hasMessage("Invalid tracking source: file sequence number is null");
185+
186+
assertThatThrownBy(() -> TrackingBuilder.deleted(missingFileSeq, 1L))
187+
.isInstanceOf(IllegalArgumentException.class)
188+
.hasMessage("Invalid tracking source: file sequence number is null");
189+
190+
assertThatThrownBy(() -> TrackingBuilder.replaced(missingFileSeq, 1L))
191+
.isInstanceOf(IllegalArgumentException.class)
192+
.hasMessage("Invalid tracking source: file sequence number is null");
193+
}
194+
195+
private static Stream<Arguments> terminalTransitionCases() {
196+
Consumer<Tracking> builderCall = source -> TrackingBuilder.from(source, 1L);
197+
Consumer<Tracking> deletedCall = source -> TrackingBuilder.deleted(source, 1L);
198+
Consumer<Tracking> replacedCall = source -> TrackingBuilder.replaced(source, 1L);
199+
return Stream.of(
200+
Arguments.of(EntryStatus.DELETED, builderCall),
201+
Arguments.of(EntryStatus.DELETED, deletedCall),
202+
Arguments.of(EntryStatus.DELETED, replacedCall),
203+
Arguments.of(EntryStatus.REPLACED, builderCall),
204+
Arguments.of(EntryStatus.REPLACED, deletedCall),
205+
Arguments.of(EntryStatus.REPLACED, replacedCall));
206+
}
207+
208+
@ParameterizedTest
209+
@MethodSource("terminalTransitionCases")
210+
void testRejectsTransitionsFromTerminalStatus(
211+
EntryStatus sourceStatus, Consumer<Tracking> factoryCall) {
212+
Tracking source = sourceTrackingWithStatus(sourceStatus);
213+
assertThatThrownBy(() -> factoryCall.accept(source))
214+
.isInstanceOf(IllegalStateException.class)
215+
.hasMessage("Cannot revive non-live entry with status " + sourceStatus);
216+
}
217+
218+
@Test
219+
void testExistingToExistingIsAllowed() {
220+
Tracking existingSource = sourceTrackingWithStatus(EntryStatus.EXISTING);
221+
222+
Tracking existing = TrackingBuilder.from(existingSource, 1L).build();
223+
224+
assertThat(existing.status()).isEqualTo(EntryStatus.EXISTING);
225+
assertThat(existing.snapshotId()).isEqualTo(existingSource.snapshotId());
226+
}
227+
228+
@Test
229+
void testExistingToTerminalTransitions() {
230+
Tracking existingSource = sourceTrackingWithStatus(EntryStatus.EXISTING);
231+
232+
Tracking deleted = TrackingBuilder.deleted(existingSource, 999L);
233+
assertThat(deleted.status()).isEqualTo(EntryStatus.DELETED);
234+
assertThat(deleted.snapshotId()).isEqualTo(999L);
235+
236+
Tracking replaced = TrackingBuilder.replaced(existingSource, 999L);
237+
assertThat(replaced.status()).isEqualTo(EntryStatus.REPLACED);
238+
assertThat(replaced.snapshotId()).isEqualTo(999L);
239+
}
240+
241+
@Test
242+
void testExistingPreservesSourceSnapshotId() {
243+
Tracking source = sourceTracking();
244+
Tracking existing = TrackingBuilder.from(source, 999L).build();
245+
assertThat(existing.status()).isEqualTo(EntryStatus.EXISTING);
246+
assertThat(existing.snapshotId()).isEqualTo(source.snapshotId()).isNotEqualTo(999L);
247+
}
248+
249+
@Test
250+
void testCarryForwardFromModifiedSourceChangesToExisting() {
251+
Tracking modifiedSource = sourceTrackingWithStatus(EntryStatus.MODIFIED);
252+
Tracking carried = TrackingBuilder.from(modifiedSource, 999L).build();
253+
assertThat(carried.status()).isEqualTo(EntryStatus.EXISTING);
254+
assertThat(carried.snapshotId()).isEqualTo(modifiedSource.snapshotId()).isNotEqualTo(999L);
255+
assertThat(carried.dvSnapshotId()).isEqualTo(modifiedSource.dvSnapshotId()).isNotEqualTo(999L);
256+
assertThat(carried.dataSequenceNumber()).isEqualTo(modifiedSource.dataSequenceNumber());
257+
assertThat(carried.fileSequenceNumber()).isEqualTo(modifiedSource.fileSequenceNumber());
258+
assertThat(carried.firstRowId()).isEqualTo(modifiedSource.firstRowId());
259+
}
260+
261+
@Test
262+
void testManifestDVPositionsProduceModified() {
263+
ByteBuffer deletedBytes = ByteBuffer.wrap(new byte[] {1, 2});
264+
265+
Tracking addedSource = manifestSourceTracking();
266+
Tracking modified =
267+
TrackingBuilder.from(addedSource, 999L).deletedPositions(deletedBytes).build();
268+
assertThat(modified.status()).isEqualTo(EntryStatus.MODIFIED);
269+
// the entry snapshot id is preserved; only the DV snapshot id advances to the commit snapshot
270+
assertThat(modified.snapshotId()).isEqualTo(addedSource.snapshotId()).isNotEqualTo(999L);
271+
assertThat(modified.dvSnapshotId()).isEqualTo(999L);
272+
assertThat(modified.deletedPositions()).isEqualTo(deletedBytes);
273+
}
274+
275+
private static TrackingStruct sourceTracking() {
276+
return sourceTrackingWithStatus(EntryStatus.ADDED);
277+
}
278+
279+
private static TrackingStruct sourceTrackingWithStatus(EntryStatus status) {
280+
return new TrackingStruct(status, 42L, 10L, 10L, 43L, 1000L, null, null);
281+
}
282+
283+
private static TrackingStruct manifestSourceTracking() {
284+
return new TrackingStruct(EntryStatus.ADDED, 42L, 10L, 10L, null, 1000L, null, null);
285+
}
286+
}

0 commit comments

Comments
 (0)