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

OT Editor - Documentation #340

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

OT Editor - Documentation #340

wants to merge 5 commits into from

Conversation

Varun-Sethu
Copy link
Member

Adds documentation for the OT editor 😎

@Varun-Sethu Varun-Sethu requested a review from hdphuong as a code owner March 13, 2023 12:44
backend/editor/OT/README.md Outdated Show resolved Hide resolved
backend/editor/OT/README.md Outdated Show resolved Hide resolved
backend/editor/OT/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@hdphuong hdphuong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm if lgt Garry 😄

Copy link
Member

@angary angary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - just one line that seemed to be missing some words :)

backend/editor/OT/README.md Outdated Show resolved Hide resolved
backend/editor/OT/README.md Outdated Show resolved Hide resolved
backend/editor/OT/README.md Outdated Show resolved Hide resolved
backend/editor/OT/README.md Outdated Show resolved Hide resolved
backend/editor/OT/README.md Outdated Show resolved Hide resolved
backend/editor/OT/docs/operation_parsing.md Outdated Show resolved Hide resolved
backend/editor/OT/docs/operation_parsing.md Outdated Show resolved Hide resolved
backend/editor/OT/docs/operation_parsing.md Show resolved Hide resolved
backend/editor/OT/docs/operation_parsing.md Outdated Show resolved Hide resolved
backend/editor/OT/docs/operation_parsing.md Outdated Show resolved Hide resolved
@Varun-Sethu Varun-Sethu requested a review from ShunyaoLiang April 4, 2023 09:05
Copy link
Member Author

@Varun-Sethu Varun-Sethu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review change

@hanyuone
Copy link
Contributor

hanyuone commented Jun 3, 2023

lgtm, main thing is i noticed in a lot of places the docs say "it's complicated" or "it's complex" - these are fluff sentences that imo not only don't add to documentation/explanation but make it harder for people reading to engage. will provide more detailed review as well

@@ -0,0 +1,163 @@
# The Concurrent Editor
As a high level overview though the concurrent editor is an OT based editor thats heavily influenced by Google's Wave algorithm, Wave was Google's predecessor to Google docs and heavily inspired the architecture for Google Docs. Our editor uses the same underlying architecture as Wave except the difference lies in the type of operations we are sending between the server and client. Wave was designed to operate with operations that modified unstructured data, while our operations modify structured JSON data.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"As a high level overview though, the concurrent editor"

## High Level Architecture
At a very high level our editor server consists of 3 distinct layers.
- **The data layer**
- This layer is the server's copy of the current state of the document. This layer is what all incoming operations will modify. Our initial design for the data layer involved a singular struct modelling the entire document that we modified using reflection. This proved tricky due to the intricacies of Go's reflection system so we moved to an AST based approach. Currently the data layer is just the AST for the JSON of the document, and operations modify this AST directly. To prevent corrupted documents we have various data integrity checks utilising reflection.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it necessary to mention reflection (i.e. is it still in the codebase)? if you really want to include it, it could be in some other document/section, but not here if the old approach is completely removed

I personally feel like its rather easy to understand a system if you understand how it achieves its key features. This section is about what exactly happens when a user clicks the "edit" button on a document and constructs an edit session. What type of objects are created? Where do they live? Etc.

### The connection starts
When the user clicks the "edit" button, this instantiates a HTTP request that's handled by the HTTP handler in `main.go`: `func EditEndpoint(w http.ResponseWriter, r *http.Request)`. This handler takes the incoming request, looks up the requested document and if it exists upgrades the connection to a WebSocket connection. This is important as a WebSocket connection allows for bidirectional communication between the client and server in real time without needing either to poll for updates
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

full stop at the end

Remove
)
```
The above code snippet uniquely defines an operation. Operations take a path to where in the document AST they are being applied (see the paper on tree based transform functions) and the physical operation being applied. The operation being applied (`OperationModel`) is actually an interface and is the reason why parsing operations is more complex than it seems. This interface defines two functions, one for transforming a operation against another operation and one for applying an operation to an AST.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to link the papers in the docs themselves, saves anyone reading having to click back and forth

```
The above code snippet uniquely defines an operation. Operations take a path to where in the document AST they are being applied (see the paper on tree based transform functions) and the physical operation being applied. The operation being applied (`OperationModel`) is actually an interface and is the reason why parsing operations is more complex than it seems. This interface defines two functions, one for transforming a operation against another operation and one for applying an operation to an AST.

The reason why `OperationModel` is an interface is because there are several distinct types of operations we can apply for varying types. Theres different operations for editing an `integer`, `array`, `boolean`, `object` and `string` field. Each of these define their own transformation functions and application logic, so in order to maintain a clean abstraction we use an interface. As an example this is how the `string` operation type implements this interface ![here](../operations/string_operation.go), its a rather intense implementation since it also implements string based transform functions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can get rid of the "it's a rather intense implementation" phrase

},
}
```
the configuration is in essence a mapping between the the `reflect.Type` representation of the interface and the `reflect.type` representation of every struct that "implements it". Implements is in quote as there is no way to statically verify this at compile time, instead if any of these config options are invalid a runtime error will be thrown during parsing. Usage of the `cmsjson` library for the most part is rather simple (thanks to generics in Go :D). An example can be found in the `ParseOperation` function within `operation_model.go`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can include a link to the code with the example

4. the operation is then communicated to all other clients

### The document server wants to propagate operations
If you remember previously how it was mentioned that the document server "propagates" operations to the clients? It does that by sending these operations down a channel maintained by each client
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rewrite sentence, doesn't make sense

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 this pull request may close these issues.

4 participants