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

git storage: handle relocated files #4307

Merged
merged 1 commit into from Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions server/models/pages.js
Expand Up @@ -650,7 +650,15 @@ module.exports = class Page extends Model {
* @returns {Promise} Promise with no value
*/
static async movePage(opts) {
const page = await WIKI.models.pages.query().findById(opts.id)
let page
if (_.has(opts, 'id')) {
page = await WIKI.models.pages.query().findById(opts.id)
} else {
page = await WIKI.models.pages.query().findOne({
path: opts.path,
localeCode: opts.locale
})
}
if (!page) {
throw new WIKI.Error.PageNotFound()
}
Expand Down Expand Up @@ -704,9 +712,11 @@ module.exports = class Page extends Model {
const destinationHash = pageHelper.generateHash({ path: opts.destinationPath, locale: opts.destinationLocale, privateNS: opts.isPrivate ? 'TODO' : '' })

// -> Move page
const destinationTitle = (page.title === page.path ? opts.destinationPath : page.title)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the page has a title that was auto-generated from its filename, this updates its title to match the new filename.

await WIKI.models.pages.query().patch({
path: opts.destinationPath,
localeCode: opts.destinationLocale,
title: destinationTitle,
hash: destinationHash
}).findById(page.id)
await WIKI.models.pages.deletePageFromCache(page.hash)
Expand Down Expand Up @@ -775,7 +785,7 @@ module.exports = class Page extends Model {
})
}
if (!page) {
throw new Error('Invalid Page Id')
throw new WIKI.Error.PageNotFound()
}

// -> Check for page access
Expand Down
45 changes: 39 additions & 6 deletions server/modules/storage/git/storage.js
Expand Up @@ -142,7 +142,9 @@ module.exports = {
if (_.get(diff, 'files', []).length > 0) {
let filesToProcess = []
for (const f of diff.files) {
const fPath = path.join(this.repoPath, f.file)
const fMoved = f.file.split(' => ')
const fName = fMoved.length === 2 ? fMoved[1] : fMoved[0]
const fPath = path.join(this.repoPath, fName)
let fStats = { size: 0 }
try {
fStats = await fs.stat(fPath)
Expand All @@ -159,7 +161,8 @@ module.exports = {
path: fPath,
stats: fStats
},
relPath: f.file
oldPath: fMoved[0],
relPath: fName
})
}
await this.processFiles(filesToProcess, rootUser)
Expand All @@ -174,11 +177,25 @@ module.exports = {
async processFiles(files, user) {
for (const item of files) {
const contentType = pageHelper.getContentType(item.relPath)
const fileExists = await fs.pathExists(item.file)
const fileExists = await fs.pathExists(item.file.path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix for fileExists always being false.

if (!item.binary && contentType) {
// -> Page

if (!fileExists && item.deletions > 0 && item.insertions === 0) {
if (fileExists && item.relPath !== item.oldPath) {
// Page was renamed by git, so rename in DB
WIKI.logger.info(`(STORAGE/GIT) Page marked as renamed: from ${item.oldPath} to ${item.relPath}`)

const contentPath = pageHelper.getPagePath(item.oldPath)
const contentDestinationPath = pageHelper.getPagePath(item.relPath)
await WIKI.models.pages.movePage({
user: user,
path: contentPath.path,
destinationPath: contentDestinationPath.path,
locale: contentPath.locale,
destinationLocale: contentPath.locale,
skipStorage: true
})
} else if (!fileExists && item.deletions > 0 && item.insertions === 0) {
// Page was deleted by git, can safely mark as deleted in DB
WIKI.logger.info(`(STORAGE/GIT) Page marked as deleted: ${item.relPath}`)

Expand Down Expand Up @@ -207,7 +224,23 @@ module.exports = {
} else {
// -> Asset

if (!fileExists && ((item.before > 0 && item.after === 0) || (item.deletions > 0 && item.insertions === 0))) {
if (fileExists && ((item.before === item.after) || (item.deletions === 0 && item.insertions === 0))) {
// Asset was renamed by git, so rename in DB
WIKI.logger.info(`(STORAGE/GIT) Asset marked as renamed: from ${item.oldPath} to ${item.relPath}`)

const fileHash = assetHelper.generateHash(item.relPath)
const assetToRename = await WIKI.models.assets.query().findOne({ hash: fileHash })
if (assetToRename) {
await WIKI.models.assets.query().patch({
filename: item.relPath,
hash: fileHash
}).findById(assetToRename.id)
await assetToRename.deleteAssetCache()
} else {
WIKI.logger.info(`(STORAGE/GIT) Asset was not found in the DB, nothing to rename: ${item.relPath}`)
}
continue
Comment on lines +227 to +242
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should theoretically work for assets, but wasn't accessible in my testing because diff.files earlier in the file wasn't being populated when a binary was renamed. (Possibly related: steveukx/git-js#243)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a limitation of git-js, which doesn't notice binaries being renamed: steveukx/git-js#885

} else if (!fileExists && ((item.before > 0 && item.after === 0) || (item.deletions > 0 && item.insertions === 0))) {
// Asset was deleted by git, can safely mark as deleted in DB
WIKI.logger.info(`(STORAGE/GIT) Asset marked as deleted: ${item.relPath}`)

Expand Down Expand Up @@ -419,7 +452,7 @@ module.exports = {
transform: async (page, enc, cb) => {
const pageObject = await WIKI.models.pages.query().findById(page.id)
page.tags = await pageObject.$relatedQuery('tags')

let fileName = `${page.path}.${pageHelper.getFileExtension(page.contentType)}`
if (WIKI.config.lang.namespacing && WIKI.config.lang.code !== page.localeCode) {
fileName = `${page.localeCode}/${fileName}`
Expand Down