Skip to content

Commit b09fc48

Browse files
Leeeon233Leeeon233
authored
Merge pull request #113 from loro-dev/refator-delete-map
refactor: map delete value
2 parents 74cbd42 + d2b8941 commit b09fc48

File tree

11 files changed

+94
-66
lines changed

11 files changed

+94
-66
lines changed

‎crates/loro-internal/src/arena.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::{
1010
container::{
1111
idx::ContainerIdx,
1212
list::list_op::{InnerListOp, ListOp},
13-
map::InnerMapSet,
13+
map::{InnerMapSet, MapSet},
1414
text::text_content::SliceRange,
1515
ContainerID,
1616
},
@@ -81,15 +81,16 @@ impl<'a> OpConverter<'a> {
8181
};
8282

8383
match content {
84-
crate::op::RawOpContent::Map(map) => {
85-
let value = _alloc_value(&mut self.values, map.value) as u32;
84+
crate::op::RawOpContent::Map(MapSet { key, value }) => {
85+
let value = if let Some(value) = value {
86+
Some(_alloc_value(&mut self.values, value) as u32)
87+
} else {
88+
None
89+
};
8690
Op {
8791
counter,
8892
container,
89-
content: crate::op::InnerContent::Map(InnerMapSet {
90-
key: map.key,
91-
value,
92-
}),
93+
content: crate::op::InnerContent::Map(InnerMapSet { key, value }),
9394
}
9495
}
9596
crate::op::RawOpContent::List(list) => match list {
@@ -291,15 +292,16 @@ impl SharedArena {
291292
container: ContainerIdx,
292293
) -> Op {
293294
match content {
294-
crate::op::RawOpContent::Map(map) => {
295-
let value = self.alloc_value(map.value) as u32;
295+
crate::op::RawOpContent::Map(MapSet { key, value }) => {
296+
let value = if let Some(value) = value {
297+
Some(self.alloc_value(value) as u32)
298+
} else {
299+
None
300+
};
296301
Op {
297302
counter,
298303
container,
299-
content: crate::op::InnerContent::Map(InnerMapSet {
300-
key: map.key,
301-
value,
302-
}),
304+
content: crate::op::InnerContent::Map(InnerMapSet { key, value }),
303305
}
304306
}
305307
crate::op::RawOpContent::List(list) => match list {

‎crates/loro-internal/src/container/map/map_content.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@ use crate::{InternalString, LoroValue};
77
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
88
pub struct MapSet {
99
pub(crate) key: InternalString,
10-
pub(crate) value: LoroValue,
10+
// the key is deleted if value is None
11+
pub(crate) value: Option<LoroValue>,
1112
}
1213

1314
#[derive(Clone, Debug, PartialEq, Eq)]
1415
pub struct InnerMapSet {
1516
pub(crate) key: InternalString,
16-
// FIXME: how to set None?
17-
pub(crate) value: u32,
17+
// the key is deleted if value is None
18+
pub(crate) value: Option<u32>,
1819
}
1920

2021
impl Mergable for MapSet {}
@@ -50,9 +51,9 @@ mod test {
5051
fn fix_fields_order() {
5152
let map_set = vec![MapSet {
5253
key: "key".to_string().into(),
53-
value: "value".to_string().into(),
54+
value: Some("value".to_string().into()),
5455
}];
55-
let map_set_buf = vec![1, 3, 107, 101, 121, 4, 5, 118, 97, 108, 117, 101];
56+
let map_set_buf = vec![1, 3, 107, 101, 121, 1, 4, 5, 118, 97, 108, 117, 101];
5657
assert_eq!(
5758
postcard::from_bytes::<Vec<MapSet>>(&map_set_buf).unwrap(),
5859
map_set

‎crates/loro-internal/src/diff_calc.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ impl DiffCalculatorTrait for MapDiffCalculator {
293293
for (key, value) in changed {
294294
let value = value
295295
.map(|v| {
296-
let value = oplog.arena.get_value(v.value as usize);
296+
let value = v.value.map(|v| oplog.arena.get_value(v as usize)).flatten();
297297
MapValue {
298298
counter: v.counter,
299299
value,
@@ -317,7 +317,7 @@ struct CompactMapValue {
317317
lamport: Lamport,
318318
peer: PeerID,
319319
counter: Counter,
320-
value: u32,
320+
value: Option<u32>,
321321
}
322322

323323
impl HasId for CompactMapValue {

‎crates/loro-internal/src/encoding/encode_changes.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ struct OpEncoding {
5151
/// key index or insert/delete pos
5252
#[columnar(strategy = "DeltaRle")]
5353
prop: usize,
54-
value: LoroValue,
54+
value: Option<LoroValue>,
5555
}
5656

5757
#[columnar(vec, ser, de, iterable)]
@@ -169,16 +169,16 @@ pub(super) fn encode_oplog_changes(oplog: &OpLog, vv: &VersionVector) -> Vec<u8>
169169
ListOp::Insert { slice, pos } => (
170170
pos,
171171
// TODO: perf may be optimized by using borrow type instead
172-
match slice {
172+
Some(match slice {
173173
ListSlice::RawData(v) => LoroValue::List(Arc::new(v.to_vec())),
174174
ListSlice::RawStr {
175175
str,
176176
unicode_len: _,
177177
} => LoroValue::String(Arc::new(str.to_string())),
178-
},
178+
}),
179179
),
180180
ListOp::Delete(span) => {
181-
(span.pos as usize, LoroValue::I32(span.len as i32))
181+
(span.pos as usize, Some(LoroValue::I32(span.len as i32)))
182182
}
183183
},
184184
};
@@ -270,6 +270,7 @@ pub(super) fn decode_changes_to_inner_format_oplog(
270270
}
271271
ContainerType::List | ContainerType::Text => {
272272
let pos = prop;
273+
let value = value.unwrap();
273274
let list_op = match value {
274275
LoroValue::I32(len) => ListOp::Delete(DeleteSpan {
275276
pos: pos as isize,

‎crates/loro-internal/src/encoding/encode_enhanced.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ struct DocEncoding<'a> {
110110
#[columnar(borrow)]
111111
root_containers: VarZeroVec<'a, RootContainerULE, Index32>,
112112
start_counter: Vec<Counter>,
113-
values: Vec<LoroValue>,
113+
values: Vec<Option<LoroValue>>,
114114
clients: Vec<PeerID>,
115115
keys: Vec<InternalString>,
116116
}
@@ -200,7 +200,7 @@ pub fn encode_oplog_v2(oplog: &OpLog, vv: &VersionVector) -> Vec<u8> {
200200
let mut len = 0;
201201
match slice {
202202
ListSlice::RawData(v) => {
203-
values.push(LoroValue::List(Arc::new(v.to_vec())));
203+
values.push(Some(LoroValue::List(Arc::new(v.to_vec()))));
204204
}
205205
ListSlice::RawStr {
206206
str,
@@ -451,7 +451,7 @@ pub fn decode_oplog_v2(oplog: &mut OpLog, input: &[u8]) -> Result<(), LoroError>
451451
})
452452
}
453453
ContainerType::List => {
454-
let value = value_iter.next().unwrap();
454+
let value = value_iter.next().flatten().unwrap();
455455
RawOpContent::List(ListOp::Insert {
456456
slice: ListSlice::RawData(Cow::Owned(
457457
match Arc::try_unwrap(value.into_list().unwrap()) {

‎crates/loro-internal/src/handler.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ impl MapHandler {
527527
self.container_idx,
528528
crate::op::RawOpContent::Map(crate::container::map::MapSet {
529529
key: key.into(),
530-
value,
530+
value: Some(value),
531531
}),
532532
None,
533533
&self.state,
@@ -548,7 +548,7 @@ impl MapHandler {
548548
self.container_idx,
549549
crate::op::RawOpContent::Map(crate::container::map::MapSet {
550550
key: key.into(),
551-
value: LoroValue::Container(container_id),
551+
value: Some(LoroValue::Container(container_id)),
552552
}),
553553
None,
554554
&self.state,
@@ -562,8 +562,7 @@ impl MapHandler {
562562
self.container_idx,
563563
crate::op::RawOpContent::Map(crate::container::map::MapSet {
564564
key: key.into(),
565-
// TODO: use another special value to delete?
566-
value: LoroValue::Null,
565+
value: None,
567566
}),
568567
None,
569568
&self.state,

‎crates/loro-internal/src/op/content.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,10 @@ mod test {
198198
RawOpContent::List(ListOp::Delete(DeleteSpan { pos: 0, len: 1 })),
199199
RawOpContent::Map(MapSet {
200200
key: "a".to_string().into(),
201-
value: "b".to_string().into(),
201+
value: Some("b".to_string().into()),
202202
}),
203203
];
204-
let remote_content_buf = vec![2, 1, 1, 0, 2, 0, 1, 97, 4, 1, 98];
204+
let remote_content_buf = vec![2, 1, 1, 0, 2, 0, 1, 97, 1, 4, 1, 98];
205205
assert_eq!(
206206
postcard::from_bytes::<Vec<RawOpContent>>(&remote_content_buf).unwrap(),
207207
remote_content

‎crates/loro-internal/src/oplog.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,10 +396,13 @@ impl OpLog {
396396
}
397397
},
398398
crate::op::InnerContent::Map(map) => {
399-
let value = self.arena.get_value(map.value as usize);
399+
let value = map
400+
.value
401+
.map(|v| self.arena.get_value(v as usize))
402+
.flatten();
400403
contents.push(RawOpContent::Map(crate::container::map::MapSet {
401404
key: map.key.clone(),
402-
value: value.unwrap_or(crate::LoroValue::Null), // TODO: decide map delete value
405+
value,
403406
}))
404407
}
405408
};

‎crates/loro-internal/src/snapshot_encode.rs

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,21 @@ pub fn decode_oplog(
141141
SnapshotOp::Map {
142142
key,
143143
value_idx_plus_one,
144-
} => Op::new(
145-
id,
146-
InnerContent::Map(InnerMapSet {
147-
key: (&*keys[key]).into(),
148-
value: value_idx_plus_one - 1,
149-
}),
150-
container_idx,
151-
),
144+
} => {
145+
let value = if value_idx_plus_one == 0 {
146+
None
147+
} else {
148+
Some(value_idx_plus_one - 1)
149+
};
150+
Op::new(
151+
id,
152+
InnerContent::Map(InnerMapSet {
153+
key: (&*keys[key]).into(),
154+
value,
155+
}),
156+
container_idx,
157+
)
158+
}
152159
_ => unreachable!(),
153160
}
154161
}
@@ -323,9 +330,9 @@ struct EncodedSnapshotOp {
323330
is_del: bool,
324331
// Text: 0
325332
// List: 0 | value index
326-
// Map: value index
333+
// Map: 0 (deleted) | value index + 1
327334
#[columnar(strategy = "DeltaRle")]
328-
value: usize,
335+
value: isize,
329336
}
330337

331338
enum SnapshotOp {
@@ -365,9 +372,10 @@ impl EncodedSnapshotOp {
365372
}
366373

367374
pub fn get_map(&self) -> SnapshotOp {
375+
let value_idx_plus_one = if self.value < 0 { 0 } else { self.value as u32 };
368376
SnapshotOp::Map {
369377
key: self.prop,
370-
value_idx_plus_one: self.value as u32,
378+
value_idx_plus_one,
371379
}
372380
}
373381

@@ -381,7 +389,7 @@ impl EncodedSnapshotOp {
381389
prop: pos,
382390
len: 0,
383391
is_del: false,
384-
value: start as usize,
392+
value: start as isize,
385393
},
386394
SnapshotOp::TextOrListDelete { pos, len } => Self {
387395
container,
@@ -393,13 +401,16 @@ impl EncodedSnapshotOp {
393401
SnapshotOp::Map {
394402
key,
395403
value_idx_plus_one: value,
396-
} => Self {
397-
container,
398-
prop: key,
399-
len: 0,
400-
is_del: false,
401-
value: value as usize,
402-
},
404+
} => {
405+
let value = if value == 0 { -1 } else { value as isize };
406+
Self {
407+
container,
408+
prop: key,
409+
len: 0,
410+
is_del: false,
411+
value,
412+
}
413+
}
403414
SnapshotOp::TextInsert { pos, len } => Self {
404415
container,
405416
prop: pos,
@@ -665,17 +676,16 @@ fn encode_oplog(oplog: &OpLog, state_ref: Option<PreEncodedState>) -> FinalPhase
665676
},
666677
InnerContent::Map(map) => {
667678
let key = record_key(&map.key);
668-
let value = oplog.arena.get_value(map.value as usize);
669-
// FIXME: delete in map
679+
let value = map.value.and_then(|v| oplog.arena.get_value(v as usize));
670680
let value = if let Some(value) = value {
671-
record_value(&value) + 1
681+
(record_value(&value) + 1) as u32
672682
} else {
673683
0
674684
};
675685
encoded_ops.push(EncodedSnapshotOp::from(
676686
SnapshotOp::Map {
677687
key,
678-
value_idx_plus_one: value as u32,
688+
value_idx_plus_one: value,
679689
},
680690
op.container.to_index(),
681691
));

‎crates/loro-internal/src/state/map_state.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use loro_common::ContainerID;
55

66
use crate::{
77
arena::SharedArena,
8-
container::idx::ContainerIdx,
8+
container::{idx::ContainerIdx, map::MapSet},
99
delta::MapValue,
1010
event::{Diff, Index},
1111
op::{RawOp, RawOpContent},
@@ -39,18 +39,30 @@ impl ContainerState for MapState {
3939

4040
fn apply_op(&mut self, op: RawOp, arena: &SharedArena) {
4141
match op.content {
42-
RawOpContent::Map(map) => {
43-
if map.value.is_container() {
44-
let idx = arena.register_container(map.value.as_container().unwrap());
42+
RawOpContent::Map(MapSet { key, value }) => {
43+
if value.is_none() {
44+
self.insert(
45+
key,
46+
MapValue {
47+
lamport: (op.lamport, op.id.peer),
48+
counter: op.id.counter,
49+
value: None,
50+
},
51+
);
52+
return;
53+
}
54+
let value = value.unwrap();
55+
if value.is_container() {
56+
let idx = arena.register_container(value.as_container().unwrap());
4557
arena.set_parent(idx, Some(self.idx));
4658
}
4759

4860
self.insert(
49-
map.key,
61+
key,
5062
MapValue {
5163
lamport: (op.lamport, op.id.peer),
5264
counter: op.id.counter,
53-
value: Some(map.value),
65+
value: Some(value),
5466
},
5567
)
5668
}

‎crates/loro-internal/src/txn.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,13 +342,13 @@ fn change_to_diff(
342342
),
343343
},
344344
crate::op::InnerContent::Map(map) => {
345-
let value = arena.get_value(map.value as usize).unwrap();
345+
let value = map.value.map(|v| arena.get_value(v as usize)).flatten();
346346
let mut updated: FxHashMap<_, _> = Default::default();
347347
updated.insert(
348348
map.key.clone(),
349349
MapValue {
350350
counter,
351-
value: Some(value),
351+
value,
352352
lamport: (lamport, peer),
353353
},
354354
);

0 commit comments

Comments
 (0)