Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception occurs when inserting nested Maps into document #99

Closed
twaddell opened this issue May 30, 2024 · 6 comments · Fixed by #100
Closed

Exception occurs when inserting nested Maps into document #99

twaddell opened this issue May 30, 2024 · 6 comments · Fixed by #100
Assignees

Comments

@twaddell
Copy link
Contributor

When attempting to insert a more complex nested map into a document the following exception occurs:

System.Runtime.InteropServices.SEHException : External component has thrown an exception.
   at YDotNet.Native.Types.Maps.MapChannel.Insert(IntPtr map, IntPtr transaction, IntPtr key, IntPtr value)
   at YDotNet.Document.Types.Maps.Map.Insert(Transaction transaction, String key, Input input) in X:\ydotnet\YDotNet\Document\Types\Maps\Map.cs:line 51
   at YDotNet.Tests.Unit.Maps.InsertTests.InsertNestedMap() in X:\ydotnet\Tests\YDotNet.Tests.Unit\Maps\InsertTests.cs:line 403
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

The following test case can be used to reproduce the issue:

[Test]
public void InsertNestedMap()
{
    // Arrange
    var doc = new Doc();
    var map = doc.Map("map");

    // Act
    var transaction = doc.WriteTransaction();
    var innerMap = Input.Map(new Dictionary<string, Input>
    {
        { "text", Input.String("Nested data") }
    });
    var outerMap = Input.Map(new Dictionary<string, Input>
    {
        { "innerMap", innerMap }
    });
    map.Insert(transaction, "outerMap", outerMap);
    var length = map.Length(transaction);
    transaction.Commit();

    // Assert
    Assert.That(length, Is.EqualTo(expected: 1));
}

In case it makes a difference I'm using the YDotNet.Native.Win32 native library.

Please do let me know if this is not the best approach to creating nested data structures as I'm relatively new to this library.

Thanks

@SebastianStehle SebastianStehle self-assigned this May 31, 2024
@SebastianStehle
Copy link
Collaborator

Thanks a lot. This does not seem to be a problem with YDotNet, but with y-crdt, the rustl library. If you have experience with rust, it would be great if you could reproduce it there.

@Horusiath
Copy link
Collaborator

You probably won't reproduce it directly in rust (we have a lot of tests to confirm that working on nested collections work). We could try to check it out at the yffi level (C FFI bindings to Rust). Sometimes issues happen on that level.

@SebastianStehle
Copy link
Collaborator

Thanks for feedback @Horusiath . I would wait for this PR, because it changes a lot anyway: #94

@twaddell
Copy link
Contributor Author

Sorry for the slow update, I'm new to Rust and it's been quite a few years since I've done any c++ so there may be some mistakes in the code snippets below.

  #[test]
  fn map_nested() {
      let d1 = Doc::with_client_id(1);
      let m1 = d1.get_or_insert_map("map");
      let mut t1 = d1.transact_mut();

      let mut innerMap: HashMap<String, _> = HashMap::new();
      innerMap.insert("text".to_owned(), "Nested data");
      
      let mut outerMap: HashMap<String, _> = HashMap::new();
      outerMap.insert("innerMap".to_owned(), MapPrelim::from(innerMap));

      m1.insert(&mut t1, "outerMap".to_owned(), MapPrelim::from(outerMap));
      
      assert_eq!(m1.len(&t1), 1);
  }
  • The following test case in yffi fails
TEST_CASE("YMap multiple nested maps") {
    YDoc* doc = ydoc_new_with_id(1);
    Branch* map = ymap(doc, "map");
    YTransaction* txn = ydoc_write_transaction(doc, 0, NULL);

    char* key = (char*)"text";
    YInput value = yinput_string("Nested data");
    YInput innerMap = yinput_ymap(&key, &value, 1);

    char* key2 = (char*)"innerMap";
    YInput outerMap = yinput_ymap(&key2, &innerMap, 1);

    ymap_insert(map, txn, "outerMap", &outerMap);
    int length = ymap_len(map, txn);
    ytransaction_commit(txn);
    REQUIRE(length == 1);

    ydoc_destroy(doc);
}
thread '<unnamed>' panicked at yffi\src\lib.rs:2488:17:
Unrecognized YVal value tag.
  • Simplifying the nesting in yffi will allow the test to pass
TEST_CASE("YMap nested map") {
    YDoc* doc = ydoc_new_with_id(1);
    Branch* map = ymap(doc, "map");
    YTransaction* txn = ydoc_write_transaction(doc, 0, NULL);

    char* key = (char*)"text";
    YInput value = yinput_string("Nested data");
    YInput innerMap = yinput_ymap(&key, &value, 1);

    ymap_insert(map, txn, "innerMap", &innerMap);
    int length = ymap_len(map, txn);
    ytransaction_commit(txn);
    REQUIRE(length == 1);

    ydoc_destroy(doc);
}

I hope that is useful let me know if there is any more I can do to help.

@SebastianStehle
Copy link
Collaborator

SebastianStehle commented Jun 17, 2024

Thanks a lot for your effort :)

Could you report this to the main repository? The last time I have coded C++ was around 2010 ;)

@SebastianStehle
Copy link
Collaborator

@LSViana It seems that it has been solved. I can work on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants