update sign stream to getTextOfSign

[Bronson-Brown-deVost]

Is this still relevant for review?

[sjones6]

@Bronson-Brown-deVost : Slow reply ... it's not quite ready for review. Updating it now. Once it's ready, I'll ping.

[coveralls]

Coverage decreased (-6.07%) to 81.977% when pulling 56c5a1e4f805c076968b876f511ace459f7cd072 on update_sign_stream into e733814082f1aba637a694f2daae719badbfff8d on master.

[sjones6]

@Bronson-Brown-deVost : I just need to ensure add/remove sign functionality is still the same.

Hopefully no more changes to deep stuff like this in the near future. Changing up the data structure of the sign stream has caused far-reaching changes through all sorts of editor code.

[sjones6]

@Bronson-Brown-deVost : this is now ready for review. Sorry for so many changes ... the change to the signs caused many changes all over the place.

Typing in a number of signs rapidly still causes some problems; I think that the root of the trouble is that the "next_sign_id" isn't an actual sign ID in the db but rather the UUID for the next sign generate on the client side.

"signs": [
        {
          "sign": "ה",
          "uuid": "33ca919e-8cf1-3e13-aade-a4ffd590486d",
          "previous_sign_id": 1733968,                                                            // < accurate sign ID
          "next_sign_id": "ce60d2e2-b7c1-338f-9d0a-797358a0591d"      // < points to the UUID ...
        },
        {
          "sign": "ו",
          "uuid": "ce60d2e2-b7c1-338f-9d0a-797358a0591d",
          "previous_sign_id": "33ca919e-8cf1-3e13-aade-a4ffd590486d",
          "next_sign_id": "d3d78f56-0dd7-3293-bba7-fe7f79fec786"
        },
        {
          "sign": "ה",
          "uuid": "d3d78f56-0dd7-3293-bba7-fe7f79fec786",
          "previous_sign_id": "ce60d2e2-b7c1-338f-9d0a-797358a0591d",    // < points to the UUID
          "next_sign_id": 998150                                                                             // < accurate sign ID
        }
]

Can you check out this branch and see if you can determine what's going on and possibly patch in a fix server side? Any other ideas on how to do this?

[Bronson-Brown-deVost]

Ugh! Yeah, that is a pain. I think I can alter the logic server-side at some point. The question is, can we declare somehow in the request that several signs in a request are part of a run? Ultimately, I don't think I can assume that when given, say, 4 signs in an add sign request, that they are all sequential and just grab the previous_sign_id of the first entry and the next_sign_id of the last. But for now if you have a sequence of signs being added all in a row, put the real previous_sign_id in the first entry, and put the only real next_sign_id in all entries:

"signs": [
        {
          "sign": "ה",
          "uuid": "33ca919e-8cf1-3e13-aade-a4ffd590486d",
          "previous_sign_id": 1733968,                                                            // < accurate sign ID
          "next_sign_id": 998150      // < points to the eventual ending sign ID
        },
        {
          "sign": "ו",
          "uuid": "ce60d2e2-b7c1-338f-9d0a-797358a0591d",
          "previous_sign_id": "33ca919e-8cf1-3e13-aade-a4ffd590486d", // < I never look at this
          "next_sign_id": 998150      // < points to the eventual ending sign ID
        },
        {
          "sign": "ה",
          "uuid": "d3d78f56-0dd7-3293-bba7-fe7f79fec786",
          "previous_sign_id": "ce60d2e2-b7c1-338f-9d0a-797358a0591d",    // < I never look at this
          "next_sign_id": 998150      // < points to the eventual ending sign ID
        }
]

The server code as I have written it assumes that it is getting a run of signs when it is given more than one sign in a single request (for adding of multiple signs that are not in a run, please just treat each of them as separate "addSigns" transactions). The code grabs the previous_sign_id and next_sign_id of the first sign and inserts it, it then receives the id of the newly created sign and treats that as the previous_sign_id for the next sign in the list (I only ever read the previous_sign_id of the first element in the list of signs). It then continues the procedure till it reaches the end of the array of signs:

...
my $counter = 1;
my $repeatLength = scalar @{$json_post->{signs}};
my $prev_sign_id = 0;
    foreach my $sign (@{$json_post->{signs}}) {
        if ($counter == 1) {
            $prev_sign_id = $sign->{previous_sign_id};
        }
        $prev_sign_id = $cgi->insert_sign($sign->{sign}, $prev_sign_id, $sign->{next_sign_id});
        print "\"$sign->{uuid}\":$prev_sign_id";
        if ($counter != $repeatLength) {
            print "},{";
            $counter++;
        }
    }
...

[Bronson-Brown-deVost]

By the way, at the beginning of my server side code, I can just grab the next_sign_id from the last element in your list of signs to add and then you don't need to make any changes. Perhaps for now I add in to the documentation that "addSigns" takes either a single sign, or a run of signs in sequential order with a real sign_id for the previous_sign_id of the first element and a real sign_id for the next_sign_id of the last element. Technically you would not need to add previous_sign_id or next_sign_id to any other elements (you can put them in, but they are ignored):

"signs": [
        {
          "sign": "ה",
          "uuid": "33ca919e-8cf1-3e13-aade-a4ffd590486d",
          "previous_sign_id": 1733968               // < previous sign ID for the whole run
        },
        {
          "sign": "ו",
          "uuid": "ce60d2e2-b7c1-338f-9d0a-797358a0591d"
        },
        {
          "sign": "ה",
          "uuid": "d3d78f56-0dd7-3293-bba7-fe7f79fec786",
          "next_sign_id": 998150      // < next sign ID for the whole run
        }
]

[Bronson-Brown-deVost]

After a little testing, the server-side patch helps. Sometimes it works right. Other times, the request is made with signs out of sequence and it fails. When adding text to the end of lines, it seems that we don't have representations of breaks, for instance, in the text editor and the post request assumes that the text typed by the user should be placed after those signs. Sometimes the sign-stream seems to be altered in such a way that Ingo's subroutines to return the text of a column can no longer work to get the text and fetching the text of a column stalls. Finally, for now, when it does work we get sometimes possible alternate sign streams returned and that is not yet visible in the text editor interface.

[Bronson-Brown-deVost]

Build tests pass. Coverage decreases for some reason, but I thought it should still pass, right? In any event, let's see if we can smooth out the addSigns issues mentioned above before we pull into master, if that is OK with you.

[sjones6]

@Bronson-Brown-deVost : Thanks for working on that. I'll take a look tonight and see if we can get it nailed down.

Hmmm, yeah, the coverage should just need to stay above 85% ... but I'll write some more tests against this stuff so we don't lose much in any case.

[sjones6]

@Bronson-Brown-deVost : I've fixed up the issue with adding signs. Give it a checkout and attempt to add a few signs in a row, typing at normal-ish speed in a single line. How do things come out?

... now deleting signs seems to be haywire. Not sure what that's about.

[sjones6]

Here's one error I'm seeing:

SyntaxError: Unexpected token A in JSON at position 57

Content-Type: application/json;charset=UTF-8

{"replies":{"1":[{"8b19ba6f-334d-3e0a-bc23-9e6ab1e4e22e":ARRAY(0x7fbf181e1518)}]}}' }

Looks like something related to the array pointer vs array value. Maybe check that the remove multiple signs in one go is alright?

[Bronson-Brown-deVost]

So I just took it for a spin. I think I know the ultimate source of the problem, but not the specific fix. Firstly, take a look at your addSigns transaction.

When I look in my DevTools I see the addSigns transaction being run several times for what looks like the same or similar runs of signs. When I look at the return of getTextOfFragment after adding a run of signs, it too suggests that the adding of signs was somehow duplicated.

As for deleting of signs, when I look at the post request, I see a "removeSigns" transaction, but I also see several "addSigns" transactions. Looking over the response, the "removeSigns" transaction appears to have finished successfully, but the "addSigns" transactions are the ones returning that errant "ARRAY(0x7fbf181e1518)", which indeed is a Perl pointer. Not sure how far I should go diagnosing that now, it is certainly caused by receiving an "addSigns" transaction request without any prev_sign_id or next_sign_id. TODO, add checks to make sure transaction requests have the necessary data to finish and give meaningful error responses (not sure if I will eventually do this in Perl or in the JS data endpoint we spoke about, or both).

[sjones6]

After some debugging last night, I found the issue (partly) deep down: the UUIDs generated on the client-side were not unique. So this was causing problems with adds and deletes because just about everything is based on UUIDs being present in order to track additions and deletes, etc. I patched that up and am pushing the update now.

Things are looking much better now on my end. Adds and deletes are behaving consistently. Ajax requests are debounced to 400ms after last change event ... we could either switch this to throttled or decrease the that to 200ms or something if this causes us troubles.

The persistence service also emits a number of events which will make it easy to wire in a UI component (think google docs where it shows "all changes saved" in the top as you type).

I didn't have a change to add tests yet so the coverage might still be unhappy.

[sjones6]

@Bronson-Brown-deVost : Minus tests, this is ready for final review/approval. I may not be able to address anything until Saturday because I have engagements tonight/tomorrow.

[ikottsi]

Why do you create UUID on the client side? Every session gets its UUID from the server

Am 14.06.2018 um 13:17 schrieb Spencer Jones notifications@github.com<mailto:notifications@github.com>:

After some debugging last night, I found the issue (partly) deep down: the UUIDs generated on the client-side were not unique. So this was causing problems with adds and deletes because just about everything is based on UUIDs being present in order to track additions and deletes, etc. I patched that up and am pushing the update now.

Things are looking much better now on my end. Adds and deletes are behaving consistently. Ajax requests are debounced to 400ms after last change event ... we could either switch this to throttled or decrease the that to 200ms or something if this causes us troubles.

The persistence service also emits a number of events which will make it easy to wire in a UI component (think google docs where it shows "all changes saved" in the top as you type).

I didn't have a change to add tests yet so the coverage might still be unhappy.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/Scripta-Qumranica-Electronica/Scrollery-website/pull/73#issuecomment-397259822, or mute the threadhttps://github.com/notifications/unsubscribe-auth/Ad56SHOcwB7oO1n7lMpMuKZ2o1FDGEg7ks5t8kZngaJpZM4UVRNd.

-- Priv.Doz. Dr. Ingo Kottsieper Forschungsstelle Qumran-Wörterbuch Akademie der Wissenschaften zu Göttingen Friedländer Weg 11 D-37085 Göttingen

//

Evangelische-Theologische Fakultät Alttestamentliches Seminar Westfälische Wilhelms-Universität Münster Universitätsstr. 13-17 D-48143 Münster

Germany

[Bronson-Brown-deVost]

@ikottsi The UUID is a temporary placeholder for new data created on the client side, like a new sign or sign_char. It allows the data model to be altered on the client in real time, the client sends a request to the server to write the new data, then if successful the server sends a message back telling the frontend the actual database ID to replace the temporary client side UUID. Spencer and I can explain it more if you wish.

[sjones6]

Right, it's especially needed when we're generating data on the client side (e.g., the user types in a new sign). Since that doesn't yet have an ID in the database, we use the UUID as a unique identifier so that we can track that bit of data.

[ikottsi]

Thanks, oiw I see the point.

Am 14.06.2018 um 13:48 schrieb Bronson Brown-deVost notifications@github.com<mailto:notifications@github.com>:

@ikottsihttps://github.com/ikottsi The UUID is a temporary placeholder for new data created on the client side, like a new sign or sign_char. It allows the data model to be altered on the client in real time, the client sends a request to the server to write the new data, then if successful the server sends a message back telling the frontend the actual database ID to replace the temporary client side UUID. Spencer and I can explain it more if you wish.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/Scripta-Qumranica-Electronica/Scrollery-website/pull/73#issuecomment-397266749, or mute the threadhttps://github.com/notifications/unsubscribe-auth/Ad56SLzQ8vN1JZRak0Xp18RJU9Zc9jfDks5t8k2dgaJpZM4UVRNd.

-- Priv.Doz. Dr. Ingo Kottsieper Forschungsstelle Qumran-Wörterbuch Akademie der Wissenschaften zu Göttingen Friedländer Weg 11 D-37085 Göttingen

//

Evangelische-Theologische Fakultät Alttestamentliches Seminar Westfälische Wilhelms-Universität Münster Universitätsstr. 13-17 D-48143 Münster

Germany

[Bronson-Brown-deVost]

I knocked our coverage requirement down to 80%. Should be able to pull after retesting.