GITGUD

neiam neiam /diogramos

diogramos

public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages

⭐

Log in to mark this repository.

Skip an unrenderable point anchor instead of raising on it

e7d9f29 · Gabriel Morell · 2026-09-16 01:52

✓ ci
28 files +677 -11
Message 
{commit_body(@commit)}

Files changed

modified android/app/src/main/java/org/neiam/diogramos/app/canvas/DocumentOps.kt
+10 −9
@@ -289,7 +289,6 @@ object DocumentOps {
289 289 return "invalid_connector"
290 290 }
291 291 if (!elements.containsKey(fromId) || !elements.containsKey(toId)) return "missing_endpoint"
292 − if (!validAnchor(from) || !validAnchor(to)) return "invalid_anchor"
293 292
294 293 connector["routing"]?.stringOrNull()
295 294 ?.let { if (it !in CONNECTOR_ROUTINGS) return "invalid_routing" }
@@ -312,14 +311,16 @@ object DocumentOps {
312 311 return "invalid_connector"
313 312 }
314 313
315 − private fun validAnchor(anchor: JsonObject): Boolean {
316 − val side = anchor["anchor"]?.stringOrNull() ?: return true
317 − if (side in ANCHORS) return true
318 − // A "point" anchor is only valid with coordinates attached.
319 − return side == "point" &&
320 − anchor["x"]?.doubleOrNull() != null &&
321 − anchor["y"]?.doubleOrNull() != null
322 − }
314 + /**
315 + * Mirrors `Document.valid_anchor?/1`, laxness included: every
316 + * map-shaped anchor is accepted, unrecognised anchor names and a
317 + * "point" without coordinates among them. A client stricter than the
318 + * server refuses edits that work on the web, which is worse than
319 + * accepting a shape that simply won't draw — `ConnectorGeometry`
320 + * skips an unrenderable connector on both sides.
321 + */
322 + @Suppress("UNUSED_PARAMETER")
323 + private fun validAnchor(anchor: JsonObject): Boolean = true
323 324
324 325 // --- JSON helpers ------------------------------------------------------
325 326
modified android/app/src/test/resources/document_ops.json
+565 −0
@@ -3364,6 +3364,571 @@
3364 3364 "type": "update_connector"
3365 3365 }
3366 3366 },
3367 + {
3368 + "document": {
3369 + "connectors": {
3370 + "k1": {
3371 + "color": "base-content",
3372 + "dash": "solid",
3373 + "end_gap": 6,
3374 + "from": {
3375 + "anchor": "e",
3376 + "element": "a"
3377 + },
3378 + "id": "k1",
3379 + "label": "",
3380 + "marker_end": "arrow",
3381 + "marker_start": "none",
3382 + "routing": "orthogonal",
3383 + "start_gap": 6,
3384 + "stroke_width": 2,
3385 + "to": {
3386 + "anchor": "w",
3387 + "element": "b"
3388 + }
3389 + },
3390 + "k2": {
3391 + "color": "base-content",
3392 + "dash": "solid",
3393 + "end_gap": 6,
3394 + "from": {
3395 + "anchor": "s",
3396 + "element": "b"
3397 + },
3398 + "id": "k2",
3399 + "label": "",
3400 + "marker_end": "arrow",
3401 + "marker_start": "none",
3402 + "routing": "curve",
3403 + "start_gap": 6,
3404 + "stroke_width": 2,
3405 + "to": {
3406 + "anchor": "n",
3407 + "element": "c"
3408 + }
3409 + }
3410 + },
3411 + "elements": {
3412 + "a": {
3413 + "h": 60,
3414 + "id": "a",
3415 + "label": "A",
3416 + "style": {
3417 + "fill": "primary",
3418 + "stroke": "base-content",
3419 + "stroke_width": 2
3420 + },
3421 + "type": "rect",
3422 + "w": 100,
3423 + "x": 0,
3424 + "y": 0
3425 + },
3426 + "b": {
3427 + "cx": 200,
3428 + "cy": 100,
3429 + "id": "b",
3430 + "r": 30,
3431 + "type": "circle"
3432 + },
3433 + "c": {
3434 + "h": 20,
3435 + "id": "c",
3436 + "type": "text",
3437 + "w": 80,
3438 + "x": 10,
3439 + "y": 200
3440 + }
3441 + },
3442 + "order": [
3443 + "a",
3444 + "b",
3445 + "c"
3446 + ]
3447 + },
3448 + "name": "update connector unknown anchor",
3449 + "ok": true,
3450 + "op": {
3451 + "id": "k1",
3452 + "patch": {
3453 + "from": {
3454 + "anchor": "northwest"
3455 + }
3456 + },
3457 + "type": "update_connector"
3458 + },
3459 + "result": {
3460 + "connectors": {
3461 + "k1": {
3462 + "color": "base-content",
3463 + "dash": "solid",
3464 + "end_gap": 6,
3465 + "from": {
3466 + "anchor": "northwest",
3467 + "element": "a"
3468 + },
3469 + "id": "k1",
3470 + "label": "",
3471 + "marker_end": "arrow",
3472 + "marker_start": "none",
3473 + "routing": "orthogonal",
3474 + "start_gap": 6,
3475 + "stroke_width": 2,
3476 + "to": {
3477 + "anchor": "w",
3478 + "element": "b"
3479 + }
3480 + },
3481 + "k2": {
3482 + "color": "base-content",
3483 + "dash": "solid",
3484 + "end_gap": 6,
3485 + "from": {
3486 + "anchor": "s",
3487 + "element": "b"
3488 + },
3489 + "id": "k2",
3490 + "label": "",
3491 + "marker_end": "arrow",
3492 + "marker_start": "none",
3493 + "routing": "curve",
3494 + "start_gap": 6,
3495 + "stroke_width": 2,
3496 + "to": {
3497 + "anchor": "n",
3498 + "element": "c"
3499 + }
3500 + }
3501 + },
3502 + "elements": {
3503 + "a": {
3504 + "h": 60,
3505 + "id": "a",
3506 + "label": "A",
3507 + "style": {
3508 + "fill": "primary",
3509 + "stroke": "base-content",
3510 + "stroke_width": 2
3511 + },
3512 + "type": "rect",
3513 + "w": 100,
3514 + "x": 0,
3515 + "y": 0
3516 + },
3517 + "b": {
3518 + "cx": 200,
3519 + "cy": 100,
3520 + "id": "b",
3521 + "r": 30,
3522 + "type": "circle"
3523 + },
3524 + "c": {
3525 + "h": 20,
3526 + "id": "c",
3527 + "type": "text",
3528 + "w": 80,
3529 + "x": 10,
3530 + "y": 200
3531 + }
3532 + },
3533 + "order": [
3534 + "a",
3535 + "b",
3536 + "c"
3537 + ]
3538 + }
3539 + },
3540 + {
3541 + "document": {
3542 + "connectors": {
3543 + "k1": {
3544 + "color": "base-content",
3545 + "dash": "solid",
3546 + "end_gap": 6,
3547 + "from": {
3548 + "anchor": "e",
3549 + "element": "a"
3550 + },
3551 + "id": "k1",
3552 + "label": "",
3553 + "marker_end": "arrow",
3554 + "marker_start": "none",
3555 + "routing": "orthogonal",
3556 + "start_gap": 6,
3557 + "stroke_width": 2,
3558 + "to": {
3559 + "anchor": "w",
3560 + "element": "b"
3561 + }
3562 + },
3563 + "k2": {
3564 + "color": "base-content",
3565 + "dash": "solid",
3566 + "end_gap": 6,
3567 + "from": {
3568 + "anchor": "s",
3569 + "element": "b"
3570 + },
3571 + "id": "k2",
3572 + "label": "",
3573 + "marker_end": "arrow",
3574 + "marker_start": "none",
3575 + "routing": "curve",
3576 + "start_gap": 6,
3577 + "stroke_width": 2,
3578 + "to": {
3579 + "anchor": "n",
3580 + "element": "c"
3581 + }
3582 + }
3583 + },
3584 + "elements": {
3585 + "a": {
3586 + "h": 60,
3587 + "id": "a",
3588 + "label": "A",
3589 + "style": {
3590 + "fill": "primary",
3591 + "stroke": "base-content",
3592 + "stroke_width": 2
3593 + },
3594 + "type": "rect",
3595 + "w": 100,
3596 + "x": 0,
3597 + "y": 0
3598 + },
3599 + "b": {
3600 + "cx": 200,
3601 + "cy": 100,
3602 + "id": "b",
3603 + "r": 30,
3604 + "type": "circle"
3605 + },
3606 + "c": {
3607 + "h": 20,
3608 + "id": "c",
3609 + "type": "text",
3610 + "w": 80,
3611 + "x": 10,
3612 + "y": 200
3613 + }
3614 + },
3615 + "order": [
3616 + "a",
3617 + "b",
3618 + "c"
3619 + ]
3620 + },
3621 + "name": "insert connector with an unknown anchor",
3622 + "ok": true,
3623 + "op": {
3624 + "connector": {
3625 + "from": {
3626 + "anchor": "sideways",
3627 + "element": "a"
3628 + },
3629 + "id": "k5",
3630 + "to": {
3631 + "element": "c"
3632 + }
3633 + },
3634 + "type": "insert_connector"
3635 + },
3636 + "result": {
3637 + "connectors": {
3638 + "k1": {
3639 + "color": "base-content",
3640 + "dash": "solid",
3641 + "end_gap": 6,
3642 + "from": {
3643 + "anchor": "e",
3644 + "element": "a"
3645 + },
3646 + "id": "k1",
3647 + "label": "",
3648 + "marker_end": "arrow",
3649 + "marker_start": "none",
3650 + "routing": "orthogonal",
3651 + "start_gap": 6,
3652 + "stroke_width": 2,
3653 + "to": {
3654 + "anchor": "w",
3655 + "element": "b"
3656 + }
3657 + },
3658 + "k2": {
3659 + "color": "base-content",
3660 + "dash": "solid",
3661 + "end_gap": 6,
3662 + "from": {
3663 + "anchor": "s",
3664 + "element": "b"
3665 + },
3666 + "id": "k2",
3667 + "label": "",
3668 + "marker_end": "arrow",
3669 + "marker_start": "none",
3670 + "routing": "curve",
3671 + "start_gap": 6,
3672 + "stroke_width": 2,
3673 + "to": {
3674 + "anchor": "n",
3675 + "element": "c"
3676 + }
3677 + },
3678 + "k5": {
3679 + "color": "base-content",
3680 + "dash": "solid",
3681 + "end_gap": 6,
3682 + "from": {
3683 + "anchor": "sideways",
3684 + "element": "a"
3685 + },
3686 + "id": "k5",
3687 + "label": "",
3688 + "marker_end": "arrow",
3689 + "marker_start": "none",
3690 + "routing": "orthogonal",
3691 + "start_gap": 6,
3692 + "stroke_width": 2,
3693 + "to": {
3694 + "element": "c"
3695 + }
3696 + }
3697 + },
3698 + "elements": {
3699 + "a": {
3700 + "h": 60,
3701 + "id": "a",
3702 + "label": "A",
3703 + "style": {
3704 + "fill": "primary",
3705 + "stroke": "base-content",
3706 + "stroke_width": 2
3707 + },
3708 + "type": "rect",
3709 + "w": 100,
3710 + "x": 0,
3711 + "y": 0
3712 + },
3713 + "b": {
3714 + "cx": 200,
3715 + "cy": 100,
3716 + "id": "b",
3717 + "r": 30,
3718 + "type": "circle"
3719 + },
3720 + "c": {
3721 + "h": 20,
3722 + "id": "c",
3723 + "type": "text",
3724 + "w": 80,
3725 + "x": 10,
3726 + "y": 200
3727 + }
3728 + },
3729 + "order": [
3730 + "a",
3731 + "b",
3732 + "c"
3733 + ]
3734 + }
3735 + },
3736 + {
3737 + "document": {
3738 + "connectors": {
3739 + "k1": {
3740 + "color": "base-content",
3741 + "dash": "solid",
3742 + "end_gap": 6,
3743 + "from": {
3744 + "anchor": "e",
3745 + "element": "a"
3746 + },
3747 + "id": "k1",
3748 + "label": "",
3749 + "marker_end": "arrow",
3750 + "marker_start": "none",
3751 + "routing": "orthogonal",
3752 + "start_gap": 6,
3753 + "stroke_width": 2,
3754 + "to": {
3755 + "anchor": "w",
3756 + "element": "b"
3757 + }
3758 + },
3759 + "k2": {
3760 + "color": "base-content",
3761 + "dash": "solid",
3762 + "end_gap": 6,
3763 + "from": {
3764 + "anchor": "s",
3765 + "element": "b"
3766 + },
3767 + "id": "k2",
3768 + "label": "",
3769 + "marker_end": "arrow",
3770 + "marker_start": "none",
3771 + "routing": "curve",
3772 + "start_gap": 6,
3773 + "stroke_width": 2,
3774 + "to": {
3775 + "anchor": "n",
3776 + "element": "c"
3777 + }
3778 + }
3779 + },
3780 + "elements": {
3781 + "a": {
3782 + "h": 60,
3783 + "id": "a",
3784 + "label": "A",
3785 + "style": {
3786 + "fill": "primary",
3787 + "stroke": "base-content",
3788 + "stroke_width": 2
3789 + },
3790 + "type": "rect",
3791 + "w": 100,
3792 + "x": 0,
3793 + "y": 0
3794 + },
3795 + "b": {
3796 + "cx": 200,
3797 + "cy": 100,
3798 + "id": "b",
3799 + "r": 30,
3800 + "type": "circle"
3801 + },
3802 + "c": {
3803 + "h": 20,
3804 + "id": "c",
3805 + "type": "text",
3806 + "w": 80,
3807 + "x": 10,
3808 + "y": 200
3809 + }
3810 + },
3811 + "order": [
3812 + "a",
3813 + "b",
3814 + "c"
3815 + ]
3816 + },
3817 + "name": "insert connector with a point anchor missing coordinates",
3818 + "ok": true,
3819 + "op": {
3820 + "connector": {
3821 + "from": {
3822 + "anchor": "point",
3823 + "element": "a"
3824 + },
3825 + "id": "k6",
3826 + "to": {
3827 + "element": "c"
3828 + }
3829 + },
3830 + "type": "insert_connector"
3831 + },
3832 + "result": {
3833 + "connectors": {
3834 + "k1": {
3835 + "color": "base-content",
3836 + "dash": "solid",
3837 + "end_gap": 6,
3838 + "from": {
3839 + "anchor": "e",
3840 + "element": "a"
3841 + },
3842 + "id": "k1",
3843 + "label": "",
3844 + "marker_end": "arrow",
3845 + "marker_start": "none",
3846 + "routing": "orthogonal",
3847 + "start_gap": 6,
3848 + "stroke_width": 2,
3849 + "to": {
3850 + "anchor": "w",
3851 + "element": "b"
3852 + }
3853 + },
3854 + "k2": {
3855 + "color": "base-content",
3856 + "dash": "solid",
3857 + "end_gap": 6,
3858 + "from": {
3859 + "anchor": "s",
3860 + "element": "b"
3861 + },
3862 + "id": "k2",
3863 + "label": "",
3864 + "marker_end": "arrow",
3865 + "marker_start": "none",
3866 + "routing": "curve",
3867 + "start_gap": 6,
3868 + "stroke_width": 2,
3869 + "to": {
3870 + "anchor": "n",
3871 + "element": "c"
3872 + }
3873 + },
3874 + "k6": {
3875 + "color": "base-content",
3876 + "dash": "solid",
3877 + "end_gap": 6,
3878 + "from": {
3879 + "anchor": "point",
3880 + "element": "a"
3881 + },
3882 + "id": "k6",
3883 + "label": "",
3884 + "marker_end": "arrow",
3885 + "marker_start": "none",
3886 + "routing": "orthogonal",
3887 + "start_gap": 6,
3888 + "stroke_width": 2,
3889 + "to": {
3890 + "element": "c"
3891 + }
3892 + }
3893 + },
3894 + "elements": {
3895 + "a": {
3896 + "h": 60,
3897 + "id": "a",
3898 + "label": "A",
3899 + "style": {
3900 + "fill": "primary",
3901 + "stroke": "base-content",
3902 + "stroke_width": 2
3903 + },
3904 + "type": "rect",
3905 + "w": 100,
3906 + "x": 0,
3907 + "y": 0
3908 + },
3909 + "b": {
3910 + "cx": 200,
3911 + "cy": 100,
3912 + "id": "b",
3913 + "r": 30,
3914 + "type": "circle"
3915 + },
3916 + "c": {
3917 + "h": 20,
3918 + "id": "c",
3919 + "type": "text",
3920 + "w": 80,
3921 + "x": 10,
3922 + "y": 200
3923 + }
3924 + },
3925 + "order": [
3926 + "a",
3927 + "b",
3928 + "c"
3929 + ]
3930 + }
3931 + },
3367 3932 {
3368 3933 "document": {
3369 3934 "connectors": {
modified docs/android.md
+16 −0
@@ -624,6 +624,22 @@ action that is always refused is worse than no button at all.
624 624 with daisyUI typography; Compose text metrics will not match exactly.
625 625 Wrapping and vertical centering are the likely visible differences. Do we
626 626 accept "close", or does the server need to hand back measured line boxes?
627 +- **Anchor validation is looser than it looks.** `Document.valid_anchor?/1`
628 + ends in a catch-all `%{}` clause, so any map-shaped anchor is accepted —
629 + an unrecognised name like `"northwest"`, or `"point"` with no
630 + coordinates. That last one used to raise `ArithmeticError` inside
631 + `ConnectorGeometry.render/2` (nil coordinates reaching the gap
632 + arithmetic), which took down every render of the canvas holding it.
633 + Harmless while the editor's own UI was the only thing writing ops;
634 + reachable by any client once `/api/v1` and the channel existed.
635 +
636 + `render/2` now skips such a connector, the way it already skips one
637 + whose endpoint is gone, and the Kotlin port matches the server's
638 + leniency rather than being stricter than it — a client that refuses
639 + what the server accepts rejects edits that work on the web. Tightening
640 + `valid_anchor?/1` to reject these at the door is the other half, and
641 + would mean deciding what to do about any already stored.
642 +
627 643 - **`set_z_order` races.** The op replaces the entire `order` list, so two
628 644 peers reordering at once means one silently wins with a stale list. Same
629 645 bug exists on web today; worth noting, not worth fixing in this scope.
modified lib/diogramos/diagrams/connector_geometry.ex
+14 −1
@@ -76,7 +76,7 @@ defmodule Diogramos.Diagrams.ConnectorGeometry do
76 76
77 77 element ->
78 78 case Map.get(anchor, "anchor", "auto") do
79 − "point" -> {:ok, {anchor["x"], anchor["y"]}, {0, 0}}
79 + "point" -> point_anchor(anchor)
80 80 a -> point_for_anchor(element, a, anchor, document)
81 81 end
82 82 end
@@ -84,6 +84,19 @@ defmodule Diogramos.Diagrams.ConnectorGeometry do
84 84
85 85 defp resolve_endpoint(_, _), do: :error
86 86
87 + # A "point" anchor without usable coordinates is skipped rather than
88 + # rendered. `Document.valid_anchor?/1` lets one through -- its
89 + # catch-all map clause accepts any anchor that isn't a well-formed
90 + # point -- and the arithmetic below would then raise on nil, taking
91 + # down whatever is drawing the canvas. Now reachable from /api/v1 and
92 + # the channel, where the op comes from a client rather than from the
93 + # editor's own UI.
94 + defp point_anchor(%{"x" => x, "y" => y}) when is_number(x) and is_number(y) do
95 + {:ok, {x, y}, {0, 0}}
96 + end
97 +
98 + defp point_anchor(_), do: :error
99 +
87 100 defp point_for_anchor(element, "auto", anchor_def, document) do
88 101 other_id = anchor_def["_other"] || nil
89 102 bbox = bbox(element)
modified lib/diogramos/diagrams/document.ex
+3 −1
@@ -362,8 +362,10 @@ defmodule Diogramos.Diagrams.Document do
362 362 defp valid_anchor?(%{"anchor" => "point", "x" => x, "y" => y})
363 363 when is_number(x) and is_number(y), do: :ok
364 364
365 + # Any other map is accepted: an anchor with no "anchor" key defaults to
366 + # "auto" downstream. Note this also lets an unrecognised anchor string
367 + # through -- see the note in docs/android.md section 9.
365 368 defp valid_anchor?(%{}), do: :ok
366 − defp valid_anchor?(_), do: {:error, :invalid_anchor}
367 369
368 370 defp validate_connector_style(connector) do
369 371 routing = Map.get(connector, "routing", "orthogonal")
modified lib/mix/tasks/diogramos.android_fixtures.ex
+27 −0
@@ -365,6 +365,33 @@ defmodule Mix.Tasks.Diogramos.AndroidFixtures do
365 365 }},
366 366 {"update connector invalid routing",
367 367 %{"type" => "update_connector", "id" => "k1", "patch" => %{"routing" => "zigzag"}}},
368 + # The server accepts an unrecognised anchor string -- valid_anchor?/1
369 + # falls through to the catch-all map clause. Pinned so the port
370 + # doesn't get stricter than the thing it mirrors.
371 + {"update connector unknown anchor",
372 + %{
373 + "type" => "update_connector",
374 + "id" => "k1",
375 + "patch" => %{"from" => %{"anchor" => "northwest"}}
376 + }},
377 + {"insert connector with an unknown anchor",
378 + %{
379 + "type" => "insert_connector",
380 + "connector" => %{
381 + "id" => "k5",
382 + "from" => %{"element" => "a", "anchor" => "sideways"},
383 + "to" => %{"element" => "c"}
384 + }
385 + }},
386 + {"insert connector with a point anchor missing coordinates",
387 + %{
388 + "type" => "insert_connector",
389 + "connector" => %{
390 + "id" => "k6",
391 + "from" => %{"element" => "a", "anchor" => "point"},
392 + "to" => %{"element" => "c"}
393 + }
394 + }},
368 395 {"delete connector", %{"type" => "delete_connector", "id" => "k1"}},
369 396 {"delete missing connector", %{"type" => "delete_connector", "id" => "ghost"}},
370 397
modified test/diogramos/diagrams/connector_geometry_test.exs
+42 −0
@@ -104,4 +104,46 @@ defmodule Diogramos.Diagrams.ConnectorGeometryTest do
104 104 # from = east edge of a = (100, 30); to = west edge of b = (300, 230)
105 105 assert rendered.d == "M 100 30 L 300 230"
106 106 end
107 +
108 + describe "malformed anchors" do
109 + test "a point anchor with no coordinates is skipped, not raised on" do
110 + document = doc()
111 +
112 + connector =
113 + connector(%{
114 + "from" => %{"element" => "a", "anchor" => "point"},
115 + "to" => %{"element" => "b"}
116 + })
117 +
118 + # Document.apply_op accepts this shape, so it can be stored -- and
119 + # once /api/v1 and the channel existed, stored by a client rather
120 + # than by the editor. Raising here took out every render of the
121 + # canvas, permanently, for everyone.
122 + assert ConnectorGeometry.render(connector, document) == nil
123 + end
124 +
125 + test "a point anchor with non-numeric coordinates is also skipped" do
126 + document = doc()
127 +
128 + connector =
129 + connector(%{
130 + "from" => %{"element" => "a", "anchor" => "point", "x" => "12", "y" => nil},
131 + "to" => %{"element" => "b"}
132 + })
133 +
134 + assert ConnectorGeometry.render(connector, document) == nil
135 + end
136 +
137 + test "a well-formed point anchor still renders" do
138 + document = doc()
139 +
140 + connector =
141 + connector(%{
142 + "from" => %{"element" => "a", "anchor" => "point", "x" => 5, "y" => 7},
143 + "to" => %{"element" => "b", "anchor" => "w"}
144 + })
145 +
146 + assert %{d: "M 5 7" <> _} = ConnectorGeometry.render(connector, document)
147 + end
148 + end
107 149 end

Parents: 444916c