Access and entity words are core constants: access::*, entity::*
"owner"/"edit"/"view" and the entity names "note"/"label"/"attachment"/"preview" were literals at about thirty sites across store, pull and push, including match arms whose spelling had to agree with the rows a different module wrote. models::access and models::entity now name them, and push names its two ops. SQL text keeps its literals; Rust-side comparisons and writes read the constants. DRY pass #2, batch 2, F7 (#5372). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -5,6 +5,23 @@
|
|||||||
|
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
|
|
||||||
|
/// What this account may do with a note: it is ours, or shared with us to edit or
|
||||||
|
/// only to view. The words the server's sharing uses, and the stored `permission`.
|
||||||
|
pub mod access {
|
||||||
|
pub const OWNER: &str = "owner";
|
||||||
|
pub const EDIT: &str = "edit";
|
||||||
|
pub const VIEW: &str = "view";
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The kinds of row a change names on the wire, and that `pending_deletes` files a
|
||||||
|
/// local delete under until it is sent.
|
||||||
|
pub mod entity {
|
||||||
|
pub const NOTE: &str = "note";
|
||||||
|
pub const LABEL: &str = "label";
|
||||||
|
pub const ATTACHMENT: &str = "attachment";
|
||||||
|
pub const PREVIEW: &str = "preview";
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Serialize)]
|
#[derive(Serialize)]
|
||||||
pub struct Note {
|
pub struct Note {
|
||||||
pub id: String,
|
pub id: String,
|
||||||
|
|||||||
+11
-11
@@ -226,7 +226,7 @@ fn permission_of(conn: &Connection, id: &str) -> rusqlite::Result<String> {
|
|||||||
/// The note's text may change here: it is ours, or shared with us to edit.
|
/// The note's text may change here: it is ours, or shared with us to edit.
|
||||||
fn require_text(conn: &Connection, id: &str) -> rusqlite::Result<String> {
|
fn require_text(conn: &Connection, id: &str) -> rusqlite::Result<String> {
|
||||||
let permission = permission_of(conn, id)?;
|
let permission = permission_of(conn, id)?;
|
||||||
if permission == "view" {
|
if permission == access::VIEW {
|
||||||
return Err(refuse(VIEW_ONLY));
|
return Err(refuse(VIEW_ONLY));
|
||||||
}
|
}
|
||||||
Ok(permission)
|
Ok(permission)
|
||||||
@@ -236,8 +236,8 @@ fn require_text(conn: &Connection, id: &str) -> rusqlite::Result<String> {
|
|||||||
/// owner.
|
/// owner.
|
||||||
fn require_owner(conn: &Connection, id: &str) -> rusqlite::Result<()> {
|
fn require_owner(conn: &Connection, id: &str) -> rusqlite::Result<()> {
|
||||||
match permission_of(conn, id)?.as_str() {
|
match permission_of(conn, id)?.as_str() {
|
||||||
"owner" => Ok(()),
|
access::OWNER => Ok(()),
|
||||||
"view" => Err(refuse(VIEW_ONLY)),
|
access::VIEW => Err(refuse(VIEW_ONLY)),
|
||||||
_ => Err(refuse(OWNER_ONLY)),
|
_ => Err(refuse(OWNER_ONLY)),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -592,7 +592,7 @@ pub fn update_note(conn: &Connection, id: &str, changes: &Value) -> rusqlite::Re
|
|||||||
} else {
|
} else {
|
||||||
permission_of(conn, id)?
|
permission_of(conn, id)?
|
||||||
};
|
};
|
||||||
let owned = permission == "owner";
|
let owned = permission == access::OWNER;
|
||||||
if !owned
|
if !owned
|
||||||
&& obj
|
&& obj
|
||||||
.keys()
|
.keys()
|
||||||
@@ -867,7 +867,7 @@ pub fn delete_attachment(conn: &Connection, id: &str, att_id: &str) -> rusqlite:
|
|||||||
// Only a file the server holds needs a tombstone; without one the next pull would
|
// Only a file the server holds needs a tombstone; without one the next pull would
|
||||||
// bring it straight back. One still waiting to upload leaves the queue with its row.
|
// bring it straight back. One still waiting to upload leaves the queue with its row.
|
||||||
if uploaded == Some(true) {
|
if uploaded == Some(true) {
|
||||||
record_pending_delete(conn, "attachment", att_id)?;
|
record_pending_delete(conn, entity::ATTACHMENT, att_id)?;
|
||||||
}
|
}
|
||||||
touch(conn, id)?;
|
touch(conn, id)?;
|
||||||
load_note(conn, id)
|
load_note(conn, id)
|
||||||
@@ -881,7 +881,7 @@ pub fn delete_preview(conn: &Connection, id: &str, preview_id: &str) -> rusqlite
|
|||||||
)?;
|
)?;
|
||||||
// Previews are made by the server, so every one it has is one it would send back.
|
// Previews are made by the server, so every one it has is one it would send back.
|
||||||
if removed > 0 {
|
if removed > 0 {
|
||||||
record_pending_delete(conn, "preview", preview_id)?;
|
record_pending_delete(conn, entity::PREVIEW, preview_id)?;
|
||||||
}
|
}
|
||||||
touch(conn, id)?;
|
touch(conn, id)?;
|
||||||
load_note(conn, id)
|
load_note(conn, id)
|
||||||
@@ -932,11 +932,11 @@ pub fn delete_forever(conn: &Connection, id: &str) -> rusqlite::Result<()> {
|
|||||||
// is someone else's is not ours to delete.
|
// is someone else's is not ours to delete.
|
||||||
if permission_of(conn, id)
|
if permission_of(conn, id)
|
||||||
.optional()?
|
.optional()?
|
||||||
.is_some_and(|p| p != "owner")
|
.is_some_and(|p| p != access::OWNER)
|
||||||
{
|
{
|
||||||
return Err(refuse(OWNER_ONLY));
|
return Err(refuse(OWNER_ONLY));
|
||||||
}
|
}
|
||||||
record_pending_delete(conn, "note", id)?;
|
record_pending_delete(conn, entity::NOTE, id)?;
|
||||||
conn.execute("DELETE FROM notes WHERE id = ?1", [id])?;
|
conn.execute("DELETE FROM notes WHERE id = ?1", [id])?;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
@@ -990,7 +990,7 @@ pub fn revisions(conn: &Connection, id: &str) -> rusqlite::Result<Vec<NoteRevisi
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn restore_revision(conn: &Connection, id: &str, rev_id: &str) -> rusqlite::Result<Note> {
|
pub fn restore_revision(conn: &Connection, id: &str, rev_id: &str) -> rusqlite::Result<Note> {
|
||||||
let owned = require_text(conn, id)? == "owner";
|
let owned = require_text(conn, id)? == access::OWNER;
|
||||||
let body: String = conn.query_row(
|
let body: String = conn.query_row(
|
||||||
"SELECT body FROM note_revisions WHERE id = ?1 AND note_id = ?2",
|
"SELECT body FROM note_revisions WHERE id = ?1 AND note_id = ?2",
|
||||||
params![rev_id, id],
|
params![rev_id, id],
|
||||||
@@ -1105,7 +1105,7 @@ pub fn set_label_color(conn: &Connection, id: &str, color: &str) -> rusqlite::Re
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn remove_label(conn: &Connection, id: &str) -> rusqlite::Result<()> {
|
pub fn remove_label(conn: &Connection, id: &str) -> rusqlite::Result<()> {
|
||||||
record_pending_delete(conn, "label", id)?;
|
record_pending_delete(conn, entity::LABEL, id)?;
|
||||||
conn.execute("DELETE FROM labels WHERE id = ?1", [id])?;
|
conn.execute("DELETE FROM labels WHERE id = ?1", [id])?;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
@@ -1129,7 +1129,7 @@ pub fn merge_labels(
|
|||||||
WHERE id IN (SELECT note_id FROM note_labels WHERE label_id = ?1)",
|
WHERE id IN (SELECT note_id FROM note_labels WHERE label_id = ?1)",
|
||||||
[source_id],
|
[source_id],
|
||||||
)?;
|
)?;
|
||||||
record_pending_delete(conn, "label", source_id)?;
|
record_pending_delete(conn, entity::LABEL, source_id)?;
|
||||||
conn.execute("DELETE FROM labels WHERE id = ?1", [source_id])?;
|
conn.execute("DELETE FROM labels WHERE id = ?1", [source_id])?;
|
||||||
load_label(conn, target_id)
|
load_label(conn, target_id)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ use super::blobs::BlobStore;
|
|||||||
use super::client;
|
use super::client;
|
||||||
use super::state;
|
use super::state;
|
||||||
use super::wire;
|
use super::wire;
|
||||||
|
use crate::local::models::{access, entity};
|
||||||
use crate::local::{now, Db};
|
use crate::local::{now, Db};
|
||||||
|
|
||||||
/// Backstop against a server that never stops saying `has_more`. At the server's
|
/// Backstop against a server that never stops saying `has_more`. At the server's
|
||||||
@@ -248,9 +249,9 @@ fn upsert_note(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> {
|
|||||||
// never changes, and the server's copy is the same value anyway.
|
// never changes, and the server's copy is the same value anyway.
|
||||||
// A server without `shares` sends no permission, and every note it sends is ours.
|
// A server without `shares` sends no permission, and every note it sends is ours.
|
||||||
let permission = match note.permission.as_deref() {
|
let permission = match note.permission.as_deref() {
|
||||||
Some("edit") => "edit",
|
Some(access::EDIT) => access::EDIT,
|
||||||
Some("view") => "view",
|
Some(access::VIEW) => access::VIEW,
|
||||||
_ => "owner",
|
_ => access::OWNER,
|
||||||
};
|
};
|
||||||
let (shared_by_id, shared_by_name) = match ¬e.shared_by {
|
let (shared_by_id, shared_by_name) = match ¬e.shared_by {
|
||||||
Some(by) => (Some(by.id.as_str()), Some(by.display_name.as_str())),
|
Some(by) => (Some(by.id.as_str()), Some(by.display_name.as_str())),
|
||||||
@@ -330,7 +331,7 @@ fn replace_attachments(conn: &Connection, note: &wire::Note) -> rusqlite::Result
|
|||||||
params![note.id],
|
params![note.id],
|
||||||
)?;
|
)?;
|
||||||
for (index, att) in note.attachments.iter().enumerate() {
|
for (index, att) in note.attachments.iter().enumerate() {
|
||||||
if removed_here(conn, "attachment", &att.id)? {
|
if removed_here(conn, entity::ATTACHMENT, &att.id)? {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
// The server listing an id this device is still waiting to upload means the
|
// The server listing an id this device is still waiting to upload means the
|
||||||
|
|||||||
+23
-18
@@ -15,6 +15,7 @@ use serde::{Deserialize, Serialize};
|
|||||||
use super::blobs::BlobStore;
|
use super::blobs::BlobStore;
|
||||||
use super::client::{self, UploadError};
|
use super::client::{self, UploadError};
|
||||||
use super::state;
|
use super::state;
|
||||||
|
use crate::local::models::{access, entity};
|
||||||
use crate::local::Db;
|
use crate::local::Db;
|
||||||
|
|
||||||
/// The server rejects a batch larger than this (`MAX_PUSH` in `sync.py`).
|
/// The server rejects a batch larger than this (`MAX_PUSH` in `sync.py`).
|
||||||
@@ -61,6 +62,10 @@ impl PushSummary {
|
|||||||
|
|
||||||
// --- outgoing shapes ---------------------------------------------------------
|
// --- outgoing shapes ---------------------------------------------------------
|
||||||
|
|
||||||
|
/// A change's operation on the wire.
|
||||||
|
const UPSERT: &str = "upsert";
|
||||||
|
const DELETE: &str = "delete";
|
||||||
|
|
||||||
/// One entry in the `changes` array. Notes and labels share the envelope; serde skips
|
/// One entry in the `changes` array. Notes and labels share the envelope; serde skips
|
||||||
/// the fields that don't apply, so the server sees exactly the shape docs/sync.md
|
/// the fields that don't apply, so the server sees exactly the shape docs/sync.md
|
||||||
/// describes for each entity.
|
/// describes for each entity.
|
||||||
@@ -108,7 +113,7 @@ impl Change {
|
|||||||
Change {
|
Change {
|
||||||
entity,
|
entity,
|
||||||
id,
|
id,
|
||||||
op: "delete",
|
op: DELETE,
|
||||||
edited_at,
|
edited_at,
|
||||||
..Default::default()
|
..Default::default()
|
||||||
}
|
}
|
||||||
@@ -201,10 +206,10 @@ fn collect_deletes(
|
|||||||
// Only these exist on the wire; anything else is a bug in a writer, and
|
// Only these exist on the wire; anything else is a bug in a writer, and
|
||||||
// shipping it would earn a rejection that no retry could clear.
|
// shipping it would earn a rejection that no retry could clear.
|
||||||
let entity: &'static str = match entity.as_str() {
|
let entity: &'static str = match entity.as_str() {
|
||||||
"note" => "note",
|
entity::NOTE => entity::NOTE,
|
||||||
"label" => "label",
|
entity::LABEL => entity::LABEL,
|
||||||
"attachment" => "attachment",
|
entity::ATTACHMENT => entity::ATTACHMENT,
|
||||||
"preview" => "preview",
|
entity::PREVIEW => entity::PREVIEW,
|
||||||
_ => continue,
|
_ => continue,
|
||||||
};
|
};
|
||||||
out.push(Change::delete(entity, id, deleted_at));
|
out.push(Change::delete(entity, id, deleted_at));
|
||||||
@@ -220,9 +225,9 @@ fn collect_labels(conn: &Connection, out: &mut Vec<Change>, limit: usize) -> rus
|
|||||||
)?;
|
)?;
|
||||||
let rows = stmt.query_map(params![remaining as i64], |r| {
|
let rows = stmt.query_map(params![remaining as i64], |r| {
|
||||||
Ok(Change {
|
Ok(Change {
|
||||||
entity: "label",
|
entity: entity::LABEL,
|
||||||
id: r.get(0)?,
|
id: r.get(0)?,
|
||||||
op: "upsert",
|
op: UPSERT,
|
||||||
name: Some(r.get(1)?),
|
name: Some(r.get(1)?),
|
||||||
color: Some(r.get(2)?),
|
color: Some(r.get(2)?),
|
||||||
edited_at: r.get(3)?,
|
edited_at: r.get(3)?,
|
||||||
@@ -313,15 +318,15 @@ fn note_change(conn: &Connection, id: &str, shared_state: bool) -> rusqlite::Res
|
|||||||
// pin, archive and place once they have been changed here, to a server that keeps
|
// pin, archive and place once they have been changed here, to a server that keeps
|
||||||
// them. Trash, reminders and labels are the owner's, and this account's labels
|
// them. Trash, reminders and labels are the owner's, and this account's labels
|
||||||
// were never on it.
|
// were never on it.
|
||||||
if row.permission != "owner" {
|
if row.permission != access::OWNER {
|
||||||
let state_at = row.state_at.filter(|_| shared_state);
|
let state_at = row.state_at.filter(|_| shared_state);
|
||||||
let own = state_at.is_some();
|
let own = state_at.is_some();
|
||||||
return Ok(Change {
|
return Ok(Change {
|
||||||
entity: "note",
|
entity: entity::NOTE,
|
||||||
id: id.to_string(),
|
id: id.to_string(),
|
||||||
op: "upsert",
|
op: UPSERT,
|
||||||
edited_at: row.updated_at,
|
edited_at: row.updated_at,
|
||||||
body: (row.permission == "edit").then_some(row.body),
|
body: (row.permission == access::EDIT).then_some(row.body),
|
||||||
pinned: own.then_some(row.pinned),
|
pinned: own.then_some(row.pinned),
|
||||||
archived: own.then_some(row.archived),
|
archived: own.then_some(row.archived),
|
||||||
position: own.then_some(row.position),
|
position: own.then_some(row.position),
|
||||||
@@ -341,9 +346,9 @@ fn note_change(conn: &Connection, id: &str, shared_state: bool) -> rusqlite::Res
|
|||||||
};
|
};
|
||||||
|
|
||||||
Ok(Change {
|
Ok(Change {
|
||||||
entity: "note",
|
entity: entity::NOTE,
|
||||||
id: id.to_string(),
|
id: id.to_string(),
|
||||||
op: "upsert",
|
op: UPSERT,
|
||||||
// The local `updated_at` IS the client's edit time, which is what the
|
// The local `updated_at` IS the client's edit time, which is what the
|
||||||
// server's last-write-wins comparison runs against.
|
// server's last-write-wins comparison runs against.
|
||||||
edited_at: row.updated_at,
|
edited_at: row.updated_at,
|
||||||
@@ -387,7 +392,7 @@ pub fn apply_results(
|
|||||||
} else {
|
} else {
|
||||||
summary.applied += 1;
|
summary.applied += 1;
|
||||||
}
|
}
|
||||||
if change.op == "delete" {
|
if change.op == DELETE {
|
||||||
forget_pending_delete(&tx, change)?;
|
forget_pending_delete(&tx, change)?;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -403,7 +408,7 @@ pub fn apply_results(
|
|||||||
// lose to the same comparison forever — and let the next pull bring
|
// lose to the same comparison forever — and let the next pull bring
|
||||||
// the server's copy down.
|
// the server's copy down.
|
||||||
clear_dirty(&tx, change, None)?;
|
clear_dirty(&tx, change, None)?;
|
||||||
if change.op == "delete" {
|
if change.op == DELETE {
|
||||||
// Our delete lost to a newer server edit; the note lives on, and
|
// Our delete lost to a newer server edit; the note lives on, and
|
||||||
// the pull will restore it locally. Drop the tombstone so we
|
// the pull will restore it locally. Drop the tombstone so we
|
||||||
// don't keep trying to delete a note the user has since edited.
|
// don't keep trying to delete a note the user has since edited.
|
||||||
@@ -448,11 +453,11 @@ pub fn apply_results(
|
|||||||
|
|
||||||
fn clear_dirty(conn: &Connection, change: &Change, revision: Option<i64>) -> rusqlite::Result<()> {
|
fn clear_dirty(conn: &Connection, change: &Change, revision: Option<i64>) -> rusqlite::Result<()> {
|
||||||
// A delete has no local row left to update.
|
// A delete has no local row left to update.
|
||||||
if change.op == "delete" {
|
if change.op == DELETE {
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
let table = match change.entity {
|
let table = match change.entity {
|
||||||
"label" => "labels",
|
entity::LABEL => "labels",
|
||||||
_ => "notes",
|
_ => "notes",
|
||||||
};
|
};
|
||||||
match revision {
|
match revision {
|
||||||
@@ -469,7 +474,7 @@ fn clear_dirty(conn: &Connection, change: &Change, revision: Option<i64>) -> rus
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn forget_pending_delete(conn: &Connection, change: &Change) -> rusqlite::Result<()> {
|
fn forget_pending_delete(conn: &Connection, change: &Change) -> rusqlite::Result<()> {
|
||||||
if change.op != "delete" {
|
if change.op != DELETE {
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
conn.execute(
|
conn.execute(
|
||||||
|
|||||||
Reference in New Issue
Block a user