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

onlyoffice rework #650

Merged
merged 55 commits into from
Oct 2, 2024
Merged

onlyoffice rework #650

merged 55 commits into from
Oct 2, 2024

Conversation

ericlinagora
Copy link
Contributor

@ericlinagora ericlinagora commented Sep 18, 2024

#525 #515 #548 #523

This is a rewrite to have a more robust connexion with only office, recovering forgotten files, handling editing sessions as global locks, that kind

ericlinagora and others added 30 commits August 25, 2024 22:26
@ericlinagora ericlinagora force-pushed the 525-515-548-523-onlyoffice-rework branch from 279694a to e2ec557 Compare September 27, 2024 10:43
getPluginKeyStatus: (key: string) => Promise<ApplicationEditingKeyStatus>;
},
attemptCount = 8,
tarpitS = 1,
Copy link
Member

Choose a reason for hiding this comment

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

tarpit?? reviewing you prs is always an English lesson for me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const swapResult = await provider.atomicSet(newKey, null);
logger.debug(`Begin edit try ${newKey}, got: ${JSON.stringify(swapResult)}`);
if (swapResult.didSet) return newKey;
if (!swapResult.currentValue) continue; // glitch in the matrix but ok because atomicCompareAndSet is not actually completely atomic
Copy link
Member

Choose a reason for hiding this comment

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

Why it's not completely atomic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

oh yeah, it's a current value that is not atomic at all

const userIdBuffer = UUIDTools.bufferFromUUIDString(userId) as unknown as Uint8Array;
const companyIdBuffer = UUIDTools.bufferFromUUIDString(companyId) as unknown as Uint8Array;
const entropyBuffer = UUIDTools.bufferFromUUIDString(randomUUID()) as unknown as Uint8Array;
const idsString = OnlyOfficeSafeDocKeyBase64.fromBuffer(
Copy link
Member

Choose a reason for hiding this comment

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

and also why do we need all this tricky conversions, uuid to hex, and then back to uuid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just using strings with available chars would be bigger than OO allows for key formats. Also because I want editing_session_keys to be sortable by timestamp lexicographically so we can batch work on the older ones

@ericlinagora ericlinagora merged commit b43ef3f into main Oct 2, 2024
7 checks passed
@ericlinagora ericlinagora deleted the 525-515-548-523-onlyoffice-rework branch October 2, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants