Skip to content

Commit

Permalink
complete auto cleaner
Browse files Browse the repository at this point in the history
  • Loading branch information
Hoie Kim committed Oct 2, 2020
1 parent bedbc4f commit 89c5007
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 18 deletions.
13 changes: 9 additions & 4 deletions database/elasticsearch/elastic.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ es.deleteAccount = async (account) => {
`/_security/role/${account.username}`,
"DELETE"
);
const r3 = await sendESRequest(`/${account.username}-ah*`, "DELETE");
const err = r1.error || r2.error;
if (err || !r1.found || !r2.found) {
logger.error("Deleting Elasticsearch user failed");
Expand All @@ -94,13 +95,17 @@ es.deleteAccount = async (account) => {
};

es.checkAccount = async (account) => {
if (!account.username || !account.password || !account.email) {
const username = account.username;
if (!username) {
logger.error("Account data is invalid");
throw new Error("Account data is invalid");
}
const index = account.username + "-example";
const r1 = await sendESRequest(`/${index}`, "GET");
if (r1[index]) return true;
const r1 = await sendESRequest(`/_security/user/${username}`, "GET");
logger.info(
`Checking Elasticsearch account for ${username} result:`,
!!r1[username]
);
if (r1[username]) return true;
return false;
};

Expand Down
2 changes: 1 addition & 1 deletion database/elasticsearch/elastic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ describe("testing es check account function", () => {
fetch.mockReturnValue(
Promise.resolve({
json: () => {
return { "testuser-example": {} };
return { testuser: {} };
},
})
);
Expand Down
4 changes: 1 addition & 3 deletions database/postgres/pg.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,10 @@ pgModule.deletePgAccount = async (account) => {
};

pgModule.userHasPgAccount = async (username) => {
logger.info(`checking to see if ${username} has a pg account`);
const result = await client.query(`SELECT 1 FROM pg_roles WHERE rolname=$1`, [
username,
]);

logger.info(`result: ${result.rowCount}`);
logger.info(`Checking pg account for ${username} result: ${result.rowCount}`);
return Boolean(result.rowCount);
};

Expand Down
26 changes: 17 additions & 9 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,26 @@ util.cleanAnonymous = async () => {
},
});
logger.info(`${userAccount.length} expired accounts are found`);
await userAccount.reduce(async (acc, e) => {
await acc;
await pg.deletePgAccount(e);
await es.deleteAccount(e);
await e.destroy();
return Promise.resolve();
}, Promise.resolve());
logger.info("Cleaning expired accounts has completed");
return Promise.all(
userAccount.map(async (e) => {
const pgDbExists = await pg.userHasPgAccount(e.username);
if (pgDbExists) await pg.deletePgAccount(e);
const esDbExists = await es.checkAccount(e);
if (esDbExists) await es.deleteAccount(e);
return await e.destroy();
})
).then(() => {
logger.info("Cleaning expired accounts has completed");
const timer = setTimeout(util.cleanAnonymous, 24 * 60 * 60 * 1000);
return {
stop: () => {
clearTimeout(timer);
},
};
});
} catch (err) {
logger.error("Cleaning expired accounts has failed", err);
}
setTimeout(util.cleanAnonymous, 24 * 60 * 60 * 1000);
};

module.exports = util;
14 changes: 14 additions & 0 deletions lib/util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ describe("Testing cleanAnonymous function", () => {
jest.clearAllMocks();
});
it("should call findAll with correct queries", async () => {
// Hi future engineers, I wrote some strict test here because I thought
// that it's important to protect this query. Someone can accidently
// clear up the whole user account database by making a little change.
// If you are changing this, please do so with caution and plentiful of tests.
Accounts.findAll.mockReturnValue([]);
await util.cleanAnonymous();
const args = Accounts.findAll.mock.calls[0][0];
Expand All @@ -42,8 +46,18 @@ describe("Testing cleanAnonymous function", () => {
);
expect(args.where.and[1].email).toBeNull();
});
it("should not call database functions if expired accounts are found but database does not exist", async () => {
Accounts.findAll.mockReturnValue([{ destroy: () => {} }]);
pg.userHasPgAccount = () => false;
es.checkAccount = () => false;
await util.cleanAnonymous();
expect(pg.deletePgAccount).not.toHaveBeenCalled();
expect(es.deleteAccount).not.toHaveBeenCalled();
});
it("should call database functions if expired accounts are found", async () => {
Accounts.findAll.mockReturnValue([{ destroy: () => {} }]);
pg.userHasPgAccount = () => true;
es.checkAccount = () => true;
await util.cleanAnonymous();
expect(pg.deletePgAccount).toHaveBeenCalled();
expect(es.deleteAccount).toHaveBeenCalled();
Expand Down
5 changes: 4 additions & 1 deletion src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const { database } = require("./routes/renderRoutes");
require("dotenv").config();
let server = null;
let app = null;
let cleaner = null;

const neo4jModule = require("../database/neo4j/neo4j");

const getApp = () => {
Expand All @@ -28,7 +30,7 @@ const startServer = async (portNumber) => {
await pgModule.startPGDB();
await neo4jModule.startNeo4j();

util.cleanAnonymous();
cleaner = await util.cleanAnonymous();

return new Promise((resolve, reject) => {
app = express();
Expand Down Expand Up @@ -82,6 +84,7 @@ const startServer = async (portNumber) => {

const stopServer = () => {
return new Promise((resolve, reject) => {
cleaner.stop();
dbModule.close();
pgModule.closePGDB();
neo4jModule.closeNeo4j();
Expand Down
11 changes: 11 additions & 0 deletions src/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ jest.mock("express");
jest.mock("mailgun-js");
jest.mock("../sequelize/db");
jest.mock("../database/postgres/pg");
jest.mock("../database/elasticsearch/elastic");

const express = require("express");
const dbModule = require("../sequelize/db");
const util = require("../lib/util");
const userRoutes = require("./routes/userRoutes");
const renderRoutes = require("./routes/renderRoutes");

Expand All @@ -25,6 +27,15 @@ const { startServer, stopServer, getApp } = require("./server");

dbModule.start = jest.fn();
dbModule.close = jest.fn();
dbModule.getModels = () => {
return {
Accounts: {
findAll: () => {
return [];
},
},
};
};

const app = {
set: () => {},
Expand Down

0 comments on commit 89c5007

Please sign in to comment.