-
Notifications
You must be signed in to change notification settings - Fork 11
[DNM] closes #232 - Arango functions to stop/start server, and create/delete account #233
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
85f2c65
closes #232 - init commit, add arango functions to start,close,create…
dc3c6b2
closes #232 - add more comments
f6d1322
closes #232 - altered createAccount function to take in object based …
244ea0f
closes #232 - remove comments i forgot to delete
1f9adc1
closes #232 - remove sendesreqeust functoin from elastic.js
f9c7584
closes #232 - fix syntax error on env variable
ec9b128
closes #232 - remove neo4j files
bd35628
closes #232 - add logger.info into file
99ef78e
Merge branch 'master' into arango
anthonykhoa fe193b8
Update elastic.js
anthonykhoa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
require("dotenv").config(); | ||
const { Database } = require("arangojs"); | ||
const { sendFetch } = require("../../lib/sendFetch"); | ||
const logger = require("./../../lib/log")(__filename); | ||
|
||
const arangoModule = {}; | ||
let db; | ||
|
||
arangoModule.checkIfDatabaseExists = async (name) => { | ||
// returns array of objects containing a '_name' key. | ||
// Ex [{ _name: 'lol' }, { _name: 'cakes' }] | ||
const names = await db.databases(); | ||
return names.map((db) => db._name).includes(name); | ||
}; | ||
|
||
arangoModule.startArangoDB = async () => { | ||
try { | ||
db = new Database({ | ||
url: process.env.ARANGO_URL, | ||
databaseName: "_system", | ||
auth: { | ||
username: process.env.ARANGO_USER, | ||
password: process.env.ARANGO_PW, | ||
}, | ||
}); | ||
logger.info("Connected to Arango Database"); | ||
} catch (err) { | ||
logger.error(err); | ||
throw new Error("Error in connecting to Arango Database", err); | ||
} | ||
}; | ||
|
||
arangoModule.closeArangoDB = () => { | ||
try { | ||
db.close(); | ||
logger.info("Successful closing of connection to Arango database"); | ||
} catch (err) { | ||
logger.error(err); | ||
throw new Error("Error closing Arango database", err); | ||
} | ||
}; | ||
|
||
arangoModule.createAccount = async (account) => { | ||
if (!account) return; | ||
const { username, dbPassword } = account; | ||
if (!username || !dbPassword) return; | ||
if (await arangoModule.checkIfDatabaseExists(username)) return; | ||
|
||
try { | ||
await db.createDatabase(username, { | ||
users: [{ username, password: dbPassword }], | ||
}); | ||
songz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
logger.info(`Successfully created Arango user and database ${username}`); | ||
} catch (err) { | ||
logger.error(err); | ||
throw new Error("Error in creating arango database :(", err); | ||
} | ||
}; | ||
|
||
arangoModule.deleteAccount = async (username) => { | ||
if (!username) return; | ||
if (!(await arangoModule.checkIfDatabaseExists(username))) return; | ||
|
||
try { | ||
// grabs JWT token for use in second fetch | ||
const { | ||
jwt, | ||
} = await sendFetch( | ||
`${process.env.ARANGO_URL}_db/_system/_open/auth`, | ||
"post", | ||
{ username: process.env.ARANGO_USER, password: process.env.ARANGO_PW } | ||
); | ||
|
||
// uses jwt token to authenticate request to delete user from arango database | ||
await sendFetch( | ||
`${process.env.ARANGO_URL}_db/_system/_api/user/${username}`, | ||
"delete", | ||
null, | ||
`bearer ${jwt}` | ||
); | ||
|
||
logger.info(`Successfully deleted Arango user ${username}`); | ||
// deletes database(username and database names are the same for each user) | ||
await db.dropDatabase(username); | ||
logger.info(`Successfully deleted Arango database ${username}`); | ||
} catch (err) { | ||
logger.error(err); | ||
throw new Error( | ||
"Sum ting wong... deletion of arango user has failed.", | ||
err | ||
); | ||
} | ||
}; | ||
|
||
module.exports = arangoModule; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
jest.mock("arangojs"); | ||
jest.mock("../../lib/sendFetch"); | ||
const { sendFetch } = require("../../lib/sendFetch"); | ||
const Arango = require("arangojs"); | ||
jest.mock("../../lib/log"); | ||
const logGen = require("../../lib/log"); | ||
const logger = { | ||
info: jest.fn(), | ||
error: jest.fn(), | ||
}; | ||
logGen.mockReturnValue(logger); | ||
jest.mock("dotenv"); | ||
Arango.Database = jest.fn().mockReturnValue({ | ||
close: jest.fn(), | ||
createDatabase: jest.fn(), | ||
}); | ||
|
||
const { | ||
startArangoDB, | ||
closeArangoDB, | ||
createAccount, | ||
deleteAccount, | ||
checkIfDatabaseExists, | ||
} = require("./arango"); | ||
|
||
describe("ArangoDB functions", () => { | ||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
test("should call Database function", () => { | ||
startArangoDB(); | ||
expect(Arango.Database).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
test("should retun false", async () => { | ||
const db = new Arango.Database(); | ||
db.databases = jest.fn().mockReturnValue([{ _name: "lol" }]); | ||
const res = await checkIfDatabaseExists("hi"); | ||
expect(res).toEqual(false); | ||
}); | ||
|
||
test("should retun true", async () => { | ||
const db = new Arango.Database(); | ||
db.databases = jest.fn().mockReturnValue([{ _name: "lol" }]); | ||
const res = await checkIfDatabaseExists("lol"); | ||
expect(res).toEqual(true); | ||
}); | ||
|
||
test("should call db.close function", () => { | ||
const db = new Arango.Database(); | ||
closeArangoDB(); | ||
expect(db.close).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
test("should call db.close function and log error", () => { | ||
const db = new Arango.Database(); | ||
db.close = jest.fn().mockImplementation(() => { | ||
throw new Error(); | ||
}); | ||
try { | ||
closeArangoDB(); | ||
} catch (err) {} | ||
expect(db.close).toHaveBeenCalledTimes(1); | ||
expect(logger.error).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
test("should call db.createDatabase function, logger.error when theres an \ | ||
error, and do nothing if argument is invalid", async () => { | ||
const db = new Arango.Database(); | ||
await createAccount(); | ||
expect(db.createDatabase).toHaveBeenCalledTimes(0); | ||
expect(logger.error).toHaveBeenCalledTimes(0); | ||
|
||
db.databases = jest.fn().mockReturnValue([{ _name: "lol" }]); | ||
await createAccount({ username: "lol", dbPassword: "hi" }); | ||
expect(db.createDatabase).toHaveBeenCalledTimes(0); | ||
|
||
db.databases = jest.fn().mockReturnValue([{ _name: "lol" }]); | ||
await createAccount({ username: "", dbPassword: "" }); | ||
expect(db.createDatabase).toHaveBeenCalledTimes(0); | ||
|
||
try { | ||
db.databases = jest.fn().mockReturnValue([{ _name: "lol" }]); | ||
await createAccount({ username: "hi", dbPassword: "hi" }); | ||
expect(db.createDatabase).toHaveBeenCalledTimes(1); | ||
|
||
db.createDatabase = jest.fn().mockImplementation(() => { | ||
throw new Error(); | ||
}); | ||
await createAccount({ username: "hi", dbPassword: "hi" }); | ||
} catch (err) {} | ||
expect(logger.error).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
test("should send two fetch requests if successful, logger.error when theres \ | ||
an error, and do nothing if argument is invalid", async () => { | ||
const db = new Arango.Database(); | ||
await deleteAccount(); | ||
expect(sendFetch).toHaveBeenCalledTimes(0); | ||
expect(logger.error).toHaveBeenCalledTimes(0); | ||
|
||
db.databases = jest.fn().mockReturnValue([{ _name: "lol" }]); | ||
sendFetch.mockReturnValue({ jwt: "lol" }); | ||
await deleteAccount("hi"); | ||
expect(sendFetch).toHaveBeenCalledTimes(0); | ||
|
||
db.dropDatabase = jest.fn().mockReturnValue("lol"); | ||
await deleteAccount("lol"); | ||
expect(sendFetch).toHaveBeenCalledTimes(2); | ||
expect(db.dropDatabase).toHaveBeenCalledTimes(1); | ||
|
||
try { | ||
sendFetch.mockImplementation(() => { | ||
throw new Error(); | ||
}); | ||
await deleteAccount("lol"); | ||
} catch (err) {} | ||
expect(logger.error).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
test("should call Database function and FAIL", () => { | ||
Arango.Database.mockImplementation(() => { | ||
throw new Error(); | ||
}); | ||
startArangoDB(); | ||
expect(logger.error).toHaveBeenCalledTimes(1); | ||
}); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this function requests all of the data from the database in JSON form and then search for the name. I think this can cause some inefficiency. Just like postgres and elasticsearch, I assume that there is a way to query the databases with specific name using arango.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes there is, I considered doing it that way, but eventually concluded that it is better to stick to using their arangojs API in order to keep the code more consistent.
Another way I could do this is by making a request to a http REST api that arango provides. This is how deleting an account is done -- I use their http REST API. Here are the pros and cons of using each method to check if a user exists:
Using http REST api:
const names = await db.databases()
, I don't have to usemap
andincludes
functions.Using arangojs api:
using .map and .includes
). Using http REST api, I would not need this type of additional logic.Efficiency
I believe there is no noticeable difference in efficiency between the two methods, unless learndatabases.dev gets a looot of users. You can see here that on average, it took arango only
1.07 seconds
to aggregate a collection of1,632,803
documents. Since efficiency is not a concern, I think using arangojs is better for now. In the future, if efficiency becomes a concern, then we can switch to using the http REST api that arango provides.Complexity
I think both methods are equally complex. With http REST api you have to first send a request to get a jwt token, and then use it in the next request to check if a user exists. With arangojs api, you have to filter through the array you receive after using
db.databases()
.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you said it won't be a big problem for now since we don't have a lot of requests but I want our app to be open to a chance to be so.
I have done some research and found something that can be a clue.
This function let us use AQL
This is how to get document that matches specific condition
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have already tried looking into using
AQL
commands to do user management, but I wasn't able to do so. This is why I used http rest API to delete arango users instead -- because I couldn't useAQL
commands to do so. It is possible for us to make a collection of users to query from, but that would make our code more complex than it has to be, and so we should not do that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#237