Skip to content

Commit

Permalink
updated connector API to not require model on each method
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-burel committed Nov 12, 2020
1 parent 66c07b2 commit 7d18ba8
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 50 deletions.
4 changes: 2 additions & 2 deletions packages/graphql/server/resolvers/defaultMutationResolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ const getMutationDocument = async ({
const { _id, input } = variables;
if (_id) {
// _id bypass input
document = await connector.findOneById(model, _id);
document = await connector.findOneById(_id);
} else {
const filterParameters = await connector.filter(model, input, context);
const filterParameters = await connector.filter(input, context);
selector = filterParameters.selector;
// get entire unmodified document from database
document = await connector.findOne(model, selector);
Expand Down
11 changes: 5 additions & 6 deletions packages/graphql/server/resolvers/defaultQueryResolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export function buildDefaultQueryResolvers<TModel extends VulcanDocument>({
const { currentUser } = context;
// get selector and options from terms and perform Mongo query

let { selector, options } = await connector.filter(model, input, context);
let { selector, options } = await connector.filter(input, context);
const filteredFields = Object.keys(selector);

// make sure all filtered fields are allowed, before fetching the document
Expand All @@ -112,7 +112,7 @@ export function buildDefaultQueryResolvers<TModel extends VulcanDocument>({

debugGraphql({ selector, options });

const docs = await connector.find(model, selector, options);
const docs = await connector.find(selector, options);
// in restrictViewableFields, null value will return {} instead of [] (because it works both for array and single doc)
let viewableDocs = [];

Expand Down Expand Up @@ -173,7 +173,7 @@ export function buildDefaultQueryResolvers<TModel extends VulcanDocument>({

if (enableTotal) {
// get total count of documents matching the selector
data.totalCount = await connector.count(model, selector);
data.totalCount = await connector.count(selector);
} else {
data.totalCount = null;
}
Expand Down Expand Up @@ -213,16 +213,15 @@ export function buildDefaultQueryResolvers<TModel extends VulcanDocument>({

// use Dataloader if doc is selected by _id
if (_id) {
doc = await connector.findOneById(model, _id);
doc = await connector.findOneById(_id);
} else {
let { selector, options, filteredFields } = await connector.filter(
model,
input,
context
);
// make sure all filtered fields are actually readable, for basic roles
checkFields(currentUser, model, filteredFields);
doc = await connector.findOne(model, selector, options);
doc = await connector.findOne(selector, options);

// check again that the fields used for filtering were all valid, this time based on retrieved document
// this second check is necessary for document based permissions like canRead:["owners", customFunctionThatNeedDoc]
Expand Down
6 changes: 3 additions & 3 deletions packages/graphql/server/resolvers/mutators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export const createMutator = async <TModel extends VulcanDocument>({

/* DB Operation */
const connector = getModelConnector<TModel>(context, model);
let document = await connector.create(model, data);
let document = await connector.create(data);

/* After */
document = await runCallbacks({
Expand Down Expand Up @@ -354,7 +354,7 @@ export const updateMutator = async <TModel extends VulcanDocument>({
if (!isEmpty(modifier)) {
// update document
// and get fresh copy of document from db
document = await connector.update(model, selector, modifier, {
document = await connector.update(selector, modifier, {
removeEmptyStrings: false,
});

Expand Down Expand Up @@ -469,7 +469,7 @@ export const deleteMutator = async <TModel extends VulcanDocument>({
});

/* DB Operation */
await connector.delete(model, selector);
await connector.delete(selector);

/* After */
document = await runCallbacks({
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql/server/resolvers/relationResolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const hasOne: RelationResolver = async ({
const relatedDocument = await getModelConnector(
context,
relatedModel
).findOneById(relatedModel, documentId);
).findOneById(documentId);
// filter related document to restrict viewable fields
return restrictViewableFields(
context.currentUser,
Expand Down
40 changes: 11 additions & 29 deletions packages/graphql/server/resolvers/typings.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
import { VulcanModel } from "@vulcanjs/model";
import { VulcanDocument } from "@vulcanjs/schema";
import { AnyARecord } from "dns";
/**
* TODO
// NOTE: the connector is shared between all models (so you must pass the model as a parameter)
// We might want to have instead one connector per model?
// So the model could be removed from all methods parameters
To be investigated after testing out the connector pattern
*/
// NOTE: vulcan/graphql CAN'T depend on vulcan/mongo or mongo so do not move this code in vulcan/mongo
// we use Mongo syntax but this is not a real dependency to Mongo, just a convenience, we only need the typings from Mongo
export type MongoSelector = Object;

/**
* A database abstraction compatible with Vulcan
Expand All @@ -18,45 +12,33 @@ export interface Connector<TModel extends VulcanDocument = Object> {
* Compute the relevant selectors
*/
filter: (
model: VulcanModel,
input: any,
context: any
) => {
// mongo like selector
selector: Object;
//
selector: MongoSelector;
options: any;
filteredFields: Array<any>; // TODO: in defaultQueryResolvers we do filteredFields = Object.keys(selector), so what is this parameter?
};
// replaces collection.loader.load
// @see https://github.com/GraphQLGuide/apollo-datasource-mongodb/#findonebyid
findOneById: (model: VulcanModel, _id: string) => Promise<TModel>;
findOneById: (_id: string) => Promise<TModel>;
// replaces get
findOne: (
model: VulcanModel,
selector: Object,
options?: Object
) => Promise<TModel>;
findOne: (selector: MongoSelector, options?: Object) => Promise<TModel>;
/**
* Find data in the database
*/
find: (
model: VulcanModel,
selector: Object,
options: Object
) => Promise<Array<TModel>>;
count: (model: VulcanModel, selector: Object) => Promise<number>;
find: (selector: MongoSelector, options: Object) => Promise<Array<TModel>>;
count: (selector: MongoSelector) => Promise<number>;
// TODO: should we keep supporting loader.load and get? or
// Mutations
create: (model: VulcanModel, data: VulcanDocument) => Promise<TModel>;
create: (data: VulcanDocument) => Promise<TModel>;
update: (
model: VulcanModel,
selector: Object,
selector: MongoSelector,
modifier: Object,
{ removeEmptyStrings: boolean }
) => Promise<TModel>;
// Returns the delete object
delete: (model: VulcanModel, selector: Object) => Promise<TModel>;
delete: (selector: Object) => Promise<TModel>;
}

export interface ContextWithUser {
Expand Down
9 changes: 5 additions & 4 deletions packages/graphql/server/tests/mutators/callbacks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe("graphql/resolvers/mutators callbacks", function () {
describe("after", () => {
test("run asynchronous 'after' callback before document is returned", async function () {
const after = jest.fn(async (doc) => ({ ...doc, createdAfter: 1 }));
const create = jest.fn(async (model, data) => ({ _id: 1, ...data }));
const create = jest.fn(async (data) => ({ _id: 1, ...data }));
const Foo = createModel({
schema,
name: "Foo",
Expand All @@ -96,15 +96,15 @@ describe("graphql/resolvers/mutators callbacks", function () {
context,
data,
});
expect((create.mock.calls[0] as any)[1]).toEqual(data); // callback is NOT yet acalled
expect((create.mock.calls[0] as any)[0]).toEqual(data); // callback is NOT yet acalled
expect(after).toHaveBeenCalledTimes(1);
expect(resultDocument.createdAfter).toBe(1); // callback is called
});
});
describe("before", () => {
test("run asynchronous 'before' callback before document is saved", async function () {
const before = jest.fn(async (doc) => ({ ...doc, createdBefore: 1 }));
const create = jest.fn(async (model, data) => ({ _id: 1, ...data }));
const create = jest.fn(async (data) => ({ _id: 1, ...data }));
const Foo = createModel({
schema,
name: "Foo",
Expand All @@ -130,7 +130,8 @@ describe("graphql/resolvers/mutators callbacks", function () {
context,
data,
});
expect((create.mock.calls[0] as any)[1]).toEqual({
const dataArg = (create.mock.calls[0] as any)[0];
expect(dataArg).toEqual({
...data,
createdBefore: 1,
}); // callback is called already
Expand Down
10 changes: 5 additions & 5 deletions packages/graphql/server/tests/mutators/mutators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,15 +212,15 @@ describe("graphql/resolvers/mutators", function () {
const context = merge({}, defaultContext, {
Foo: {
connector: {
create: async (model, data) => ({
create: async (data) => ({
...data, // preserve provided data => this is needed to test the callbacks
id: "1",
}),
findOne: async () => ({
id: "1",
foo2: "bar",
}),
update: async (model, selector, modifier) => ({
update: async (selector, modifier) => ({
id: "1",
...modifierToData(modifier), // we need to preserve the existing document
}),
Expand Down Expand Up @@ -298,7 +298,7 @@ describe("graphql/resolvers/mutators", function () {
currentUser: { _id: "42" },
Foo: {
model: Foo,
connector: { create: async (model, data) => ({ _id: 1, ...data }) },
connector: { create: async (data) => ({ _id: 1, ...data }) },
},
});
const { data: resultDocument } = await createMutator({
Expand All @@ -314,15 +314,15 @@ describe("graphql/resolvers/mutators", function () {
const context = {
Foo: {
connector: {
create: async (model, data) => ({
create: async (data) => ({
...data, // preserve provided data => this is needed to test the callbacks
id: "1",
}),
findOne: async () => ({
id: "1",
foo2: "bar",
}),
update: async (model, selector, modifier) => ({
update: async (selector, modifier) => ({
id: "1",
...modifierToData(modifier), // we need to preserve the existing document
}),
Expand Down

0 comments on commit 7d18ba8

Please sign in to comment.