-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add computed fields to SQL #92
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -25,50 +25,61 @@ interface File { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
url_path: string | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
filetype: string | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
metadata: MetaData | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[key: string]: string | null | MetaData | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class MddbFile { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
static table = Table.Files; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
static supportedExtensions = ["md", "mdx"]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_id: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
file_path: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
extension: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
url_path: string | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// TODO there should be a separate table for filetypes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// and another one for many-to-many relationship between files and filetypes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
filetype: string | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
metadata: MetaData | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
file: File = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_id: "", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
file_path: "", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
extension: "", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
url_path: null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
filetype: null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
metadata: null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't do this... It's not necessary at all. If you want to be able to assign dynamic properties I think you can just add this to the class:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this way we lose tract of all fields, it's like using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's not what I meant. Use original properties + this one for dynamic ones (aka computed fields) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// TODO type? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
constructor(file: any) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._id = file._id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.file_path = file.file_path; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.extension = file.extension; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.url_path = file.url_path; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.filetype = file.filetype; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.metadata = file.metadata ? JSON.parse(file.metadata) : null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.file._id = file._id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.file.file_path = file.file_path; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.file.extension = file.extension; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.file.url_path = file.url_path; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.file.filetype = file.filetype; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.file.metadata = file.metadata ? JSON.parse(file.metadata) : null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Assign dynamic properties using index signature to this.file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for (const key in file) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
key !== "_id" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
key !== "file_path" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
key !== "extension" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
key !== "url_path" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
key !== "filetype" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
key !== "metadata" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.file[key] = file[key]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this could be much simpler:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
toObject(): File { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_id: this._id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
file_path: this.file_path, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
extension: this.extension, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
url_path: this.url_path, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
filetype: this.filetype, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
metadata: this.metadata, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return { ...this.file }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
static async createTable(db: Knex) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
static async createTable(db: Knex, properties: string[]) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const creator = (table: Knex.TableBuilder) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("_id").primary(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("file_path").unique().notNullable(); // Path relative to process.cwd() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("extension").notNullable(); // File extension | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("url_path"); // Sluggfied path relative to content folder | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("filetype"); // Type field in frontmatter if it exists | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("metadata"); // All frontmatter data | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("file_path").unique().notNullable(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("extension").notNullable(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("url_path"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("filetype"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string("metadata"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for (let index = 0; index < properties.length; index++) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
table.string(properties[index]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if a computed property returns an array or a boolean? Shouldn't it be
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see what you are saying, I thought about a couple of scenarios:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, use any SQL types that are most suited for each JS/TS type. But it's not like everything needs to be stringified 😅 If it's an object/array, stringify to JSON.
You could type the computed field function signature in a way that it always should return a single type defined by the user. But for now, ok, just stringify everything. We'll properly implement it when we move computed fields definitions to their final destination = document types. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but the issue is currently the current implementation of |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const tableExists = await db.schema.hasTable(this.table); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -88,15 +99,58 @@ class MddbFile { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!areUniqueObjectsByKey(files, "file_path")) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
throw new Error("Files must have unique file_path"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const serializedFiles = files.map((file) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
...file, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
metadata: JSON.stringify(file.metadata), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const serializedFile: any = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const defaultProperties = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"_id", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"file_path", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"extension", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"url_path", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"filetype", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for (const key in file) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (Object.prototype.hasOwnProperty.call(file, key)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
mohamedsalem401 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const value = file[key]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// If the value is undefined, default it to null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
serializedFile[key] = defaultProperties.includes(key) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
? value | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
: value !== undefined | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
? JSON.stringify(value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
: null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
mohamedsalem401 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return serializedFile; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return db.batchInsert(Table.Files, serializedFiles); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
get _id(): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return this.file._id; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
get file_path(): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return this.file.file_path; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
get extension(): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return this.file.extension; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
get url_path(): string | null { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return this.file.url_path; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
get filetype(): string | null { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return this.file.filetype; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
get metadata(): MetaData | null { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return this.file.metadata; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
mohamedsalem401 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
interface Link { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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.