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

Provide API for working with farmOS Aggregator #35

Closed
jgaehring opened this issue Sep 21, 2021 · 8 comments
Closed

Provide API for working with farmOS Aggregator #35

jgaehring opened this issue Sep 21, 2021 · 8 comments
Milestone

Comments

@jgaehring
Copy link
Member

In discussion today, @mstenta mentioned that Our-Sci (and potentially other partners working on integrations with farmOS) will soon have a need to make calls to a farmOS Aggregator service in the very near future. I don't think it makes sense to provide such an API in farmOS.js for 1.x, which is effectively deprecated, but it would be nice to have this ready by the general release of 2.0.0 at the latest. With the changes I've made to the connect portion of the library, particularly in how the OAuth portions are separated into their own functions, I hope this will not require too much effort.

A first step, at least for myself, would be to get better acquainted with the Aggregator's own API, and how this can be dovetailed with the work already done with the farmOS instance API. Perhaps @paul121 and I can sit down at some point to take a look at this together.

@jgaehring
Copy link
Member Author

Bumping this up to the beta milestone, so we can look at this at the same time we address farmOS/field-kit#452, which will need to be resolved by then.

@jgaehring
Copy link
Member Author

I pushed this back to the general release milestone a few weeks ago, and still don't want this to block the beta release, which will hopefully happen later today; however, I believe we may move more quickly on this than I originally expected, depending on when this is needed for integration with SurveyStack.

Also, @paul121, do I recall you saying yesterday that there were still some changes that needed to happen on the Aggregator itself, to enable relaying requests to the individual farmOS instances?

@jgaehring
Copy link
Member Author

I spoke a bit with @paul121 about how best to implement this, and to copy and past liberally from that convo...

I think for the case of the Aggregator, since it's already so close to the farmOS API, we can probably just drop in (or pass in) a different authorization method for the client itself. The OAuth stuff is all pretty much isolated to oauth.js and this line in client/index.js:

const { authorize } = oauth(request, oAuthOpts);

So to extend the options already being passed to the client:

const auth = apiKeyAuth;
const farm = client(host, {
  clientId, getToken, setToken, auth,
});

where apiKeyAuth can be defined as:

const apiKeyAuth = (request, authOpts = {}) => {
  request.interceptors.request.use(/** set up API key access */);
  request.interceptors.response.use(/** additional handlers */);
  return {
    autorize() {
      // etc...
    },
  };
};

The basic idea I'd like to develop for the client as a whole is for it to be an interface, with get, send and delete methods for each entity type, and a generic request method.... all the rest, including authorize and getToken are extensions made by the particular d9JsonApi client. Meanwhile, the farmOS constructor should be relatively transparent to whatever client/adapter is being passed to it.

Ideally the client would be similarly transparent to the auth portion. Right now, I realize it's not fully transparent, in the sense that it has knowledge of clientId, getToken and setToken, but that really shouldn't be necessary since it's just rewrapping those options with the host as oAuthOpts and passing them along.

I think there are relatively few changes we need to make within farmOS.js to make this possible, so @paul121 can begin testing it out in his local Aggregator dev environment by next week.

@jgaehring
Copy link
Member Author

Okay, so I've parameterized OAuth as the default auth option in 7ebc20f, so it should now be possible to pass in something like the apiKeyAuth function I sketched out above. Here's the type definition for that option:

/**
 * @typedef {Function} AuthMixin
 * @param {import('axios').AxiosInstance} request
 * @param {Object} authOptions
 * @property {String} authOptions.host
 * @returns {Object<string,function>}
 */

Hopefully that should be enough to get you started, @paul121!

@jgaehring
Copy link
Member Author

And I just tagged these changes as 2.0.0-beta.6, so if you install the latest from npm you should be able to use that new auth option.

@jgaehring
Copy link
Member Author

Blah, I accidentally broke HEAD with 2.0.0-beta.6 and didn't notice til I deployed it to develop.farmos.app, so make sure to use 2.0.0-beta.7.

@paul121
Copy link
Member

paul121 commented Feb 25, 2022

A little overdue, but this is working! These changes make it possible to create a farmOS.js client that uses an api-key header for authentication with the aggregator.

I have an example of this here, eventually we should include this in the aggregator docs directly: https://gist.github.com/paul121/26bed0987b73c6886fa3a0743c0f47eb

@jgaehring I think we can close this issue :-)

@jgaehring
Copy link
Member Author

Oh awesome! We could include something in the farmOS.js docs, too! Maybe just a link to the Aggregator docs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants