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

[WIP] feat: ファイル書き出しのサポート #15

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions lib/download-utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
const fs = require('fs')
const util = require('util')
const path = require('path')

const downloadFileAndGetName = async (page, downloadPath, downloadSelector) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

downloadFileというメソッド名で戻り値がファイル名でもそんなに違和感がないので、AndGetNameはなくても良いかも

欲を言うとgetDownloadFileNamedownloadFileに分けて、「ファイル名を取得する」「ダウンロードする」という処理でメソッドを分けるととてもしっくりくる。

Copy link
Collaborator Author

@latica-jp latica-jp Aug 7, 2019

Choose a reason for hiding this comment

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

たしかに、andGetName なくてよさそうですね!

こちら、前述のものと同じく、書き出しなので download を export に直そうかと思います。とすると、

exportFile = async (page, exportSelector, downloadPath) かな?と。

処理順を考えるとセレクタが前で、パスはあとかな…

exportPath ではなくて downloadPath なのは、kintone での「書き出し」、ブラウザでの「ダウンロード」を区別している感じです。

// テンポラリのディレクトリを作成(ファイルのダウンロードの監視に空のディレクトリが都合がよいため)
const tempPath = path.resolve(downloadPath, require('uuid/v4')())
await util.promisify(fs.mkdir)(tempPath)
// パスを指定してダウンロード
await page._client.send('Page.setDownloadBehavior', {
behavior: 'allow',
downloadPath: tempPath,
})
await page.waitForSelector(downloadSelector, { visible: true })
await page.click(downloadSelector)
// ダウンロード完了の待機
const fileName = await waitFileDownloadAndGetName(tempPath)
// ダウンロードしたファイルを移動、テンポラリのダウンロードパスを削除
await util.promisify(fs.rename)(path.resolve(tempPath, fileName), path.resolve(downloadPath, fileName))
await util.promisify(fs.rmdir)(tempPath)
return fileName
}

const waitFileDownloadAndGetName = async downloadPath => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

【割と感覚的な名前の話】

  • メソッド名にwaitがあると、待つ以外の処理をしていると想像し辛い
  • xxx and xxx というメソッド名は2つの処理をしていることを示唆しているので、できれば避けたい
    • 一つのメソッドでは一つの処理のみを行う
  • getDownloadedFileName とかでも良いのかなと思う
    • ある程度時間がかかる(=waitする)のはawaitからも分かるので

とはいえこのファイルでしか参照されないローカルな関数なので、そこまで気にしなくてよいのかも

Copy link
Collaborator Author

@latica-jp latica-jp Aug 7, 2019

Choose a reason for hiding this comment

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

なるほど…

「指定したパスにファイルがダウンロードされてくるのを監視して、完了を検知したらファイル名を返す」という処理を端的に表すと…

wait より、もっと具体的に monitor とか detect とかかな。

detectFileDownload とか。

https://www.google.com/search?ei=UE5KXa-lEqGKr7wPo_m8wA8&q=detect+File+Download&oq=detect+File+Download

detect + file + download でぐぐると
そのものの記事がけっこう引っかかるところをみると、英語的にはありな感じかも。

動詞が「検知」なので、ファイルハンドルとか、ファイル名とか返すのは自然だと考えれば、-getName は不要かもですね。

Copy link
Collaborator

@t-kojima t-kojima Aug 7, 2019

Choose a reason for hiding this comment

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

detectFileDownload とか。

良いと思います 👍

const MAX_RETRY = 300
let retry = 0
do {
const dirents = await util.promisify(fs.readdir)(downloadPath, { withFileTypes: true })
const found = dirents.find(dirent => {
// ダウンロード中のファイルと隠しファイルを除外
return dirent.isFile() && !dirent.name.match(/\.crdownload$|(^|\/)\.[^/.]/)
})
if (found) return found.name
await new Promise(resolve => setTimeout(resolve, 200))
} while ((retry += 1) < MAX_RETRY)
return null
}

module.exports = {
downloadFileAndGetName,
}
65 changes: 65 additions & 0 deletions lib/export-csv.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
const { getDownloadFileUrl } = require('./url-utils')
const { downloadFileAndGetName } = require('./download-utils')

const exportToCsv = async (page, domain, appId, options = {}) => {
const url = getDownloadFileUrl(domain, appId)
const response = await page.goto(url, { waitUntil: 'networkidle2' })
if (response.status === 520) {
await page.close()
throw new Error(`Got 520 error while goto export screen. \
Maybe export file is not allowed to the test user.`)
}

if (options.additionalColumns) await addExportColumns(page, options.additionalColumns)

await Promise.all([page.waitForNavigation('.downloadlist-content-header-gaia'), page.click('#export-submit-gaia')])
let processed = false
let retries = 120 // 2 min
do {
await page.waitFor(1000)
// 再読み込みボタンだとクリック直後にダウンロードリンクが有効にならないため、ブラウザの再読み込みをつかう
await page.reload({ waitUntil: 'networkidle0' })
processed = await page.evaluate(() => {
const td = document.querySelector('#view-list-data-processing-gaia td > span')
return td && td.textContent === 'データがありません。'
})
} while (!processed && (retries -= 1))
return downloadFileAndGetName(page, options.path, 'a.download-image-gaia')
}

const addExportColumns = async (page, columnLabels) => {
for (const columnLabel of columnLabels) {
await addExportColumn(page, columnLabel)
}
}

const addExportColumn = async (page, columnLabel) => {
const selector = '#fm-field-toolitems-cybozu .fm-toolitem-gaia'
const index = await page.evaluate(
({ columnLabel, selector }) => {
const nodes = [...document.querySelectorAll(selector)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

return nodes.reduce((acc, node, index) => {
return node.textContent === columnLabel ? index : acc
}, -1)
},
{ columnLabel, selector }
)
const item = (await page.$$(selector))[index]
if (item) {
const itemBox = await item.boundingBox()
const x = itemBox.x + itemBox.width / 2
const y = itemBox.y + itemBox.height / 2
const columnsView = await page.$('.fm-row-gaia')
const columnsViewBox = await columnsView.boundingBox()

await page.mouse.move(x, y)
await page.mouse.down()
await page.waitFor(100)
await page.mouse.move(columnsViewBox.x + 5, y)
Copy link
Collaborator

Choose a reason for hiding this comment

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

おおお。。。すごい、苦労が垣間見れるメソッドだ。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

いやー、もう苦労しました…と言いたいところですが、けっこう単純でした 😄

await page.waitFor(100)
await page.mouse.up()
await page.waitFor(2000)
}
}

module.exports = { exportToCsv }
7 changes: 6 additions & 1 deletion lib/url-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ const getIndexUrl = (domain, appId, options = {}) => {
return `https://${domain}/k/${appId}/${queryString}`
}

const getDownloadFileUrl = (domain, appId, options = {}) => {
const queryString = toQueryString({ view: 20 })
return `https://${domain}/k/${appId}/exportRecord${queryString}`
Copy link
Collaborator

Choose a reason for hiding this comment

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

optionsをシカトしているのが気になる。
viewの指定なしでexportRecordを開くとデフォルトビューの内容でカラムが選択されるので、
options無しならデフォルトビューの内容でexportでもいい気がする。
指定されたらそのviewで出力する感じで。

Copy link
Collaborator

Choose a reason for hiding this comment

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

デフォルトビュー→ 一覧を表示したときデフォルトで表示されるビュー ≠ view:20
※念の為

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ここ、ちょっとまだ暫定なのですが、ご指摘のとおりと思います。
view=id つけなければ、デフォルトビューになるんですね。
あと、メソッド名は exportFileUrl にしたほうがいいかな、と思いました。
「書き出し」ですもんね。

Copy link
Collaborator

@t-kojima t-kojima Aug 7, 2019

Choose a reason for hiding this comment

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

あ、でも対となる処理にupload.jsがあるのでdownloadのままでもよいかも

もしexportにするならあちらもimportにして貰えると 🙏

}

const toQueryString = obj => qs.stringify(obj, { addQueryPrefix: true })

module.exports = { getCreateUrl, getIndexUrl }
module.exports = { getCreateUrl, getIndexUrl, getDownloadFileUrl }
36 changes: 36 additions & 0 deletions test/export-csv.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
const lib = require('../lib')
const path = require('path')
const fs = require('fs')
const util = require('util')
const { exportToCsv } = require('../lib/export-csv')
const { app, domain, username, password } = require('./config')

jest.setTimeout(60 * 1000)
/* eslint-disable no-console */
process.on('unhandledRejection', console.dir)

describe('#downloadFile', () => {
let page, appId, fileName, downloadPath

beforeAll(async () => {
appId = app['jinzo-ningen-test']
downloadPath = path.resolve()
page = await global.__BROWSER__.newPage()
await page.setViewport({ width: 1920, height: 980 })
lib.setConsole(page)
await lib.login(page, { domain, username, password })
})

it('ファイルが正常にダウンロードされたこと', async () => {
fileName = await exportToCsv(page, domain, appId, {
path: downloadPath,
additionalColumns: ['作成者', '更新者'],
})
expect(fileName).toBeTruthy()
})

afterAll(async () => {
const fullPath = `${downloadPath}/${fileName}`
await util.promisify(fs.unlink)(fullPath)
})
})
2 changes: 1 addition & 1 deletion test/settings/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const DIR = path.join(os.tmpdir(), 'jest_puppeteer_global_setup')

module.exports = async function() {
console.info(chalk.green('Setup Puppeteer'))
const browser = await puppeteer.launch({ headless: true })
const browser = await puppeteer.launch({ headless: false })
global.__BROWSER__ = browser
mkdirp.sync(DIR)
fs.writeFileSync(path.join(DIR, 'wsEndpoint'), browser.wsEndpoint())
Expand Down