Skip to content

Commit

Permalink
Merge pull request #75 from y-crdt/wrapper-refactor
Browse files Browse the repository at this point in the history 
Wrapper Refactor
  • Loading branch information
@Waidhoferj
Waidhoferj authored Jul 22, 2022
2 parents 9da503d + 8d82dc1 commit 8bc9bd1
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 135 deletions.
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[build-system]
requires = ["maturin>=0.13"]
requires = ["maturin>=0.13,<0.14"]
build-backend = "maturin"
8 changes: 4 additions & 4 deletions src/shared_types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ pub enum SubId {
}

#[derive(Clone)]
pub enum SharedType<T, P> {
Integrated(T),
pub enum SharedType<I, P> {
Integrated(I),
Prelim(P),
}

impl<T, P> SharedType<T, P> {
impl<I, P> SharedType<I, P> {
#[inline(always)]
pub fn new(value: T) -> Self {
pub fn new(value: I) -> Self {
SharedType::Integrated(value)
}

Expand Down
167 changes: 47 additions & 120 deletions src/type_conversions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use pyo3::types as pytypes;
use pyo3::types::PyList;
use std::collections::HashMap;
use std::convert::TryFrom;
use std::convert::TryInto;
use std::ops::Deref;
use yrs::block::{ItemContent, Prelim};
use yrs::types::Events;
Expand Down Expand Up @@ -113,6 +114,7 @@ impl ToPython for &Change {
}
}

#[repr(transparent)]
struct EntryChangeWrapper<'a>(&'a EntryChange);

impl<'a> IntoPy<PyObject> for EntryChangeWrapper<'a> {
Expand Down Expand Up @@ -142,11 +144,12 @@ impl<'a> IntoPy<PyObject> for EntryChangeWrapper<'a> {
}
}

#[repr(transparent)]
pub(crate) struct PyObjectWrapper(pub PyObject);

impl Prelim for PyObjectWrapper {
fn into_content(self, _txn: &mut Transaction) -> (ItemContent, Option<Self>) {
let content = match py_into_any(self.0.clone()) {
let content = match PyObjectWrapper(self.0.clone()).try_into() {
Ok(any) => ItemContent::Any(vec![any]),
Err(err) => {
if Python::with_gil(|py| err.is_instance_of::<MultipleIntegrationError>(py)) {
Expand Down Expand Up @@ -202,7 +205,7 @@ impl Prelim for PyObjectWrapper {
let mut y_map = v.borrow_mut(py);
if let SharedType::Prelim(entries) = y_map.0.to_owned() {
for (k, v) in entries {
map.insert(txn, k, PyValueWrapper(v));
map.insert(txn, k, PyObjectWrapper(v));
}
}
y_map.0 = SharedType::Integrated(map.clone());
Expand All @@ -215,60 +218,58 @@ impl Prelim for PyObjectWrapper {
}
}

const MAX_JS_NUMBER: i64 = 2_i64.pow(53) - 1;
/// Converts a Python object into an integrated Any type.
pub(crate) fn py_into_any(v: PyObject) -> PyResult<Any> {
Python::with_gil(|py| {
let v = v.as_ref(py);
impl TryFrom<PyObjectWrapper> for Any {
type Error = PyErr;

if let Ok(b) = v.downcast::<pytypes::PyBool>() {
Ok(Any::Bool(b.extract().unwrap()))
} else if let Ok(l) = v.downcast::<pytypes::PyInt>() {
let num: i64 = l.extract().unwrap();
if num > MAX_JS_NUMBER {
Ok(Any::BigInt(num))
} else {
Ok(Any::Number(num as f64))
}
} else if v.is_none() {
Ok(Any::Null)
} else if let Ok(f) = v.downcast::<pytypes::PyFloat>() {
Ok(Any::Number(f.extract().unwrap()))
} else if let Ok(s) = v.downcast::<pytypes::PyString>() {
let string: String = s.extract().unwrap();
Ok(Any::String(string.into_boxed_str()))
} else if let Ok(list) = v.downcast::<pytypes::PyList>() {
let result: PyResult<Vec<Any>> = list
.into_iter()
.map(|py_val| py_into_any(py_val.into()))
.collect();
result.map(|res| Any::Array(res.into_boxed_slice()))
} else if let Ok(dict) = v.downcast::<pytypes::PyDict>() {
if let Ok(shared) = Shared::extract(v) {
Err(MultipleIntegrationError::new_err(format!(
"Cannot integrate a nested Ypy object because is already integrated into a YDoc: {shared}"
)))
} else {
fn try_from(wrapper: PyObjectWrapper) -> Result<Self, Self::Error> {
const MAX_JS_NUMBER: i64 = 2_i64.pow(53) - 1;
let py_obj = wrapper.0;
Python::with_gil(|py| {
let v = py_obj.as_ref(py);

if let Ok(b) = v.downcast::<pytypes::PyBool>() {
Ok(Any::Bool(b.extract().unwrap()))
} else if let Ok(l) = v.downcast::<pytypes::PyInt>() {
let num: i64 = l.extract().unwrap();
if num > MAX_JS_NUMBER {
Ok(Any::BigInt(num))
} else {
Ok(Any::Number(num as f64))
}
} else if v.is_none() {
Ok(Any::Null)
} else if let Ok(f) = v.downcast::<pytypes::PyFloat>() {
Ok(Any::Number(f.extract().unwrap()))
} else if let Ok(s) = v.downcast::<pytypes::PyString>() {
let string: String = s.extract().unwrap();
Ok(Any::String(string.into_boxed_str()))
} else if let Ok(list) = v.downcast::<pytypes::PyList>() {
let result: PyResult<Vec<Any>> = list
.into_iter()
.map(|py_val| PyObjectWrapper(py_val.into()).try_into())
.collect();
result.map(|res| Any::Array(res.into_boxed_slice()))
} else if let Ok(dict) = v.downcast::<pytypes::PyDict>() {
let result: PyResult<HashMap<String, Any>> = dict
.iter()
.map(|(k, v)| {
let key: String = k.extract()?;
let value = py_into_any(v.into())?;
let value = PyObjectWrapper(v.into()).try_into()?;
Ok((key, value))
})
.collect();
result.map(|res| Any::Map(Box::new(res)))
} else if let Ok(v) = Shared::try_from(PyObject::from(v)) {
Err(MultipleIntegrationError::new_err(format!(
"Cannot integrate a nested Ypy object because is already integrated into a YDoc: {v}"
)))
} else {
Err(PyTypeError::new_err(format!(
"Cannot integrate this type into a YDoc: {v}"
)))
}
} else if let Ok(v) = Shared::try_from(PyObject::from(v)) {
Err(MultipleIntegrationError::new_err(format!(
"Cannot integrate a nested Ypy object because is already integrated into a YDoc: {v}"
)))
} else {
Err(PyTypeError::new_err(format!(
"Cannot integrate this type into a YDoc: {v}"
)))
}
})
})
}
}

impl ToPython for Any {
Expand Down Expand Up @@ -328,77 +329,3 @@ pub(crate) fn events_into_py(txn: &Transaction, events: &Events) -> PyObject {
PyList::new(py, py_events).into()
})
}

pub struct PyValueWrapper(pub PyObject);

impl Prelim for PyValueWrapper {
fn into_content(self, _txn: &mut Transaction) -> (ItemContent, Option<Self>) {
let content = match py_into_any(self.0.clone()) {
Ok(any) => ItemContent::Any(vec![any]),
Err(err) => {
if Python::with_gil(|py| err.is_instance_of::<MultipleIntegrationError>(py)) {
let shared = Shared::try_from(self.0.clone()).unwrap();
if shared.is_prelim() {
let branch = Branch::new(shared.type_ref(), None);
ItemContent::Type(branch)
} else {
Python::with_gil(|py| err.restore(py));
ItemContent::Any(vec![])
}
} else {
Python::with_gil(|py| err.restore(py));
ItemContent::Any(vec![])
}
}
};

let this = if let ItemContent::Type(_) = &content {
Some(self)
} else {
None
};

(content, this)
}

fn integrate(self, txn: &mut Transaction, inner_ref: BranchPtr) {
if let Ok(shared) = Shared::try_from(self.0) {
if shared.is_prelim() {
Python::with_gil(|py| {
match shared {
Shared::Text(v) => {
let text = Text::from(inner_ref);
let mut y_text = v.borrow_mut(py);

if let SharedType::Prelim(v) = y_text.0.to_owned() {
text.push(txn, v.as_str());
}
y_text.0 = SharedType::Integrated(text.clone());
}
Shared::Array(v) => {
let array = Array::from(inner_ref);
let mut y_array = v.borrow_mut(py);
if let SharedType::Prelim(items) = y_array.0.to_owned() {
let len = array.len();
YArray::insert_multiple_at(&array, txn, len, items);
}
y_array.0 = SharedType::Integrated(array.clone());
}
Shared::Map(v) => {
let map = Map::from(inner_ref);
let mut y_map = v.borrow_mut(py);

if let SharedType::Prelim(entries) = y_map.0.to_owned() {
for (k, v) in entries {
map.insert(txn, k, PyValueWrapper(v));
}
}
y_map.0 = SharedType::Integrated(map.clone());
}
Shared::XmlElement(_) | Shared::XmlText(_) => unreachable!("As defined in Shared::is_prelim(), neither XML type can ever exist outside a YDoc"),
}
})
}
}
}
}
10 changes: 6 additions & 4 deletions src/y_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ use crate::y_transaction::YTransaction;

use super::shared_types::SharedType;
use crate::type_conversions::ToPython;
use lib0::any::Any;
use pyo3::exceptions::PyIndexError;

use crate::type_conversions::{py_into_any, PyObjectWrapper};
use crate::type_conversions::PyObjectWrapper;
use pyo3::prelude::*;
use pyo3::types::{PyList, PySlice, PySliceIndices};
use yrs::types::array::{ArrayEvent, ArrayIter};
Expand Down Expand Up @@ -103,7 +104,8 @@ impl YArray {
pub fn insert(&mut self, txn: &mut YTransaction, index: u32, item: PyObject) -> PyResult<()> {
match &mut self.0 {
SharedType::Integrated(array) if array.len() >= index => {
py_into_any(item).map(|any| array.insert(txn, index, any))
array.insert(txn, index, PyObjectWrapper(item));
Ok(())
}
SharedType::Prelim(vec) if vec.len() >= index as usize => {
Ok(vec.insert(index as usize, item))
Expand Down Expand Up @@ -348,9 +350,9 @@ impl YArray {
let mut j = index;
let mut i = 0;
while i < src.len() {
let mut anys = Vec::default();
let mut anys: Vec<Any> = Vec::default();
while i < src.len() {
if let Ok(any) = py_into_any(src[i].clone()) {
if let Ok(any) = PyObjectWrapper(src[i].clone()).try_into() {
anys.push(any);
i += 1;
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/y_map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::shared_types::{
DeepSubscription, DefaultPyErr, PreliminaryObservationException, ShallowSubscription,
SharedType, SubId,
};
use crate::type_conversions::{events_into_py, PyValueWrapper, ToPython};
use crate::type_conversions::{events_into_py, PyObjectWrapper, ToPython};
use crate::y_transaction::YTransaction;

/// Collection used to store key-value entries in an unordered manner. Keys are always represented
Expand Down Expand Up @@ -103,7 +103,7 @@ impl YMap {
pub fn set(&mut self, txn: &mut YTransaction, key: &str, value: PyObject) {
match &mut self.0 {
SharedType::Integrated(v) => {
v.insert(txn, key.to_string(), PyValueWrapper(value));
v.insert(txn, key.to_string(), PyObjectWrapper(value));
}
SharedType::Prelim(v) => {
v.insert(key.to_string(), value);
Expand Down
8 changes: 4 additions & 4 deletions src/y_text.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ use crate::shared_types::{
DeepSubscription, DefaultPyErr, IntegratedOperationException, PreliminaryObservationException,
ShallowSubscription, SharedType, SubId,
};
use crate::type_conversions::py_into_any;
use crate::type_conversions::{events_into_py, ToPython};
use crate::type_conversions::{events_into_py, PyObjectWrapper, ToPython};
use crate::y_transaction::YTransaction;
use lib0::any::Any;
use pyo3::prelude::*;
use pyo3::types::PyList;
use std::collections::HashMap;
use std::convert::TryInto;
use std::rc::Rc;
use yrs::types::text::TextEvent;
use yrs::types::Attrs;
Expand Down Expand Up @@ -136,7 +136,7 @@ impl YText {
) -> PyResult<()> {
match &mut self.0 {
SharedType::Integrated(text) => {
let content = py_into_any(embed)?;
let content = PyObjectWrapper(embed).try_into()?;
if let Some(Ok(attrs)) = attributes.map(Self::parse_attrs) {
text.insert_embed_with_attributes(txn, index, content, attrs)
} else {
Expand Down Expand Up @@ -250,7 +250,7 @@ impl YText {
.into_iter()
.map(|(k, v)| {
let key = Rc::from(k);
let value = py_into_any(v)?;
let value = PyObjectWrapper(v).try_into()?;
Ok((key, value))
})
.collect()
Expand Down
9 changes: 9 additions & 0 deletions tests/test_y_map.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,12 @@ def callback(e: list):
container["inner"].set(txn, "don't show up", 1)

assert events is None


def test_borrow_issue():
doc = Y.YDoc()
wrapper = doc.get_array("wrapper")
inner = Y.YMap({"Foo": "Bar"})

with doc.begin_transaction() as txn:
wrapper.append(txn, inner)

0 comments on commit 8bc9bd1

Please sign in to comment.