Use POST for check_store instead of GET (#313)

The name `check_store`, in particular the use of an active verb “check”,
implies that an action is taken by this endpoint instead of passively
reading a value. For consistency, it should be a `POST` request.

This PR:
- allows it to be used as a `POST` request
- continues to allow it to be used as a `GET` request, but logs a
warning when it is used
- changes the SDK to use a `POST` request
- adds a test that explicitly issues a `GET` request

crates/y-sweet-worker/src/lib.rs

@@ -34,6 +34,7 @@ pub fn router(
     Ok(Router::with_data(context)
         .get("/", |_, _| Response::ok("Y-Sweet!"))
         .get_async("/check_store", check_store_handler)
+        .post_async("/check_store", check_store_handler)
         .post_async("/doc/new", new_doc_handler)
         .post_async("/doc/:doc_id/auth", auth_doc_handler)
         .get_async("/doc/:doc_id/as-update", as_update_handler)

crates/y-sweet/src/server.rs

@@ -334,7 +334,8 @@ impl Server {
     pub fn routes(self: &Arc<Self>) -> Router {
         Router::new()
             .route("/ready", get(ready))
-            .route("/check_store", get(check_store))
+            .route("/check_store", post(check_store))
+            .route("/check_store", get(check_store_deprecated))
             .route("/doc/ws/:doc_id", get(handle_socket_upgrade))
             .route("/doc/new", post(new_doc))
             .route("/doc/:doc_id/auth", post(auth_doc))
@@ -570,6 +571,16 @@ async fn check_store(
     Ok(Json(json!({"ok": true})))
 }
 
+async fn check_store_deprecated(
+    authorization: Option<TypedHeader<headers::Authorization<headers::authorization::Bearer>>>,
+    State(server_state): State<Arc<Server>>,
+) -> Result<Json<Value>, AppError> {
+    tracing::warn!(
+        "GET check_store is deprecated, use POST check_store with an empty body instead."
+    );
+    check_store(authorization, State(server_state)).await
+}
+
 /// Always returns a 200 OK response, as long as we are listening.
 async fn ready() -> Result<Json<Value>, AppError> {
     Ok(Json(json!({"ok": true})))

js-pkg/sdk/src/main.ts

@@ -42,7 +42,7 @@ export class DocumentManager {
   }
 
   public async checkStore(): Promise<CheckStoreResult> {
-    return await (await this.client.request('check_store', 'GET')).json()
+    return await (await this.client.request('check_store', 'POST', {})).json()
   }
 
   /**

tests/src/doc-server.test.ts

@@ -86,10 +86,11 @@ test('get doc as update', async () => {
   await connection.getAsUpdate()
 })
 
-test('get doc as update after delay', async () => {
-  let connection = SERVER.connection()
+// // Slows tests by a lot, so commented by default.
+// test('get doc as update after delay', async () => {
+//   let connection = SERVER.connection()
 
-  await new Promise((resolve) => setTimeout(resolve, 24_000))
+//   await new Promise((resolve) => setTimeout(resolve, 24_000))
 
-  await connection.getAsUpdate()
-}, 30_000)
+//   await connection.getAsUpdate()
+// }, 30_000)

tests/src/index.test.ts

@@ -99,6 +99,18 @@ describe.each(CONFIGURATIONS)(
       expect(result).toEqual({ ok: true })
     })
 
+    test('Check store over GET (deprecated)', async () => {
+      // Note: this tests deprecated behavior.
+      // It will be removed when the behavior is removed.
+      // It's ugly to access a private member like this, but
+      // it's the best way to avoid changing the SDK API for a
+      // test that is temporary anyway.
+      let client = (DOCUMENT_MANANGER as any).client
+
+      let result = await client.request('check_store', 'GET')
+      expect(result.ok).toBe(true)
+    })
+
     test('Create new doc', async () => {
       const result = await DOCUMENT_MANANGER.createDoc()
       expect(typeof result.docId).toBe('string')