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

Example: how to create dedicated email and URL types #109

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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 @app/client/src/graphql/AddEmail.graphql
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#import "./EmailsForm_UserEmail.graphql"

mutation AddEmail($email: String!) {
mutation AddEmail($email: Email!) {
createUserEmail(input: { userEmail: { email: $email } }) {
user {
id
Expand Down
2 changes: 1 addition & 1 deletion @app/client/src/graphql/ForgotPassword.graphql
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
mutation ForgotPassword($email: String!) {
mutation ForgotPassword($email: Email!) {
forgotPassword(input: { email: $email }) {
# This mutation does not return any meaningful result,
# but we still need to request _something_...
Expand Down
2 changes: 1 addition & 1 deletion @app/client/src/graphql/Register.graphql
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
mutation Register(
$username: String!
$password: String!
$email: String!
$email: Email!
$name: String
) {
register(
Expand Down
31 changes: 18 additions & 13 deletions @app/db/migrations/committed/000001.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--! Previous: -
--! Hash: sha1:4be49e527161e4b03af6630d795e00271405d754
--! Hash: sha1:269a8e30333545dd7d76a2591aa8637a27603ebb

drop schema if exists app_public cascade;

Expand Down Expand Up @@ -34,6 +34,11 @@ create schema app_private;

/**********/

create domain app_public."URL" as text check(VALUE ~ '^https?://[^/]+');
create domain app_public.email as citext check(VALUE ~ '[^@]+@[^@]+\.[^@]+');

/**********/

create function app_private.tg__add_job() returns trigger as $$
begin
perform graphile_worker.add_job(tg_argv[0], json_build_object('id', NEW.id), coalesce(tg_argv[1], public.gen_random_uuid()::text));
Expand Down Expand Up @@ -121,7 +126,7 @@ create table app_public.users (
id serial primary key,
username citext not null unique check(length(username) >= 2 and length(username) <= 24 and username ~ '^[a-zA-Z]([a-zA-Z0-9][_]?)+$'),
name text,
avatar_url text check(avatar_url ~ '^https?://[^/]+'),
avatar_url app_public."URL",
is_admin boolean not null default false,
is_verified boolean not null default false,
created_at timestamptz not null default now(),
Expand Down Expand Up @@ -220,7 +225,7 @@ $$ language sql stable security definer set search_path to pg_catalog, public, p
create table app_public.user_emails (
id serial primary key,
user_id int not null default app_public.current_user_id() references app_public.users on delete cascade,
email citext not null check (email ~ '[^@]+@[^@]+\.[^@]+'),
email app_public.email not null,
is_verified boolean not null default false,
is_primary boolean not null default false,
created_at timestamptz not null default now(),
Expand Down Expand Up @@ -457,7 +462,7 @@ $$ language plpgsql security definer volatile set search_path to pg_catalog, pub
/**********/

create table app_private.unregistered_email_password_resets (
email citext constraint unregistered_email_pkey primary key,
email app_public.email constraint unregistered_email_pkey primary key,
attempts int not null default 1,
latest_attempt timestamptz not null
);
Expand All @@ -470,7 +475,7 @@ comment on column app_private.unregistered_email_password_resets.latest_attempt

/**********/

create function app_public.forgot_password(email citext) returns void as $$
create function app_public.forgot_password(email app_public.email) returns void as $$
declare
v_user_email app_public.user_emails;
v_token text;
Expand Down Expand Up @@ -558,7 +563,7 @@ begin
end;
$$ language plpgsql strict security definer volatile set search_path to pg_catalog, public, pg_temp;

comment on function app_public.forgot_password(email public.citext) is
comment on function app_public.forgot_password(email app_public.email) is
E'If you''ve forgotten your password, give us one of your email addresses and we''ll send you a reset token. Note this only works if you have added an email address!';

/**********/
Expand Down Expand Up @@ -747,10 +752,10 @@ grant execute on function app_public.change_password(text, text) to :DATABASE_VI

create function app_private.really_create_user(
username citext,
email text,
email app_public.email,
email_is_verified bool,
name text,
avatar_url text,
avatar_url app_public."URL",
password text default null
) returns app_public.users as $$
declare
Expand Down Expand Up @@ -787,7 +792,7 @@ begin
end;
$$ language plpgsql volatile set search_path to pg_catalog, public, pg_temp;

comment on function app_private.really_create_user(username citext, email text, email_is_verified bool, name text, avatar_url text, password text) is
comment on function app_private.really_create_user(username citext, email app_public.email, email_is_verified bool, name text, avatar_url app_public."URL", password text) is
E'Creates a user account. All arguments are optional, it trusts the calling method to perform sanitisation.';

/**********/
Expand All @@ -801,10 +806,10 @@ create function app_private.register_user(
) returns app_public.users as $$
declare
v_user app_public.users;
v_email citext;
v_email app_public.email;
v_name text;
v_username citext;
v_avatar_url text;
v_avatar_url app_public."URL";
v_user_authentication_id int;
begin
-- Extract data from the user’s OAuth profile data.
Expand Down Expand Up @@ -874,9 +879,9 @@ create function app_private.link_or_register_user(
declare
v_matched_user_id int;
v_matched_authentication_id int;
v_email citext;
v_email app_public.email;
v_name text;
v_avatar_url text;
v_avatar_url app_public."URL";
v_user app_public.users;
v_user_email app_public.user_emails;
begin
Expand Down
6 changes: 5 additions & 1 deletion @app/graphql/codegen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ documents: "../client/src/**/*.graphql"
config:
scalars:
Datetime: "string"
Email: "string"
URL: "string"
JSON: "{ [key: string]: any }"
noGraphQLTag: false
withHOC: false
Expand All @@ -12,7 +14,9 @@ config:
generates:
index.tsx:
plugins:
- add: "/* DO NOT EDIT! This file is auto-generated by graphql-code-generator - see `codegen.yml` */"
- add:
"/* DO NOT EDIT! This file is auto-generated by graphql-code-generator
- see `codegen.yml` */"
- "typescript"
- "typescript-operations"
- "typescript-react-apollo"
6 changes: 6 additions & 0 deletions @app/server/src/middleware/installPostGraphile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { Express, Request, Response } from "express";
import PgPubsub from "@graphile/pg-pubsub";
import PgSimplifyInflectorPlugin from "@graphile-contrib/pg-simplify-inflector";
import GraphilePro from "@graphile/pro"; // Requires license key
import PgTypeEmailPlugin from "../plugins/PgTypeEmailPlugin";
import PgTypeUrlPlugin from "../plugins/PgTypeUrlPlugin";
import PassportLoginPlugin from "../plugins/PassportLoginPlugin";
import PrimaryKeyMutationsOnlyPlugin from "../plugins/PrimaryKeyMutationsOnlyPlugin";
import SubscriptionsPlugin from "../plugins/SubscriptionsPlugin";
Expand Down Expand Up @@ -151,6 +153,10 @@ export function getPostGraphileOptions({
* https://www.graphile.org/postgraphile/extending/
*/
appendPlugins: [
// Exposes `Email` and `URL` types in the GraphQL schema
PgTypeEmailPlugin,
PgTypeUrlPlugin,

// Adds support for our `postgraphile.tags.json5` file
TagsFilePlugin,

Expand Down
4 changes: 2 additions & 2 deletions @app/server/src/plugins/PassportLoginPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ const PassportLoginPlugin = makeExtendSchemaPlugin(build => ({
typeDefs: gql`
input RegisterInput {
username: String!
email: String!
email: Email!
password: String!
name: String
avatarUrl: String
avatarUrl: URL
}

type RegisterPayload {
Expand Down
149 changes: 149 additions & 0 deletions @app/server/src/plugins/PgTypeEmailPlugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import { Plugin, Build, ScopeGraphQLScalarType } from "graphile-build";
import { PgType } from "graphile-build-pg";

declare module "graphile-build" {
interface ScopeGraphQLScalarType {
isEmailScalar?: boolean;
}
}

function isValidEmail(email: string) {
return /[^@]+@[^@]+\.[^@]+/.test(email);
}

export default (function PgTypeEmailPlugin(builder) {
builder.hook(
"build",
build => {
// This hook tells graphile-build-pg about the email database type so it
// knows how to express it in input/output.
const {
pgIntrospectionResultsByKind: rawIntrospectionResultsByKind,
pgRegisterGqlTypeByTypeId,
pgRegisterGqlInputTypeByTypeId,
pg2GqlMapper,
pgSql: sql,
} = build;

if (
!rawIntrospectionResultsByKind ||
!sql ||
!pgRegisterGqlTypeByTypeId ||
!pgRegisterGqlInputTypeByTypeId ||
!pg2GqlMapper
) {
throw new Error("Required helpers were not found on Build.");
}

const introspectionResultsByKind = rawIntrospectionResultsByKind;

// Get the 'email' type:
const emailType = introspectionResultsByKind.type.find(
(t: PgType) => t.name === "email"
);

if (!emailType) {
return build;
}

const emailTypeName = build.inflection.builtin("Email");

const GraphQLEmailType = makeGraphQLEmailType(
build as Build,
emailTypeName
);

// Now register the Email type with the type system for both output and input.
pgRegisterGqlTypeByTypeId(emailType.id, () => GraphQLEmailType);
pgRegisterGqlInputTypeByTypeId(emailType.id, () => GraphQLEmailType);

// Finally we must tell the system how to translate the data between PG-land and JS-land:
pg2GqlMapper[emailType.id] = {
// Turn string (from node-postgres) into email: no-op
map: (email: string) => email,
// When unmapping we need to convert back to SQL framgent
unmap: (email: string) =>
sql.fragment`(${sql.value(email)}::${sql.identifier(
emailType.namespaceName,
emailType.name
)})`,
};

return build;
},
["PgTypeEmail"],
[],
["PgTypes"]
);

/* End of email type */
} as Plugin);

function makeGraphQLEmailType(build: Build, emailTypeName: string) {
const {
graphql: { GraphQLScalarType, Kind },
} = build;
function parseValue(obj: unknown): string {
if (!(typeof obj === "string")) {
throw new TypeError(
`This is not a valid ${emailTypeName} object, it must be a string.`
);
}
if (!isValidEmail(obj)) {
throw new TypeError(
`This is not a properly formatted ${emailTypeName} object.`
);
}
return obj;
}

const parseLiteral: import("graphql").GraphQLScalarLiteralParser<any> = (
ast,
variables
) => {
switch (ast.kind) {
case Kind.STRING: {
const email = ast.value;
if (!isValidEmail(email)) {
throw new TypeError(
`This is not a properly formatted ${emailTypeName} object.`
);
}
return email;
}

case Kind.NULL:
return null;

case Kind.VARIABLE: {
const name = ast.name.value;
const email = variables ? variables[name] : undefined;
if (!isValidEmail(email)) {
throw new TypeError(
`This is not a properly formatted ${emailTypeName} object.`
);
}
return email;
}

default:
return undefined;
}
};

const scope: ScopeGraphQLScalarType = { isEmailScalar: true };
const GraphQLEmailType = build.newWithHooks(
GraphQLScalarType,
{
name: emailTypeName,
description:
"An address in the electronic mail system. Email addresses such as `[email protected]` are made up of a local-part, followed by an `@` symbol, followed by a domain.",
serialize: (email: string) => email,
parseValue,
parseLiteral,
},
scope
);

return GraphQLEmailType;
}
Copy link
Member

Choose a reason for hiding this comment

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

@singingwolfboy It seems to me that this plugin adds the Email type, with a comment, and performs validation on it. However we already perform validation on the type using the domain constraint you added; so I'm not sure if these 149 lines of code add much additional value? If this plugin were removed, would the following be sufficient?

create domain app_public.email as citext check(VALUE ~ '[^@]+@[^@]+\.[^@]+');
comment on domain app_public.email is
  'An address in the electronic mail system. Email addresses such as `[email protected]` are made up of a local-part, followed by an `@` symbol, followed by a domain.';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As I mentioned in the description of the pull request, the benefit of making an Email GraphQL scalar is that anyone introspecting the schema knows that the corresponding field won't accept just any string: it must be a valid email address. You're correct that this was already enforced at the database level, but because it wasn't exposed at the schema level, it could cause surprises. Using an Email scalar is a fairly trivial example, but consider that this repo is also meant to be an example of how to do things with PostGraphile: I think that learning how to create your own GraphQL scalar with PostGraphile is very useful!

For a more concrete example of how this could be useful, consider a front-end application that you can point at an arbitrary GraphQL schema, and it can generate a pretty HTML form for doing queries and mutations on that schema. Because it needs to work with arbitrary schemas, it can't have much knowledge about the structure of the schema that it's going to see: as a result, it makes sense to have this application look at the scalars in the schema, and generate form fields based on that. If we expose the User.email field as type String, this application could only generate a standard text field; but if we expose it as type Email, it could style the field differently and provide dynamic validation based on how emails are formatted.

There's a section in the Shopify GraphQL Design Tutorial about naming and scalars, which I think is relevant here. Semantic value can be really useful for GraphQL clients, so I think it makes sense to expose that semantic value as clearly as possible.

Copy link
Member

Choose a reason for hiding this comment

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

I’m cool with the custom scalar and totally understand the reasoning; for some reason I was expecting creating a custom domain to result in a custom scalar being created. Seems maybe I never implemented that https://github.com/graphile/graphile-engine/blob/v4/packages/graphile-build-pg/src/plugins/PgTypesPlugin.js#L844

Loading