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

feat: Allow to enforce Stack link on request chain #1575

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

Ldoppea
Copy link
Member

@Ldoppea Ldoppea commented Dec 17, 2024

In previous commits we implemented supports for offline mode through CozyPouchLink and FlagshipLink

Since this feature, the StackLink is able to check for internet reachability by using the isOnline() method. When no internet is detected, then the request is forwarded to the next link (i.e. the CozyPouchLink)

In some scenario we may want to prevent that behavior. This is the case when we are working on a feature that cannot work without internet access, or when the query's result is expected to contain data that is injected by the cozy-stack (and so that is not available in the local Pouch database)

To make this possible we introduce the forceStack parameter that can be set in the query options

When set to true then the StackLink will not attempt to forward the request when offline, instead it will still attempt to do the request and throw

We chose to attempt the request instead of directly throwing an error in order to prevent hypothetical scenario where isOnline() method returns false even if internet is reachable

BREAKING CHANGE: CozyLink's request methods now takes an additional options argument that can be used to enforce usage of Stack link. Although .request() is meant to be an internal API, if your code calls it, you'll want to introduce a new argument for options. This is also the case if you instanciate a new CozyLink with a handler argument for request

Before:

// Initialization
new CozyLink((operation, result = '', forward) => {
  return forward(operation, result + 'foo')
})

// Call
link.request(operation)

// Call with result and forward
link.request(operation, null, () => { /* do stuff */ })

After:

// Initialization
new CozyLink((operation, options, result = '', forward) => {
  return forward(operation, options, result + 'foo')
})

// Call
link.request(operation, options)

// Call with result and forward
link.request(operation, options, null, () => { /* do stuff */ })

Related PR: cozy/cozy-flagship-app#1269

@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch from 4aba700 to a47af8c Compare December 17, 2024 16:22
@Ldoppea Ldoppea force-pushed the feat/performances branch 3 times, most recently from 0d2969b to b3815e2 Compare December 18, 2024 11:54
@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch 3 times, most recently from b8c1989 to d233213 Compare December 18, 2024 16:01
@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch from d233213 to 7f3835c Compare December 19, 2024 10:23
@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch from 7f3835c to 71f2363 Compare December 19, 2024 10:31
@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch from 71f2363 to 7de1401 Compare December 19, 2024 10:33
@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch from 7de1401 to 11c1865 Compare December 19, 2024 10:48
if (this.isOnline && !(await this.isOnline())) {
return forward(operation)
async request(operation, options, result, forward) {
if (this.isOnline && !(await this.isOnline()) && !options?.forceStack) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worth it to reverse the conditions here, by evaluation forceStack first: if the options?.forceStack is true, there is no need to call the this.isOnline(), and thus we save a network call

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed here

async request(operation, result, forward) {
if (this.isOnline && !(await this.isOnline())) {
return forward(operation)
async request(operation, options, result, forward) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No jsdoc for the options ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed here

@@ -410,7 +410,7 @@ class PouchLink extends CozyLink {
return !!this.getPouch(impactedDoctype)
}

async request(operation, result = null, forward = doNothing) {
async request(operation, options, result = null, forward = doNothing) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If the forceStack: true option is given here, we should forward the request

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed here

@Ldoppea Ldoppea force-pushed the feat/performances branch 2 times, most recently from ee1b801 to fabe643 Compare December 19, 2024 14:09
@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch from 11c1865 to c6fda04 Compare December 19, 2024 14:09
@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch from c6fda04 to 99d56c8 Compare December 20, 2024 15:05
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
When doing a backup, we don't want instable network to make the
CozyClient use local PouchLink for some queries that need to read files
paths

In cozy/cozy-client#1575 we implemented the `forceStack` option that
allows to enforce the usage of StackLink instead of other links that
may retrieve local incomplete data

This commit enforce the stack on backup related queries

Related PR: cozy/cozy-client#1575
@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch 2 times, most recently from 8007276 to d1b66bb Compare December 20, 2024 16:56
@Ldoppea Ldoppea marked this pull request as ready for review December 20, 2024 16:56
@Ldoppea Ldoppea requested a review from Merkur39 as a code owner December 20, 2024 16:56
Base automatically changed from feat/performances to master December 20, 2024 17:05
In previous commits we implemented supports for offline mode through
CozyPouchLink and FlagshipLink

Since this feature, the StackLink is able to check for internet
reachability by using the `isOnline()` method. When no internet is
detected, then the request is forwarded to the next link (i.e. the
CozyPouchLink)

In some scenario we may want to prevent that behavior. This is the case
when we are working on a feature that cannot work without internet
access, or when the query's result is expected to contain data that is
injected by the cozy-stack (and so that is not available in the local
Pouch database)

To make this possible we introduce the `forceStack` parameter that can
be set in the query options

When set to `true` then the StackLink will not attempt to forward the
request when offline, instead it will still attempt to do the request
and throw

We chose to attempt the request instead of directly throwing an error
in order to prevent hypothetical scenario where `isOnline()` method
returns false even if internet is reachable

BREAKING CHANGE: CozyLink's request methods now takes an additional
`options` argument that can be used to enforce usage of Stack link.
Although `.request()` is meant to be an internal API, if your code
calls it, you'll want to introduce a new argument for `options`. This
is also the case if you instanciate a new CozyLink with a handler
argument for `request`

Before:
```
// Initialization
new CozyLink((operation, result = '', forward) => {
  return forward(operation, result + 'foo')
})

// Call
link.request(operation)

// Call with result and forward
link.request(operation, null, () => { /* do stuff */ })
```

After:
```
// Initialization
new CozyLink((operation, options, result = '', forward) => {
  return forward(operation, options, result + 'foo')
})

// Call
link.request(operation, options)

// Call with result and forward
link.request(operation, options, null, () => { /* do stuff */ })
```
@Ldoppea Ldoppea force-pushed the feat/links_force_stack branch from d1b66bb to e00d74d Compare December 20, 2024 17:09
@Ldoppea Ldoppea merged commit 9e0c657 into master Dec 20, 2024
3 checks passed
@Ldoppea Ldoppea deleted the feat/links_force_stack branch December 20, 2024 17:14
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
`cozy-client` and `cozy-pouch-link` has been upgraded to `52.0.0` in
order to retrieve the ability to enforce StackLink usage for a query

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
When doing a backup, we don't want instable network to make the
CozyClient use local PouchLink for some queries that need to read files
paths

In cozy/cozy-client#1575 we implemented the `forceStack` option that
allows to enforce the usage of StackLink instead of other links that
may retrieve local incomplete data

This commit enforce the stack on backup related queries

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
`cozy-client` and `cozy-pouch-link` has been upgraded to `52.0.0` in
order to retrieve the ability to enforce StackLink usage for a query

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
When doing a backup, we don't want instable network to make the
CozyClient use local PouchLink for some queries that need to read files
paths

In cozy/cozy-client#1575 we implemented the `forceStack` option that
allows to enforce the usage of StackLink instead of other links that
may retrieve local incomplete data

This commit enforce the stack on backup related queries

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
`cozy-client` and `cozy-pouch-link` has been upgraded to `52.0.0` in
order to retrieve the ability to enforce StackLink usage for a query

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
When doing a backup, we don't want instable network to make the
CozyClient use local PouchLink for some queries that need to read files
paths

In cozy/cozy-client#1575 we implemented the `forceStack` option that
allows to enforce the usage of StackLink instead of other links that
may retrieve local incomplete data

This commit enforce the stack on backup related queries

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
`cozy-client` and `cozy-pouch-link` has been upgraded to `52.0.0` in
order to retrieve the ability to enforce StackLink usage for a query

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
When doing a backup, we don't want instable network to make the
CozyClient use local PouchLink for some queries that need to read files
paths

In cozy/cozy-client#1575 we implemented the `forceStack` option that
allows to enforce the usage of StackLink instead of other links that
may retrieve local incomplete data

This commit enforce the stack on backup related queries

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
`cozy-client` and `cozy-pouch-link` has been upgraded to `52.0.0` in
order to retrieve the ability to enforce StackLink usage for a query

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Dec 20, 2024
When doing a backup, we don't want instable network to make the
CozyClient use local PouchLink for some queries that need to read files
paths

In cozy/cozy-client#1575 we implemented the `forceStack` option that
allows to enforce the usage of StackLink instead of other links that
may retrieve local incomplete data

This commit enforce the stack on backup related queries

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Jan 9, 2025
`cozy-client` and `cozy-pouch-link` has been upgraded to `52.0.0` in
order to retrieve the ability to enforce StackLink usage for a query

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Jan 9, 2025
When doing a backup, we don't want instable network to make the
CozyClient use local PouchLink for some queries that need to read files
paths

In cozy/cozy-client#1575 we implemented the `forceStack` option that
allows to enforce the usage of StackLink instead of other links that
may retrieve local incomplete data

This commit enforce the stack on backup related queries

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Jan 17, 2025
`cozy-client` and `cozy-pouch-link` has been upgraded to `52.0.0` in
order to retrieve the ability to enforce StackLink usage for a query

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Jan 17, 2025
When doing a backup, we don't want instable network to make the
CozyClient use local PouchLink for some queries that need to read files
paths

In cozy/cozy-client#1575 we implemented the `forceStack` option that
allows to enforce the usage of StackLink instead of other links that
may retrieve local incomplete data

This commit enforce the stack on backup related queries

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Jan 17, 2025
`cozy-client` and `cozy-pouch-link` has been upgraded to `52.0.0` in
order to retrieve the ability to enforce StackLink usage for a query

Related PR: cozy/cozy-client#1575
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Jan 17, 2025
When doing a backup, we don't want instable network to make the
CozyClient use local PouchLink for some queries that need to read files
paths

In cozy/cozy-client#1575 we implemented the `forceStack` option that
allows to enforce the usage of StackLink instead of other links that
may retrieve local incomplete data

This commit enforce the stack on backup related queries

Related PR: cozy/cozy-client#1575
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants