Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 59 additions & 7 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,13 @@ export interface RegisterOptions {
* Using an improper one will results in the registration being rejected.
*/
deviceDesc?:
| "desktop-windows"
| "desktop-macos"
| "desktop-linux"
| "mobile-android"
| "mobile-ios"
| "browser-chrome"
| "remarkable";
| "desktop-windows"
| "desktop-macos"
| "desktop-linux"
| "mobile-android"
| "mobile-ios"
| "browser-chrome"
| "remarkable";
/**
* the unique id of this device
*
Expand Down Expand Up @@ -634,6 +634,18 @@ export interface RemarkableApi {
*/
delete(hash: string, refresh?: boolean): Promise<HashEntry>;


/**
* permanently delete an entry from all devices connected to the account
*
* @example
* ```ts
* await api.purge(file.hash);
* ```
* @param hash - the hash of the entry to delete
*/
purge(hash: string, refresh?: boolean): Promise<void>;

/**
* rename an entry
*
Expand Down Expand Up @@ -694,6 +706,21 @@ export interface RemarkableApi {
refresh?: boolean,
): Promise<HashesEntry>;

/**
* delete many entries permanently from all devices connected to the account
*
* @example
* ```ts
* await api.bulkPurge([file.hash]);
* ```
*
* @param hashes - the hashes of the entries to delete
*/
bulkPurge(
hashes: readonly string[],
refresh?: boolean,
): Promise<[SimpleEntry[], string]>;

/**
* get the current cache value as a string
*
Expand Down Expand Up @@ -1308,6 +1335,11 @@ class Remarkable implements RemarkableApi {
async delete(hash: string, refresh: boolean = false): Promise<HashEntry> {
return await this.move(hash, "trash", refresh);
}

/** permanently delete an entry */
async purge(hash: string, refresh: boolean = false): Promise<void> {
Copy link
Owner

Choose a reason for hiding this comment

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

I would make this have a similar return type, e.g. [SimpleEntry, string] although I left a comment on the bulk purge return type as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I did not return the simpleentry is because it no longer exists, but it would be a simple change.

Copy link
Owner

Choose a reason for hiding this comment

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

yeah, lets wait until the more important details are hashed out.

await this.bulkPurge([hash], refresh);
}

/** rename an entry */
async rename(
Expand Down Expand Up @@ -1381,6 +1413,26 @@ class Remarkable implements RemarkableApi {
return await this.bulkMove(hashes, "trash", refresh);
}

/** permanent delete many hashes */
async bulkPurge(
hashes: readonly string[],
refresh: boolean = false,
): Promise<[SimpleEntry[], string]> {
Copy link
Owner

Choose a reason for hiding this comment

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

the similar methods return the entries that were modified. Similarly it seems to make sense to return something relating to the entries that were actually deleted. These don't tend to return the root hash.

This API has evolved over a bit of time, so I'm open to an argument of why it helps to return something different, but baring that, I lean towards a consistent interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no strong feelings on the return type. I was thinking about what could be useful data to return in this scenario and the only thing I could come up with was the root hash.

Copy link
Owner

Choose a reason for hiding this comment

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

One benefit of doing something like the the other returns is that it'll tell you which were actually removed, which would indicate that maybe things were already purged.

const [rootHash, generation] = await this.#getRootHash(refresh);
// Get the raw text of the root entry
const entries = await this.raw.getEntries(rootHash)
const newEntries = entries.filter(
(entry) => !hashes.includes(entry.hash));
Comment on lines +1424 to +1425
Copy link
Owner

Choose a reason for hiding this comment

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

I would make hashes a Set so this check is O(1) not O(n)

// If we didn't delete anything, just return the current root
if (newEntries.length < entries.length) {
const hash = this.raw.makeListHash(newEntries);
await this.#putRootHash(hash, generation);
Comment on lines +1428 to +1429
Copy link
Owner

Choose a reason for hiding this comment

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

from this, it looks like your setting the roothash to a raw entries file, rather than the hash to a file. I've never seen a rootHash specified this way. It may work, but if writing the entries first and then writing that hash to that file also works, I strongly lean towards that option.

I also want you to be careful since this api while somewaht being non-destructive, can be destructive if you're not careful, also ideally keep backups of any documents, and a backup of the current rootHash before attempting any of this.

In short, I'm skeptical that this is the right thing to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am doing this exactly like the remarkable app does, I added fiddler to my system in order to make sure I did it the same way. I can double check and possibly make a dump of the files so there is nothing I have missed.

Copy link
Owner

Choose a reason for hiding this comment

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

yeah, I guess try double checking. Does the rest of the api work as intended? If so, I can't imagine that the root hash would change from a hash to a raw file.

Looking at the beginning of your function, the rootHash is a normal hash. You pass that into raw.getEntries which reads the entries list at that hash. If that step works, then presumably rootHash should contain a hash and not the full entries list. If root hash did contain an entries file, then raw.getEntries should fail, probably at validation before even making the request since I think it checks for a hash of the appropriate length.

return [newEntries, hash];
}
return [newEntries, rootHash];

}

/** dump the raw cache */
dumpCache(): string {
return this.raw.dumpCache();
Expand Down
22 changes: 17 additions & 5 deletions src/raw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,12 @@ export interface RawRemarkableApi {
metadata: Metadata,
): Promise<[RawFileEntry, Promise<void>]>;

/**
* create a list hash from a set of entries
* @param entries - the entries to hash
*/
makeListHash(entries: RawEntry[]): string;
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think there's much utility in exposing this, and it doesn't rely on the state of raw, so I'd move to a utility function. It could still be placed within this file, I just wouldn't put it in the RawApi.

That said, I wouldn't make any changes until there's agreement on the rootHash, as this would be moot.


/**
* put a set of entries to make an entry list file
*
Expand Down Expand Up @@ -1145,6 +1151,14 @@ export class RawRemarkable implements RawRemarkableApi {
}
}

makeListHash(entries: RawEntry[]): string {
const records = ["3\n"];
for (const { hash, type, id, subfiles, size } of entries) {
records.push(`${hash}:${type}:${id}:${subfiles}:${size}\n`);
}
return records.join("");
}

async putEntries(
id: string,
entries: RawEntry[],
Expand All @@ -1161,10 +1175,8 @@ export class RawRemarkable implements RawRemarkableApi {
const hash = await digest(hashBuff);
const size = entries.reduce((acc, ent) => acc + ent.size, 0);

const records = ["3\n"];
for (const { hash, type, id, subfiles, size } of entries) {
records.push(`${hash}:${type}:${id}:${subfiles}:${size}\n`);
}
const listHash = this.makeListHash(entries);

const res: RawListEntry = {
id,
hash,
Expand All @@ -1176,7 +1188,7 @@ export class RawRemarkable implements RawRemarkableApi {
return [
res,
// NOTE when monitoring requests, this had the extension .docSchema appended, but I'm not entirely sure why
this.#putFile(hash, `${id}.docSchema`, enc.encode(records.join(""))),
this.#putFile(hash, `${id}.docSchema`, enc.encode(listHash)),
];
}

Expand Down