FixPostgreSQLSessionStorage@hasSessionTable always return false
#413
Conversation
ghost
added
the
|
I have signed the shopify CLA. |
|
@surma Hi, it would be great if you could help review my PR at your convenience? |
|
Thanks for the fix chuangbo, i need it as well 🙏 |
|
Looking forward to see this merged, it's blocking my current development as well. |
|
@yoan-elba @Eiles We are currently making a local copy with fixes to make it work temporally. import pg from "pg";
import { SessionInterface } from "@shopify/shopify-api";
import { SessionStorage } from "@shopify/shopify-api/dist/auth/session/session_storage.js";
import {
sessionEntries,
sessionFromEntries,
} from "@shopify/shopify-api/dist/auth/session/session-utils.js";
export interface PostgreSQLSessionStorageOptions {
sessionTableName: string;
port: number;
}
const defaultPostgreSQLSessionStorageOptions: PostgreSQLSessionStorageOptions =
{
sessionTableName: "shopify_sessions",
port: 3211,
};
export class PostgreSQLSessionStorage implements SessionStorage {
static withCredentials(
host: string,
dbName: string,
username: string,
password: string,
opts: Partial<PostgreSQLSessionStorageOptions>
) {
return new PostgreSQLSessionStorage(
new URL(
`postgres://${encodeURIComponent(username)}:${encodeURIComponent(
password
)}@${host}/${encodeURIComponent(dbName)}`
),
opts
);
}
public readonly ready: Promise<void>;
private options: PostgreSQLSessionStorageOptions;
private client: pg.Client;
constructor(
private dbUrl: URL,
opts: Partial<PostgreSQLSessionStorageOptions> = {}
) {
if (typeof this.dbUrl === "string") {
this.dbUrl = new URL(this.dbUrl);
}
this.options = { ...defaultPostgreSQLSessionStorageOptions, ...opts };
// fix: move from init() to constructor as typescript throws error
this.client = new pg.Client({ connectionString: this.dbUrl.toString() });
this.ready = this.init();
}
public async storeSession(session: SessionInterface): Promise<boolean> {
await this.ready;
const entries = sessionEntries(session);
const query = `
INSERT INTO ${this.options.sessionTableName}
(${entries.map(([key]) => key).join(", ")})
VALUES (${entries.map((_, i) => `$${i + 1}`).join(", ")})
ON CONFLICT (id) DO UPDATE SET ${entries
.map(([key]) => `${key} = Excluded.${key}`)
.join(", ")};
`;
await this.query(
query,
entries.map(([_key, value]) => value)
);
return true;
}
public async loadSession(id: string): Promise<SessionInterface | undefined> {
await this.ready;
const query = `
SELECT * FROM ${this.options.sessionTableName}
WHERE id = $1;
`;
const rows = await this.query(query, [id]);
if (!Array.isArray(rows) || rows?.length !== 1) return undefined;
const rawResult = rows[0] as any;
return sessionFromEntries(Object.entries(rawResult));
}
public async deleteSession(id: string): Promise<boolean> {
await this.ready;
const query = `
DELETE FROM ${this.options.sessionTableName}
WHERE id = $1;
`;
await this.query(query, [id]);
return true;
}
public disconnect(): Promise<void> {
return this.client.end();
}
private async init() {
// fix: move this to constructor as typescript throws error
// this.client = new pg.Client({ connectionString: this.dbUrl.toString() });
await this.connectClient();
await this.createTable();
}
private async connectClient(): Promise<void> {
await this.client.connect();
}
private async hasSessionTable(): Promise<boolean> {
const query = `
SELECT * FROM pg_catalog.pg_tables WHERE tablename = $1
`;
// fix: change [rows] to rows
// see https://github.com/Shopify/shopify-api-node/pull/413
const rows = await this.query(query, [this.options.sessionTableName]);
return Array.isArray(rows) && rows.length === 1;
}
private async createTable() {
const hasSessionTable = await this.hasSessionTable();
if (!hasSessionTable) {
const query = `
CREATE TABLE ${this.options.sessionTableName} (
id varchar(255) NOT NULL PRIMARY KEY,
shop varchar(255) NOT NULL,
state varchar(255) NOT NULL,
isOnline boolean NOT NULL,
scope varchar(255),
expires integer,
onlineAccessInfo varchar(255),
accessToken varchar(255)
)
`;
await this.query(query);
}
}
private async query(sql: string, params: any[] = []): Promise<any> {
const result = await this.client.query(sql, params);
return result.rows;
}
} |
|
@chuangbo Thanks for the tips, i will do that for this moment, I hope your PR will be merge soon |
|
@paulomarg many thanks to the merge and the fix to my broken English 😃 |
|
Hey @paulomarg, I'm still having this issue on v6.0.1. Thanks |
|
It seems this fix wasn't carried over when we separated the package here. PR is up! |
|
@paulomarg, when should we expect the release with the Postgres fix? |
|
@paulomarg, I don't know if it's just me, but I'm still having the issue on v6.0.2. Here's the full error message: |
|
Yes, I'm also getting the same error message @alexissel This error currently prevents my app from re-deploying if the shopify_sessions table already exists |
|
I'm also getting the same error. |
|
but right now in development this is working for me :) |
|
I have ended up monkey-patching 1.0.1 version like below, as anyways I don't want the table to be created automatically, in my case its created alongside other app-related tables via version-controlled schema, also the service account running the app will not enough privileges to create tables only Read/Write access,
|
|
Any updates here? I just received this error using Is there a workaround? EDIT: This seems odd looking at the code: Is my version correct? |
Can also confirm this seems to be working.. would love for this to be a full release by the time my app hits production :) |
WHY are these changes introduced?
Fixes #412
PostgreSQLSessionStorage@hasSessionTablefunction callsPostgreSQLSessionStorage@queryand check the length of return rows. But thequeryfunction returnrowsdirectly instead of[rows], so after the array restructuring,rowswill always beundefined, which cause functionhasSessionTablereturns false even the table exists.WHAT is this pull request doing?
Remove array destructuring.
Type of change
Checklist