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

[epic] Experiment with mockable operations #482

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ yarn-error.log*
lerna-debug.log*

# Generated jsdocs
docs/
packages/**/docs/

# Working folders
tmp/
Expand Down
33 changes: 33 additions & 0 deletions docs/best-practice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
style guide and best practice notes

## Code Style

* Use `camelCase` in variable and parameter names. This helps keep adaptors consistent.
* Try to avoid exposing the underlying implementation.
* * For example, if wrapping a function and the function exposes an options object, it's usually better to define your own options object and map it.
* * This lets us provide our own documention, enforce camelCase conventions (particularly when wrapping web services), and insulates us from changes in the underlying implementation.
* * It can also simplify an API to just expose options an end-user needs. For example we'd expect than an adaptor should deal with pagination automagically, so exporting page options is probably unhelpful
* * This isn't _always_ possible or sensible, particularly for large APIs, but should be considered.

## Documentation

All operations should be well documented in JSdoc annotations.

It is usually best not to link to the underlying implementation's documentation - their context, requirements and expectations are very different to ours, and it may confuse our users. Better to keep users on our own docsite, with consistent style and navigation.

## API Design

A successful adaptor should have a clean, re-usable and easy to understand API design.

There are two schools of thought for adaptor API design [and frankly we haven't quite settled on an answer yet].

1) An adaptor is a one-for-one wrapper around some backing web service or library. For each endpoint/function in the service, there should be an operation, with the same name, in the adaptor.
2) An adaptor is a high-level, user friendly, opinionated wrapper around a web service or library. It is not a mapping, but a high level abstraction.

The approach you take my depend on your experience, and that of your users. If you know your users are experts in a particular rest interface, it probably makes sense just to expose that interface in an openfn-y way (so, a 1:1 mapping).

On the other hand, if an API is particularly complex or heavy in boilerplate, it probably makes sense to provide some user-friendly, high level abstractions.

A good approach is probably to do a little of both. Provide low-level helper functions to give access to the underlying implementaton (wrapping the `request` function is often the first step to a successful adaptor!), but then creating opinionated helper functions with specific tasks in mind.

Also, when creating/editing adaptors, it's often best to have a goal in mind. Don't try and wrap an entire implementation - just do the bits you need for the job you (or your users) have in mind.
9 changes: 9 additions & 0 deletions docs/changes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
## Internal / Temporary docs

Tracking changes in this repo at a high level

This will feed into the PR.

* Introduce the `operation` factory helper
* Introduce impl pattern
* Introduce integration testing
101 changes: 101 additions & 0 deletions docs/creating-adaptors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
explain the basic structure of how to create an adaptor

explain the adaptor/impl pattern

## Folder structure

In order to make it easier for developers to create and maintain adaptors, we adopt strong conventions around style and structure.

An adaptor should have the following file structure:
```
src
+- operations.js
+- impl.js
+- util.js
+- index.js
+- mock.js
```

`operations.js` is the most important file and every adaptor should have one. It's where your operations will live, along with your documentation. Anything you expect users to call from job code should live here. Keeping your operations in one place makes it easy for other developers and maintainers to see where the user-facing code is.

`impl.js` is not required but encouraged. A useful pattern is to declare the interface to your operations in `operations.js`, but to add the actual implementation, the actual logic, in `impl.js`. Splitting each operation into two parts makes it easier to define unit tests (see the testing doc)

`util.js` is another optional file. If you have helper functions to support your implementation, you can add them here. If you think they might be of use to job writers, you can export them in index (see below)

`mock.js` contains mock code and data for your unit and integration tests. This is excluded from the release build and not published to npm.

`index.js` declares the exports used by the final npm module.

## Defining operations

The common adaptor exposes a helper function to make it easier to define adaptors.

Call the `operation` function with a single function argument. The function takes state as its first argument and whatever else you need. Assign the function to an exported const.

```
export const getPatient = operation((state, name, country, options) => {
// Implement the operation here
})
```

If you prefer, you can stll define operations the "old" way, by creating your own factory wrapper.
```
export function getPatient(name, country, options) {
return (state) => {
// Implement the operation here
}
}
```
You'll still see this pattern used across the repo.

## Implementing Operations

Testing OpenFn adaptors has traditionally been a very hard problem. The factory pattern and the nature of the backend service makes it very hard to test code in isolation.

The impl pattern helps this.

You can cut the body of each operation out into an impl. The impl has almost the same structure, but it also accepts a library, client or exectutor function as an argument. This makes the implementation mockable.

For example:

```
// operations.js
import client from 'my-library';
import impl from './impl';

export const getPatient = operation((state, name, country, options) => {
impl.getPatient(state, client, name, country, options)
})
```

This operation is really just a signature and declaration which calls out to an implementation function. Look closely at the impl signature though and you'll notice an extra argument: `client`.

Our implementation might look like ths:
```
// impl.js
export const getPatient = (state, client, name, country, options) => {
const result = await client.findPatient(name, { country, ...options })
state.data = result.patients[0];
return state;
}
```

Default values should be set in the outer operation wrapper.

The implementation is easy to unit test because we can pass in a mock client object or function. Different adaptors have different needs but they ultimately will either call out to a webservice or another npm module.

## Exports

In order for anyone to use your adaptor, you need to export the correct functions in the index.

Usually this is as simple as:
```
export * from './operations`
```
Which will make all operations available to user jobs.

You may also want to export util functions to job code. It's good practice to namespace these so that it's clear to users what is an operation and what is a function.
```
export * as util from './utils'
```
Users can then do `util.convertPatient(patient)` in their job code.
40 changes: 40 additions & 0 deletions docs/overview.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
Overview of adaptors and how they work

key concepts - explain the words job, expression, workflow etc

## Jobs

A job is a unit of work, expressed via Javascript-like

We sometimes use the word "expression" when referring to a job. Technically, the code written by end users (and executed by the OpenFn runtime) is the _expression_. A job is an expression plus a bunch of metadata about it, like which adaptor to use, what initial state to pass in, what options to use during execution.

But mostly the terms are interchangable.

An expression can be compiled or uncompiled.

## DSL

OpenFn code does not use Javascript.

It actually uses a Javascript-like DSL (Domain Specific Language). This needs to be compiled into executable Javascript.

The major differences between openfn code and Javascript are:
* The top level functions in the code are executed synchronously (in sequence), even if they contain asynchronous code
* OpenFn code does not contain import statements (although technically it can). These are compiled in.

## Operations

Users write openfn code by composing operations into a sequence of commands.

Adaptors export operations - basically helper functions - to users.

### Operations vs functions

While an operation is of course a Javascript function, there are several differences between operationns and regular javascript functions.

* An operation is a function that returns a function which in turn takes and returns a state object.
* The operations declared in an adaptor are _factories_ which return state-handling functions.
* Operations can be invoked at the top level of job code
* To call an operation within another function in job code, it has to be wrapped with state, ie `myOp(options)(state)`. We consider this to be an antipattern in job code - but it is occasionally neccessary

In short, an operation is a function designed to run in an openfn job and handle state.
15 changes: 15 additions & 0 deletions docs/testing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
explain how unit tests and integration tests work


There are two types of test which serve different purposes;

* Unit Tests are tightly scoped tests which execute specific functions in isolation. The smaller a unit of code is tested, the better the test. We would expect one unit test per operation (usually against the implementation, rather than the operation)
* Integration tests are broadly scoped tests against multiple operations. These look like actual job code and are executed against the actual Openfn Runtime

## How to Unit Test

TODO

## How to Integration Test

TODO
Loading
Loading