Skip to content

Commit

Permalink
fix: reuse active transaction when converting values
Browse files Browse the repository at this point in the history
  • Loading branch information
eliias committed Nov 13, 2023
1 parent 724c40d commit 21711a6
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 10 deletions.
32 changes: 30 additions & 2 deletions ext/yrb/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use crate::yvalue::YValue;
use lib0::any::Any;
use magnus::r_hash::ForEach::Continue;
use magnus::{exception, Error, RHash, RString, Symbol, Value};
use magnus::{exception, Error, RArray, RHash, RString, Symbol, Value};
use std::sync::Arc;
use yrs::types::Attrs;
use yrs::types::{Attrs, Value as YrsValue};
use yrs::{Array, Map, TransactionMut};

#[derive(Debug, Clone)]
pub(crate) struct TypeConversionError;
Expand Down Expand Up @@ -35,3 +36,30 @@ pub(crate) fn map_rhash_to_attrs(hash: RHash) -> Result<Attrs, Error> {

Ok(a)
}

pub(crate) fn convert_yvalue_to_ruby_value(value: YrsValue, tx: &TransactionMut) -> YValue {
match value {
YrsValue::Any(val) => YValue::from(val),
YrsValue::YText(text) => YValue::from(text),
YrsValue::YXmlElement(el) => YValue::from(el),
YrsValue::YXmlText(text) => YValue::from(text),
YrsValue::YArray(val) => {
let arr = RArray::new();
for item in val.iter(tx) {
let val = convert_yvalue_to_ruby_value(item.clone(), tx);
let val = *val.0.borrow();
arr.push(val).expect("cannot push item event to array");
}
YValue::from(arr)
}
YrsValue::YMap(val) => {
let iter = val.iter(tx).map(|(key, val)| {
let val = convert_yvalue_to_ruby_value(val.clone(), tx);
let val = val.0.into_inner();
(key, val)
});
YValue::from(RHash::from_iter(iter))
}
v => panic!("cannot map given yrs values to yvalue: {:?}", v),
}
}
8 changes: 5 additions & 3 deletions ext/yrb/src/yarray.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::utils::convert_yvalue_to_ruby_value;
use crate::ytransaction::YTransaction;
use crate::yvalue::YValue;
use lib0::any::Any;
Expand All @@ -22,20 +23,21 @@ impl YArray {

let arr = self.0.borrow();
arr.iter(tx).for_each(|val| {
let yvalue = YValue::from(val);
let args = (yvalue.into(),);
let yvalue = *convert_yvalue_to_ruby_value(val, tx).0.borrow();
let args = (yvalue,);
let _ = block.call::<(Value,), Qnil>(args);
});

Ok(())
}

pub(crate) fn yarray_get(&self, transaction: &YTransaction, index: u32) -> Value {
let tx = transaction.transaction();
let tx = tx.as_ref().unwrap();

let arr = self.0.borrow();
let v = arr.get(tx, index).unwrap();
YValue::from(v).into()
*convert_yvalue_to_ruby_value(v, tx).0.borrow()
}
pub(crate) fn yarray_insert(&self, transaction: &YTransaction, index: u32, value: Value) {
let yvalue = YValue::from(value);
Expand Down
9 changes: 5 additions & 4 deletions ext/yrb/src/ymap.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::indifferent_hash_key;
use crate::utils::{convert_yvalue_to_ruby_value, indifferent_hash_key};
use crate::yvalue::YValue;
use crate::YTransaction;
use lib0::any::Any;
Expand Down Expand Up @@ -30,25 +30,26 @@ impl YMap {
Some(k) => self.0.borrow().contains_key(tx, k.as_str()),
}
}

pub(crate) fn ymap_each(&self, transaction: &YTransaction, proc: Proc) {
let tx = transaction.transaction();
let tx = tx.as_ref().unwrap();

self.0.borrow().iter(tx).for_each(|(key, val)| {
let k = key.to_string();
let v = *YValue::from(val).0.borrow();
let v = *convert_yvalue_to_ruby_value(val, tx).0.borrow();
proc.call::<(String, Value), Value>((k, v))
.expect("cannot iterate map");
})
}

pub(crate) fn ymap_get(&self, transaction: &YTransaction, key: Value) -> Option<Value> {
let tx = transaction.transaction();
let tx = tx.as_ref().unwrap();

indifferent_hash_key(key)
.map(|k| self.0.borrow().get(tx, k.as_str()))
.map(|v| v.unwrap_or(YrsValue::Any(Any::Undefined)))
.map(|v| *YValue::from(v).0.borrow())
.map(|v| *convert_yvalue_to_ruby_value(v, tx).0.borrow())
}
pub(crate) fn ymap_insert(
&self,
Expand Down
4 changes: 3 additions & 1 deletion ext/yrb/src/yvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,18 @@ impl From<YrsValue> for YValue {
YrsValue::YXmlElement(el) => YValue::from(el),
YrsValue::YXmlText(text) => YValue::from(text),
YrsValue::YArray(val) => {
print!("try to acquire transaction");
let tx = val.transact();
let arr = RArray::new();
for item in val.iter(&tx) {
let val = YValue::from(item.clone());
let val = val.0.borrow().clone();
let val = *val.0.borrow();
arr.push(val).expect("cannot push item event to array");
}
YValue::from(arr)
}
YrsValue::YMap(val) => {
print!("try to acquire transaction");
let tx = val.transact();
let iter = val.iter(&tx).map(|(key, val)| {
let val = YValue::from(val);
Expand Down
1 change: 1 addition & 0 deletions spec/files/issue131_ydoc.base64.txt

Large diffs are not rendered by default.

46 changes: 46 additions & 0 deletions spec/y/bugs_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require "base64"

RSpec.describe "Bugs" do
it "issue #45" do
diffs = [[1, 3, 197, 134, 244, 186, 10, 0, 7, 1, 7, 100, 101, 102, 97, 117, 108, 116, 3, 9, 112, 97, 114, 97, 103, 114, 97, 112, 104, 7, 0, 197, 134, 244, 186, 10, 0, 6, 4, 0, 197, 134, 244, 186, 10, 1, 1, 115, 0],
Expand Down Expand Up @@ -107,4 +109,48 @@
doc.sync(diff)
end
end

it "issue #131" do
file = File.join(__dir__, "/../files/issue131_ydoc.base64.txt")
message = File.read(file)
update = Base64.decode64(message).unpack("C*")

doc = Y::Doc.new
doc.sync(update)

ymap = doc.get_map("data")

expect(ymap["agents"]).to eq([])
expect(ymap["fields"]).to eq(
{
"test_text" => {
"order" => 1.0,
"type" => "text",
"label" => "Test Text"
},
"test_date" => {
"order" => 1.0,
"type" => "date",
"label" => "Test Date"
},
"test_person" => {
"order" => 2.0,
"type" => "person",
"label" => "Test Person"
}
}
)
expect(ymap["testArray"]).to eq([1.0, "two", true])
expect(ymap["testBool"]).to be(false)
expect(ymap["testFloat"]).to eq(3.1415)
expect(ymap["testInt"]).to eq(1.0)
expect(ymap["title"]).to eq("New Document 3")
expect(ymap["testHash"]).to eq(
{
"c" => false,
"a" => 1.0,
"b" => "2"
}
)
end
end

0 comments on commit 21711a6

Please sign in to comment.