Summary:
- Implements MemoryPool for waiting on memory reservations.
- Uses MemoryPool to control memory used for stringifying JSON responses in Client.ts
- Limits total size of _missedMessages that may be queued for a particular client.
- Upgrades ws library, which may reduce memory usage, and allows pausing the websocket for testing.
- The upgrade changed subtle behavior corners, requiring various fixes to code and tests.
- dos.ts:
- Includes Paul's fixes and updates to the dos.ts script for manual stress-testing.
- Logging tweaks, to avoid excessive dumps on uncaughtError, and include timestamps.
Test Plan:
- Includes a test that measures heap size, and fails without memory management.
- Includes a unittest for MemoryPool
- Some cleanup and additions to TestServer helper; in particular adds makeUserApi() helper used in multiple tests.
- Some fixes related to ws upgrade.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3974
Summary: Column and widget descriptions now support links in text.
Test Plan: Updated
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3981
Summary: Also fixes a few bugs with some telemetry events not being recorded.
Test Plan: Manual.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3960
* Fix support of Scaleway S3 bucket #359
While MinIO and AWS return versionId as strings, other S3 API
implementations return versionId as integers.
We must carefully convert the versionId as string in order to cover
these various behaviors.
Also ensure that docStorage is initialized before attempting to
calculate the data size in order to avoid an exception.
* Add unit tests for MinIOExternalStorage#versions() #359
Introduced some unit tests to :
- ensure listObjects is called with the right arguments;
- cover the case when a S3 bucket implementation does not return the
versionId as a string but rather as an integer (like Scaleway):
in such a case, ensure that the returned snapshotId is a string;
- cover the case when the listObjects function emits an error, ensure the
versions() call rejets with the error emitted;
- that the deleteMarkers are only returned when the
includeDeleteMarkers is passed;
---------
Co-authored-by: Florent FAYOLLE <florent.fayolle@beta.gouv.fr>
Summary:
introduces POST /api/docs/{docId}/webhooks and DELETE /api/docs/{docId}/webhooks/{webhookId} on place of old _subscribe and _unsubscribe endpoints.
Remove checking for unsubscribeKey while deleting webhook - only owner can delete webhook using DELETE endpoint. subscription key is still needed for _unsubscribe endpoint.
old _unsubscribe and _subscribe endpoints are still active and work as before - no changes there.
Posting schema:
```
POST /api/docs/[docId]/webhooks
```
Request Body:
```
{
"webhooks": [
{
"fields": {
"url": "https://webhook.site/3bd02246-f122-445e-ba7f-bf5ea5bb6eb1",
"eventTypes": [
"add",
"update"
],
"enabled": true,
"name": "WebhookName",
"memo": "just a text",
"tableId": "Table1"
}
},
{
"fields": {
"url": "https://webhook.site/3bd02246-f122-445e-ba7f-bf5ea5bb6eb2",
"eventTypes": [
"add",
],
"enabled": true,
"name": "OtherWebhookName",
"memo": "just a text",
"tableId": "Table1"
}
}
]
}
```
Expected response: WebhookId for each webhook posted:
```
{
"webhooks": [
{
"id": "85c77108-f1e1-4217-a50d-acd1c5996da2"
},
{
"id": "d87a6402-cfd7-4822-878c-657308fcc8c3"
}
]
}
```
Deleting webhooks:
```
DELETE api/docs/[docId]/webhooks/[webhookId]
```
there is no payload in DELETE request. Therefore only one webhook can be deleted at once
Response:
```
{
"success": true
}
```
Test Plan: Old unit test improved to handle new endpoints, and one more added to check if endpoints are in fact created/removed
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: paulfitz, alexmojaki
Differential Revision: https://phab.getgrist.com/D3916
Summary:
Adds a new Support Grist page (accessible only in grist-core), containing
options to opt in to telemetry and sponsor Grist Labs on GitHub.
A nudge is also shown in the doc menu, which can be collapsed or permanently
dismissed.
Test Plan: Browser and server tests.
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz
Subscribers: jarek, dsagal
Differential Revision: https://phab.getgrist.com/D3926
Summary:
This uses a newer version of mocha in grist-core so that tests can be run in parallel. That allows more tests to be moved without slowing things down overall. Tests moved are venerable browser tests; only the ones that "just work" or worked without too much trouble to are moved, in order to keep the diff from growing too large. Will wrestle with more in follow up.
Parallelism is at the file level, rather than the individual test.
The newer version of mocha isn't needed for grist-saas repo; tests are parallelized in our internal CI by other means. I've chosen to allocate files to workers in a cruder way than our internal CI, based on initial characters rather than an automated process. The automated process would need some reworking to be compatible with mocha running in parallel mode.
Test Plan: this diff was tested first on grist-core, then ported to grist-saas so saas repo history will correctly track history of moved files.
Reviewers: jarek
Reviewed By: jarek
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3927
Summary: Also fixes a few small bugs with telemetry collection.
Test Plan: Server and manual tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3915
Summary:
Adds support for optional telemetry to grist-core.
A new environment variable, GRIST_TELEMETRY_LEVEL, controls the level of telemetry collected.
Test Plan: Server and unit tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal, anaisconce
Differential Revision: https://phab.getgrist.com/D3880
Summary:
sanitazing errors output in webhooks to protect users data (not show them in logs and other places).
Because redis is returing whole payload when error occur, best approach is to hijack exception as close to redis operation as posible and sanitize the data.
We need to know data structure do do this corretly tho. Currently I decided to just censore everything that has "payload" key.
Test Plan: Because logs that need to be sanitized come from redis, to be valid tested we should force redis to crash. It's hard to do in our integration test setup. In this moment, unit test is all we got.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3905
Summary:
Adding a way to detach an editor. Initially only implemented for the formula editor, includes redesign for the AI part.
- Initially, the detached editor is tight with the formula assistant and both are behind GRIST_FORMULA_ASSISTANT flag, but this can be relaxed
later on, as the detached editor can be used on its own.
- Detached editor is only supported in regular fields and on the creator panel. It is not supported yet for conditional styles, due to preview limitations.
- Old code for the assistant was removed completely, as it was only a temporary solution, but the AI conversation part was copied to the new one.
- Prompting was not modified in this diff, it will be included in the follow-up with more test cases.
Test Plan: Added only new tests; existing tests should pass.
Reviewers: JakubSerafin
Reviewed By: JakubSerafin
Differential Revision: https://phab.getgrist.com/D3863
Summary:
Migrations were failing in snapshots due to the sandbox no longer
being started in snapshots. We now start up an instance of the
sandbox whenever there are migrations to run, and immediately shut
it down on completion.
Test Plan: Server test.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3898
Summary:
Also:
- Move ProxyAgent to from app/server/utils to app/server/lib, which is
the more usual place for such classes.
- Refactor a helper (delayAbort) that node was reporting a leak in.
Test Plan: Added a test case, and tested manually.
Reviewers: JakubSerafin
Reviewed By: JakubSerafin
Subscribers: JakubSerafin, paulfitz
Differential Revision: https://phab.getgrist.com/D3897
Summary:
This adds a UI panel for managing webhooks. Work started by Cyprien Pindat. You can find the UI on a document's settings page. Main changes relative to Cyprien's demo:
* Changed behavior of virtual table to be more consistent with the rest of Grist, by factoring out part of the implementation of on-demand tables.
* Cell values that would create an error can now be denied and reverted (as for the rest of Grist).
* Changes made by other users are integrated in a sane way.
* Basic undo/redo support is added using the regular undo/redo stack.
* The table list in the drop-down is now updated if schema changes.
* Added a notification from back-end when webhook status is updated so constant polling isn't needed to support multi-user operation.
* Factored out webhook specific logic from general virtual table support.
* Made a bunch of fixes to various broken behavior.
* Added tests.
The code remains somewhat unpolished, and behavior in the presence of errors is imperfect in general but may be adequate for this case.
I assume that we'll soon be lifting the restriction on the set of domains that are supported for webhooks - otherwise we'd want to provide some friendly way to discover that list of supported domains rather than just throwing an error.
I don't actually know a lot about how the front-end works - it looks like tables/columns/fields/sections can be safely added if they have string ids that won't collide with bone fide numeric ids from the back end. Sneaky.
Contains a migration, so needs an extra reviewer for that.
Test Plan: added tests
Reviewers: jarek, dsagal
Reviewed By: jarek, dsagal
Differential Revision: https://phab.getgrist.com/D3856
Summary:
- Webhooks form Triggers.ts should now use proxy if it's configured
- Proxy handling code separated to ProxyAgent.ts
- Tests for ProxyAgent
- Integration/API Tests for using Proxy in webhooks
- a bit of refactor - proxy test uses mostly the same codebase as DocApi.ts, but because last one if over 4000 lines long, I've put it into separated file, and extract some common parts (there is some duplicates tho)
- some cleanup in files that I've touched
Test Plan:
Manual test to check if proxy is used on the staging env
Automatic test checking if (fake) proxy was called
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3860
Summary:
Adds a new endpoint to update webhook.
Perform some refactoring to allow code reuse from endpoint allowing to _subscribe and _unsubscribe webhooks.
One aspect of webhook is that url are stored in the home db while the rest of the fields (tableRef, isReadyColRef, ...) are stored in sqlite. So care must be taken when updating fields, to properly rollback if anything should fail.
Follow up diff will bring UI to edit webhook list
Test Plan: Updated doc api server tests
Reviewers: jarek
Reviewed By: jarek
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3821
Summary:
Porting script that run an evaluation against our formula dataset.
To test you need an openai key (see here: https://platform.openai.com/)
or hugging face (it should work as well), then checkout the branch and run
`OPENAI_API_KEY=<my_openai_api_key> node core/test/formula-dataset/runCompletion.js`
Test Plan:
Needs manually testing: so far there is no plan to make it part of CI.
The current score is somewhere around 34 successful prompts over a total of 47.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3816
Summary:
Adds information about forks to the home db. This will be used
later by the UI to list forks of documents.
Test Plan: Browser and server tests.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3772
There was no script for updating typescript schema information after
a python-based document migration. Moving one in here, along with its
test. Tweaked the code slightly to work with grist-core's directory
structure. Also fixed a formatting error in mocha calls that was resulting
in some root tests not running.
Summary:
By default editor inherits permission for a new document or workspace.
Now editor is added explicitly as an owner of a new doc or workspace.
Test Plan: Updated
Reviewers: georgegevoian, paulfitz
Reviewed By: georgegevoian, paulfitz
Subscribers: dsagal, paulfitz
Differential Revision: https://phab.getgrist.com/D3734
Summary: This test has been added to core. This diff also updates some storage-related code in monorepo to match changes in core.
Test Plan: moving test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3746
Summary:
for users who don't automatically have deep rights
to the document, provide them with attachment metadata only
for rows they have access to. This is a little tricky to
do efficiently. We provide attachment metadata when an
individual table is fetched, rather than on initial document
load, so we don't block that load on a full document scan.
We provide attachment metadata to a client when we see that
we are shipping rows mentioning particular attachments,
without making any effort to keep track of the metadata they
already have.
Test Plan: updated tests
Reviewers: dsagal, jarek
Reviewed By: dsagal, jarek
Differential Revision: https://phab.getgrist.com/D3722
Summary:
Checking SCHEMA_EDIT permission when user wants to
update document's name.
Test Plan: New test
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3733
Summary:
- New /webhooks event that lists all webhooks in a document (available for owners),
- Monitoring webhooks usage and saving it in memory or Redis,
- Loosening _usubscribe API endpoint, so that the information returned from the /webhook endpoint is enough to unsubscribe,
- Owners can remove webhook without the unsubscribe key.
The endpoint lists all webhooks that are registered in a document, not just webhooks from a single table.
There are two status fields. First for the webhook, second for the last request attempt.
Webhook can have 5 statuses: 'idle', 'sending', 'retrying', 'postponed', 'error', which roughly describes what the
sendLoop is currently doing. The 'error' status describes a situation when all request attempts failed and the queue needs
to be drained, so some requests were dropped.
The last request status can only be: 'success', 'failure' or 'rejected'. Rejected means that the last batch was dropped because the
queue was too long.
Test Plan: New and updated tests
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3727
Summary:
Creating an API endpoint to cancel any queued webhook messages from
a document.
Test Plan: Updated
Reviewers: paulfitz, georgegevoian
Reviewed By: paulfitz, georgegevoian
Differential Revision: https://phab.getgrist.com/D3713
Summary:
Guest editors added to a document were able to remove it. This limits this permission
by allowing only owners of a doc to delete it.
Test Plan: Updated
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal, anaisconce
Differential Revision: https://phab.getgrist.com/D3708
Summary:
The /replace endpoint was built with home-level access control in mind. Updates needed:
* Only an owner can now replace a document. Only owners are permitted to change granular access rules, and a document replacement could change granular access rules.
* For the document being substituted in: the user must have complete access to view all material within it.
Test Plan: extended test
Reviewers: georgegevoian, dsagal
Reviewed By: georgegevoian, dsagal
Differential Revision: https://phab.getgrist.com/D3694
Summary:
Allow requests from untrusted origins instead of returning an error, but don't allow credentials (Cookie header) or API keys (Authorization header).
Allow setting the header `Content-type: application/json` as an alternative to `X-Requested-With: XMLHttpRequest` to make it easier for clients to make POST/PUT/PATCH/DELETE requests without authentication.
Discussion: https://grist.slack.com/archives/C0234CPPXPA/p1666355281535479
Test Plan: Added and updated DocApi tests. Tested manually how this affects requests made from a browser.
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz, dsagal
Differential Revision: https://phab.getgrist.com/D3678
Summary:
Moving bulk of nbrowser tests to core. Some tests were split and only part of them were moved.
Tests that are left are either: not suitable for grist-core (like billing) or are failing during browser tests (are not reliable).
Four fixtures directory (uploads, docs, exports-csv/excel) where completely moved to grist-core and are linked as folders.
Those changes allows to add an nbrowser test in grist-core or in the main test folder without any need to link it or link a fixture document.
Other changes:
- testrun.sh has been modified, now it runs tests from both folders (test and core/test),
- TestServer used in grist-core is now adding sample orgs and users (kiwi and others),
Test modified
- SelectionSummary: now it is run on a bigScreen, it was failing randomly
- Billing.ts: relative paths were used
- DateEditor: added waitForServer - it was failing in browser mode
- FrozenColumns, ImportFromGDrive, Printing: updated import paths
- UserManager.ts: was split into two parts (it assumed limited products)
- ViewLayoutResize.ts: this test is still in main repo, it is still failing in browser mode tests
Test Plan: Existing
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal, paulfitz
Differential Revision: https://phab.getgrist.com/D3664
Summary:
- DocApi test for Allowed Origin was using a home server endpoint
- Fixing waitForServer, as gristApp can be unavailable for a moment when browser is refreshed
- Fixing MergedOrgs tests typing issue
Test Plan: Updated
Reviewers: cyprien, paulfitz
Reviewed By: cyprien, paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3648
Summary:
This diff adds a new `BulkAddOrUpdateRecord` user action which is what is sounds like:
- A bulk version of the existing `AddOrUpdateRecord` action.
- Much more efficient for operating on many records than applying many individual actions.
- Column values are specified as maps from `colId` to arrays of values as usual.
- Produces bulk versions of `AddRecord` and `UpdateRecord` actions instead of many individual actions.
Examples of users wanting to use something like `AddOrUpdateRecord` with large numbers of records:
- https://grist.slack.com/archives/C0234CPPXPA/p1651789710290879
- https://grist.slack.com/archives/C0234CPPXPA/p1660743493480119
- https://grist.slack.com/archives/C0234CPPXPA/p1660333148491559
- https://grist.slack.com/archives/C0234CPPXPA/p1663069291726159
I tested what made many `AddOrUpdateRecord` actions slow in the first place. It was almost entirely due to producing many individual `AddRecord` user actions. About half of that time was for processing the resulting `AddRecord` doc actions. Lookups and updates were not a problem. With these changes, the slowness is gone.
The Python user action implementation is more complex but there are no surprises. The JS API now groups `records` based on the keys of `require` and `fields` so that `BulkAddOrUpdateRecord` can be applied to each group.
Test Plan: Update and extend Python and DocApi tests.
Reviewers: jarek, paulfitz
Reviewed By: jarek, paulfitz
Subscribers: jarek
Differential Revision: https://phab.getgrist.com/D3642
Summary:
This endpoint has started to fail when called between a pair
of doc workers. The simplest fix is to simply remove it, it serves no
purpose.
Test Plan: added basic deployment test
Reviewers: dsagal
Reviewed By: dsagal
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3636
XLSX export of active view / table
Co-authored-by: Louis Delbosc <louis.delbosc.prestataire@anct.gouv.fr>
Co-authored-by: Vincent Viers <vincent.viers@beta.gouv.fr>
Summary: This particular test fails often enough to be annoying but not often enough to be worrying. It's not clear why, but it seems like a race condition involving redis. Fixing the test 'properly' seems hard and not worth the effort. Looking at the past 20 Jenkins builds, I found this test failing once. If we assume that the probability of failing is 1/20 (it's probably less since the test actually runs 3 times with different server configurations) then the probability of failing 3 times independently is (1/20)^3 = 1/8000, so `this.retry(3)` seems like a good enough solution. See also: https://grist.slack.com/archives/C0234CPPXPA/p1652909955773049
Test Plan: Manually made the test fail randomly 90% of the time, in which case `this.retries(3)` was not usually enough to prevent failures, but `this.retries(300)` was.
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3595
Summary: `nbrowser/CopyPaste` was failing on Mac. Diff fixes that issue.
Test Plan: Update `test/nbrowser/CopyPaste` and `core/test/server/customUtil.ts`.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3552
Summary:
This adds rudimentary support for opening certain SQLite files in Grist.
If you have a file such as `landing.db` in Grist, you can convert it to Grist format by doing (either in monorepo or grist-core):
```
yarn run cli -h
yarn run cli sqlite -h
yarn run cli sqlite gristify landing.db
```
The file is now openable by Grist. To actually do so with the regular Grist server, you'll need to either import it, or convert some doc you don't care about in the `samples/` directory to be a soft link to it (and then force a reload).
This implementation is a rudimentary experiment. Here are some awkwardnesses:
* Only tables that happen to have a column called `id`, and where the column happens to be an integer, can be opened directly with Grist as it is today. That could be generalized, but it looked more than a Gristathon's worth of work, so I instead used SQLite views.
* Grist will handle tables that start with an uncapitalized letter a bit erratically. You can successfully add columns, for example, but removing them will cause sadness - Grist will rename the table in a confused way.
* I didn't attempt to deal with column names with spaces etc (though views could deal with those).
* I haven't tried to do any fancy type mapping.
* Columns with constraints can make adding new rows impossible in Grist, since Grist requires that a row can be added with just a single cell set.
Test Plan: added small test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3502
Summary:
When a user requests to read the contents of an attachment, only allow the request if there exists a cell in an attachment column that contains the attachment and which they have read access to.
This does not cover:
* Granular write access for attachments. In particular, a user who can write to any attachment column should be considered to have full read access to all attachment columns, currently.
* Access control of attachment metadata such as name and format.
The implementation uses a sql query that requires a scan, and some notes on how this could be optimized in future. The web client was updated to specify the cell to check for access, and performance seemed fine in casual testing on a doc with 1000s of attachments. I'm not sure how performance would hold up as the set of access rules grows as well.
Test Plan: added tests
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D3490
Summary:
Building:
- Builds no longer wait for tsc for either client, server, or test targets. All use esbuild which is very fast.
- Build still runs tsc, but only to report errors. This may be turned off with `SKIP_TSC=1` env var.
- Grist-core continues to build using tsc.
- Esbuild requires ES6 module semantics. Typescript's esModuleInterop is turned
on, so that tsc accepts and enforces correct usage.
- Client-side code is watched and bundled by webpack as before (using esbuild-loader)
Code changes:
- Imports must now follow ES6 semantics: `import * as X from ...` produces a
module object; to import functions or class instances, use `import X from ...`.
- Everything is now built with isolatedModules flag. Some exports were updated for it.
Packages:
- Upgraded browserify dependency, and related packages (used for the distribution-building step).
- Building the distribution now uses esbuild's minification. babel-minify is no longer used.
Test Plan: Should have no behavior changes, existing tests should pass, and docker image should build too.
Reviewers: georgegevoian
Reviewed By: georgegevoian
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3506
Summary:
- Get Jenkins to run on 4 agents in parallel, each executing 4 parallel test runs.
- Add a scheme for automatically selecting non-conflicting ports and Redis DB numbers.
- Add a scheme for automatically deciding how to group tests in large suites (nbrowser, server) to keep groups roughly equal.
- Add a recording of test timings, that's used for the auto-grouping.
- Fix tests that were sensitive to the order in which they were running.
Test Plan: All 5020 tests passed in 9 minutes (as opposed to the previous passing run which took 30).
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3500
Summary:
- Upgrades to build-related packages:
- Upgrade typescript, related libraries and typings.
- Upgrade webpack, eslint; add tsc-watch, node-dev, eslint_d.
- Build organization changes:
- Build webpack from original typescript, transpiling only; with errors still
reported by a background tsc watching process.
- Typescript-related changes:
- Reduce imports of AWS dependencies (very noticeable speedup)
- Avoid auto-loading global @types
- Client code is now built with isolatedModules flag (for safe transpilation)
- Use allowJs to avoid copying JS files manually.
- Linting changes
- Enhance Arcanist ESLintLinter to run before/after commands, and set up to use eslint_d
- Update eslint config, and include .eslintignore to avoid linting generated files.
- Include a bunch of eslint-prompted and eslint-generated fixes
- Add no-unused-expression rule to eslint, and fix a few warnings about it
- Other items:
- Refactor cssInput to avoid circular dependency
- Remove a bit of unused code, libraries, dependencies
Test Plan: No behavior changes, all existing tests pass. There are 30 tests fewer reported because `test_gpath.py` was removed (it's been unused for years)
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3498
Summary:
- Substantial refactoring of the logic when the server fails to send some
messages to a client.
- Add seqId numbers to server messages to ensure reliable order.
- Add a needReload flag in clientConnect for a clear indication whent the
browser client needs to reload the app.
- Reproduce some potential failure scenarios in a test case (some of which
previously could have led to incorrectly ordered messages).
- Convert other Comm tests to typescript.
- Tweak logging of Comm and Client to be slightly more concise (in particular,
avoid logging sessionId)
Note that despite the big refactoring, this only addresses a fairly rare
situation, with websocket failures while server is trying to send to the
client. It includes no improvements for failures while the client is sending to
the server.
(I looked for an existing library that would take care of these issues. A relevant article I found is https://docs.microsoft.com/en-us/azure/azure-web-pubsub/howto-develop-reliable-clients, but it doesn't include a library for both ends, and is still in review. Other libraries with similar purposes did not inspire enough confidence.)
Test Plan: New test cases, which reproduce some previously problematic scenarios.
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3470
Summary:
For self-hosted Grist, forward auth has proven useful, where
some proxy wrapped around Grist manages authentication, and
passes on user information to Grist in a trusted header.
The current implementation is adequate when Grist is the
only place where the user logs in or out, but is confusing
otherwise (see https://github.com/gristlabs/grist-core/issues/207).
Here we take some steps to broaden the scenarios Grist's
forward auth support can be used with:
* When a trusted header is present and is blank, treat
that as the user not being logged in, and don't look
any further for identity information. Specifically,
don't look in Grist's session information.
* Add a `GRIST_IGNORE_SESSION` flag to entirely prevent
Grist from picking up identity information from a cookie,
in order to avoid confusion between multiple login methods.
* Add tests for common scenarios.
Test Plan: added tests
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3482
Summary:
- Also converted sandboxUtil to typescript.
- The issue with %s manifested when a Python traceback contained "%s" in the
string; in that case the object with log metadata (e.g. docId) would
confusingly replace %s as if it were part of the message from Python.
Test Plan: Added a test case for the fix.
Reviewers: alexmojaki
Reviewed By: alexmojaki
Differential Revision: https://phab.getgrist.com/D3486
Summary:
- Add app/common/CommTypes.ts to define types shared by client and server.
- Include @types/ws npm package
Test Plan: Intended to have no changes in behavior
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3467
Summary: Combines the code and behaviour of the existing endpoints `GET /records` (for the general shape of the result and the parameters for sort/filter/limit etc) and retrieving a specific attachment with `GET /attachments/:id` for handling fields specific to attachments.
Test Plan: Added a DocApi test. Also updated one test to use the new endpoint instead of raw `GET /tables/_grist_Attachments/records`.
Reviewers: cyprien
Reviewed By: cyprien
Subscribers: cyprien
Differential Revision: https://phab.getgrist.com/D3443
Summary:
This makes it possible to configure a SendGrid-based Notifier
instance via a JSON configuration file.
Test Plan: Tested manually.
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: paulfitz
Differential Revision: https://phab.getgrist.com/D3432
Summary:
DocApi tests occasionally fail in a confusing way due to redis
state. This resets the redis state at the beginning of the test.
Test Plan: existing tests pass
Reviewers: alexmojaki
Reviewed By: alexmojaki
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3424
Summary:
Adds attachment and data size to the usage section of
the raw data page. Also makes in-document usage banners
update as user actions are applied, causing them to be
hidden/shown or updated based on the current state of
the document.
Test Plan: Browser tests.
Reviewers: jarek
Reviewed By: jarek
Subscribers: alexmojaki
Differential Revision: https://phab.getgrist.com/D3395
Summary: Allow exceeding the daily API usage limit for a doc based on additional allocations for the current hour and minute. See the doc comment on getDocApiUsageKeysToIncr for details. This means that up to 5 redis keys may be relevant at a time for a single document.
Test Plan: Updated and expanded 'Daily API Limit' tests.
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3368
Summary:
Call ActiveDoc.removeUnusedAttachments every hour using setInterval, and in ActiveDoc.shutdown (which also clears said interval).
Unrelated: small fix to my webhooks code which was creating a redis client on shutdown just to quit it.
Test Plan:
Tweaked DocApi test to remove expired attachments by force-reloading the doc, so that it removes them during shutdown. Extracted a new testing endpoint /verifyFiles to support this test (previously running that code only happened with `/removeUnused?verifyfiles=1`).
Tested the setInterval part manually.
Reviewers: paulfitz, dsagal
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3387
Summary: Adds methods to delete metadata rows based on timeDeleted. The flag expiredOnly determines if it only deletes attachments that were soft-deleted 7 days ago, or just all soft-deleted rows. Then any actual file data that doesn't have matching metadata is deleted.
Test Plan: DocApi test
Reviewers: paulfitz
Reviewed By: paulfitz
Subscribers: dsagal
Differential Revision: https://phab.getgrist.com/D3364
Summary:
Builds on https://phab.getgrist.com/D3352
Add DocStorage.scanAttachmentsForUsageChanges to do fancy JSON query to find all attachment metadata rows whose soft deletion status needs updating.
Add ActiveDoc.updateUsedAttachments which uses the above and then applies the appropriate user action if needed to soft delete/undelete metadata rows.
Add endpoint in DocApi calling ActiveDoc method.
Test Plan: Added DocApi test
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D3357
Summary:
This annotates the plugin api sufficiently to generate some documentation
for it. See https://github.com/gristlabs/grist-help/pull/139
Contains some small code tweaks for things that caused typedoc some
trouble.
Test Plan: manual inspection of output
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D3342
* remove stray redis dependency in test
* tweak handling of database connection between tests
* upgrade node versions in tests, type guessing in node 10 has problems
Summary:
This shuffles some server tests to make them available in grist-core,
and adds a test for the `GRIST_PROXY_AUTH_HEADER` feature added in
https://github.com/gristlabs/grist-core/pull/165
It includes a fix for a header normalization issue for websocket connections.
Test Plan: added test
Reviewers: georgegevoian
Reviewed By: georgegevoian
Differential Revision: https://phab.getgrist.com/D3326
Summary:
This is preparatory work for running tests with the new sandbox in jenkins.
* Makes a base image that is now shared by grist servers and jenkins workers.
* Needed to allow jenkins to run `sudo runsc`.
* Converged on port 2020 for ssh to workers and servers.
Test Plan: added one runsc-based test and confirmed it was run
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D3029
Summary:
This applies some mitigations suggested by SQLite authors when
opening untrusted SQLite databases, as we do when Grist docs
are uploaded by the user. See:
https://www.sqlite.org/security.html#untrusted_sqlite_database_files
Steps implemented in this diff are:
* Setting `trusted_schema` to off
* Running a SQLite-level integrity check on uploads
Other steps will require updates to our node-sqlite3 fork, since they
are not available via the node-sqlite3 api (one more reason to migrate
to better-sqlite3).
I haven't yet managed to create a file that triggers an integrity
check failure without also being detected as corruption by sqlite
at a more basic level, so that is a TODO for testing.
Test Plan:
existing tests pass; need to come up with exploits to
actually test the defences and have not yet
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2909
Summary:
Replaces https://phab.getgrist.com/D2854
Refactoring of NSandbox:
- Simplify arguments to NSandbox.spawn. Only half the arguments were used depending on the flavour, adding a layer of confusion.
- Ensure the same environment variables are passed to both flavours of sandbox
- Simplify passing down environment variables.
Implement deterministic mode with libfaketime and a seeded random instance.
- Include static prebuilt libfaketime.so.1, may need another solution in future for other platforms.
Recording pycalls:
- Add script recordDocumentPyCalls.js to open a single document outside of tests.
- Refactor out recordPyCalls.ts to support various uses.
- Add afterEach hook to save all pycalls from server tests under $PYCALLS_DIR
- Make docTools usable without mocha.
- Add useLocalDoc and loadLocalDoc for loading non-fixture documents
Test Plan:
Made a document with formulas NOW() and UUID()
Compare two document openings in normal mode:
diff <(test/recordDocumentPyCalls.js samples/d4W6NrzCMNVSVD6nWgNrGC.grist /dev/stdout) \
<(test/recordDocumentPyCalls.js samples/d4W6NrzCMNVSVD6nWgNrGC.grist /dev/stdout)
Output:
< 1623407499.58132,
---
> 1623407499.60376,
1195c1195
< "B": "bd2487f6-63c9-4f02-bbbc-5c0d674a2dc6"
---
> "B": "22e1a4fd-297f-4b86-91a2-bc42cc6da4b2"
`export DETERMINISTIC_MODE=1` and repeat. diff is empty!
Reviewers: paulfitz
Reviewed By: paulfitz
Differential Revision: https://phab.getgrist.com/D2857
Summary:
* adds a smoke test to grist-core
* fixes a problem with highlight.js failing to load correctly
* skips survey for default user
* freshens docker build
Utility files in test/nbrowser are moved to core/test/nbrowser, so that gristUtils are available there. This increased the apparent size of the diff as "./" import paths needed replacing with "test/nbrowser/" paths. The utility files are untouched, except for the code to start a server - it now has a small grist-core specific conditional in it.
Test Plan: adds test
Reviewers: dsagal
Reviewed By: dsagal
Differential Revision: https://phab.getgrist.com/D2768
