feat: add action attachment.storeDecryptedPdf#381
feat: add action attachment.storeDecryptedPdf#381ahochsteger merged 9 commits intoahochsteger:mainfrom
Conversation
|
@ahochsteger I started adding the feature to the gmail-processor and would like to ask for some guidance. |
|
@MikeDabrowski thanks for the PR - so far it looks good to me for the start. For local testing using Jest tests I usually mock services provided by GAS like Give me some time to try it out myself and I'll give you some more guidance or maybe directly change some things myself in case I feel that it may be a bit tricky to solve. |
ahochsteger
left a comment
There was a problem hiding this comment.
I've reviewed the changes and put some comments into the code.
But I still have to think about how to support async functions in Gmail Processor actions though ...
In case you've got some ideas I'm all ears ;-).
src/lib/actions/AttachmentActions.ts
Outdated
| }, | ||
| ) | ||
| }); | ||
| const actionMeta = context.proc.gdriveAdapter.getActionMeta( |
There was a problem hiding this comment.
I guess this is the tricky part here, since getDecryptedPdf is an async function that has been setup but not yet executed when reaching this line, so we cannot tell, if the decryption was successful and return the correct state.
This is something we have to check and test in-depth and find a good way to allow async actions in general, since this currently really limits what can be used in Gmail Processor actions.
There was a problem hiding this comment.
I am sorry I do not have any idea how to solve that problem. I know that GAS has at least some partial support for top level await but probably using that would turn most of this lib to async/await functions - which will be a lot of work
src/lib/actions/AttachmentActions.ts
Outdated
|
|
||
| /** Store and decrypt an attachment to a Google Drive location. */ | ||
| @writingAction<AttachmentContext>() | ||
| public static storeAndDecrypt( |
There was a problem hiding this comment.
When certain changes affect the documentation and the configuration schema which is auto-generated.
That's why it is recommended to run npm run pre-commit that takes care of that and also runs all the local tests to verify if everything is still ok.
Also have a look at the Development Guide for more information.
There was a problem hiding this comment.
Many scripts are not cross platform I am afraid.
I switched to linux but running the pre-commit throws lots of errors of missing dependencies.
[docs] scripts/update-docs.sh: line 33: gojq: command not found
[docs] npm run update:docs exited with code 127
[examples] scripts/update-examples.sh: line 20: gojq: command not found
..and more
| */ | ||
| decryptedPdfDescription?: string | ||
| } | ||
|
|
There was a problem hiding this comment.
Just some ideas about the features and configurability:
- We may let the user decide, if both files should be kept or just the decrypted version (in which case just
locationwould be enough to set. - Is there really a use case to provide a different description for the decrypted file instead of just use
description?
There was a problem hiding this comment.
I agree with the first point - Does saveOriginal sound good?
And I don't have any use case for desc of decrypted file. But I also do not have any use case for descriptions in general. Just followed the pattern here
src/lib/utils/PdfLibUtils.ts
Outdated
| export async function getDecryptedPdf(processedFileObject: GoogleAppsScript.Gmail.GmailAttachment, password: string) { | ||
| const bytes = processedFileObject.getBytes(); | ||
| const fileBase64 = Utilities.base64Encode(bytes); | ||
| const pdfDoc = await PDFDocument.load(fileBase64, { password, ignoreEncryption: true}); |
There was a problem hiding this comment.
I'd support Pattern substitution for the password to be able to set it as variable in the global settings (or maybe provide a way in the future to securely provide secrets) without having to hard-code them inside the action configuration.
There was a problem hiding this comment.
I thought having it defined in the action args is the right way - you may have different passwords for different actions/emails/pdfs. How would you marry them if they were placed in the global settings?
Just from the top of my head - the web IDE of GAS mentioned me at some point that top level await is available. So GAS have at least some async support built in already. When I was hacking the decrypting without gmail-processor I just made the outer function async and went with it. I assume that doing the same here might not be so easy - this lib is fare more complex than I thought initially. However, even if you'd have to make every single fn async, I suppose it would still be usable. At least in the basic way, as described in the docs. The way I am using it is I just have an outer function The other idea, the 'kinda works' idea, is to put the async stuff into separate action, just add then and leave it be. Just make sure that whatever and whenever it does its thing it won't impact any other process. But lets leave it as the last resort option, it is not really for production code :/ Thanks for the review, I'll try to carve some time to address it in the next few days |
4376bc2 to
c88ac73
Compare
Pull Request Test Coverage Report for Build 13615723321Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
Have you figured a way to handle async functions ? |
|
@MikeDabrowski I was able to do update this PR and it is now in a functional state using an async custom action. function decryptPdfRun() {
const config = {
description:
"The action `custom.decryptAndStorePdf` decrypts and stores a PDF file.",
settings: {
markProcessedMethod: "mark-read",
},
global: {
thread: {
match: {
query:
"has:attachment -in:trash -in:drafts -in:spam after:{{date.now|formatDate('yyyy-MM-dd')}} is:unread subject:\"[GmailProcessor-Test] decryptPdf\"",
},
},
},
threads: [
{
match: {
query: "subject:([GmailProcessor-Test] decryptPdf)",
},
attachments: [
{
description: "Process all attachments named 'encrypted*.pdf'",
match: {
name: "(?<basename>encrypted.*)\\.pdf$",
},
actions: [
{
name: "custom.decryptAndStorePdf",
args: {
location:
"/GmailProcessor-Tests/e2e/advanced/{{message.date|formatDate('yyyy-MM-dd')}}/decrypted.pdf",
conflictStrategy: "replace",
password: "test",
},
},
],
},
],
},
],
}
const customActions = [
{
name: "decryptAndStorePdf",
action: async (ctx, args) => {
const location = args.location
try {
ctx.log.info(`decryptAndStorePdf(): location=${location}`)
const attachment = ctx.attachment.object
const base64Content = ctx.env.utilities.base64Encode(
attachment.getBytes(),
)
ctx.log.info(`decryptAndStorePdf(): Loading PDF document ...`)
const pdfDoc = await GmailProcessorLib.PDFDocument.load(
base64Content,
{
password: args.password,
ignoreEncryption: true,
},
)
ctx.log.info(`decryptAndStorePdf(): Decrypt PDF content ...`)
const decryptedContent = await pdfDoc.save()
ctx.log.info(`decryptAndStorePdf(): Create new PDF blob ...`)
const decryptedPdf = ctx.env.utilities.newBlob(
decryptedContent,
attachment.getContentType(),
attachment.getName(),
)
ctx.log.info(
`decryptAndStorePdf(): Store PDF file to '${location}' ...`,
)
return ctx.proc.gdriveAdapter.createFileFromAction(
ctx,
args.location,
decryptedPdf,
args.conflictStrategy,
args.description,
"decrypted PDF",
"custom",
"custom.decryptAndStorePdf",
)
} catch (e) {
ctx.log.error(
`Error while saving decrypted pdf to ${location}: ${e}`,
)
throw e
}
},
},
]
return GmailProcessorLib.run(config, "dry-run", customActions)
} |
|
To summarize, these topics I'd like to address before releasing it (added to the description as well):
|
|
@MikeDabrowski I was now able to fully integrate it as a async action {
"name": "attachment.storeDecryptedPdf",
"args": {
"location": "decrypted.pdf",
"conflictStrategy": "replace",
"password": "...",
},
}I intentionally left all additional properties to store both the original and the decrypted version out to keep the implementation as simple as possible. The original version may stored by an additional |
|
@ahochsteger Brilliant work! Thank you for completing this. I can confirm that version 35 is working for me in my 'production' case! |
Description
This PR intends to add decryptPDF action. It will take attached pdfs, store the original and decrypted in the chosen location.
It also adds new dependency
@cantoo/pdf-libfork that allows for pdf decrypting. This library uses promises, which makes decrypting the pdf async as well. The rest of the code is synchronous.Fixes #355)
Type of change
How has this been tested?
TODOs
ctx.env.pdfDocument.Checklist