Skip to content

Commit

Permalink
Increase the open file limit before writing PDFs (#64)
Browse files Browse the repository at this point in the history 
**What is changing**: Check the OS-level open file limit (and adjust it
if necessary) prior to writing PDF files.

**Why this change is being made**: There is the potential for
wkhtmltopdf to need many open files (see
wkhtmltopdf/wkhtmltopdf#3081). This was fixed
in #14, where it depended on the order of events:
1. Stage the PDF file, getting the number of images
2. Check and adjust the open file limit
3. Flush the PDF file to disk

Number 3 was achieved via the call `defer outFile.Close()`:
https://github.com/tagatac/bagoup/blob/86f9b32870d2127f3fd3e196cecfc24265cf8d87/write.go#L29

However, #29 removed `Close()` from the `OutFile` interface, collapsing
`Stage()` and `Flush()` into a single function `Flush()` run prior to
checking and adjusting the open file limit.

**Related issue(s)**: Fixes #63 

**Follow-up changes needed**: None AFAIK

**Is the change completely covered by unit tests? If not, why not?**:
Yes
  • Loading branch information
@tagatac
tagatac authored Sep 10, 2024
1 parent ee3d3ae commit 2ec2203
Show file tree
Hide file tree
Showing 7 changed files with 184 additions and 55 deletions.
5 changes: 4 additions & 1 deletion example-exports/examplegen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,11 @@ func main() {
log.Panic(errors.Wrapf(err, "write message %q", msg))
}
}
if _, err := of.Flush(); err != nil {
if _, err := of.Stage(); err != nil {
log.Panic(errors.Wrap(err, "stage outfile"))
}
if err := of.Flush(); err != nil {
log.Panic(errors.Wrap(err, "flush outfile"))
}
}
}
30 changes: 20 additions & 10 deletions internal/bagoup/export_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,16 @@ func TestExportChats(t *testing.T) {
osMock.EXPECT().MkdirAll("messages-export/testdisplayname", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname/testguid;;;testguid2.txt").Return(chatFile, nil),
osMock.EXPECT().NewTxtOutFile(chatFile).Return(ofMocks[0]),
ofMocks[0].EXPECT().Flush(),
ofMocks[0].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[0].EXPECT().Flush(),
dbMock.EXPECT().GetMessageIDs(3),
osMock.EXPECT().MkdirAll("messages-export/testdisplayname2", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname2/testguid3.txt").Return(chatFile, nil),
osMock.EXPECT().NewTxtOutFile(chatFile).Return(ofMocks[1]),
ofMocks[1].EXPECT().Flush(),
ofMocks[1].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[1].EXPECT().Flush(),
)
},
},
Expand Down Expand Up @@ -113,8 +115,9 @@ func TestExportChats(t *testing.T) {
osMock.EXPECT().MkdirAll("messages-export/testdisplayname", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname/testguid;;;testguid2.txt").Return(chatFile, nil),
osMock.EXPECT().NewTxtOutFile(chatFile).Return(ofMocks[0]),
ofMocks[0].EXPECT().Flush(),
ofMocks[0].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[0].EXPECT().Flush(),
)
},
},
Expand Down Expand Up @@ -155,14 +158,16 @@ func TestExportChats(t *testing.T) {
osMock.EXPECT().MkdirAll("messages-export/testdisplayname", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname/testguid;;;testguid2.txt").Return(chatFile, nil),
osMock.EXPECT().NewTxtOutFile(chatFile).Return(ofMocks[0]),
ofMocks[0].EXPECT().Flush(),
ofMocks[0].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[0].EXPECT().Flush(),
dbMock.EXPECT().GetMessageIDs(3),
osMock.EXPECT().MkdirAll("messages-export/testdisplayname2", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname2/testguid3.txt").Return(chatFile, nil),
osMock.EXPECT().NewTxtOutFile(chatFile).Return(ofMocks[1]),
ofMocks[1].EXPECT().Flush(),
ofMocks[1].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[1].EXPECT().Flush(),
)
},
},
Expand Down Expand Up @@ -193,14 +198,16 @@ func TestExportChats(t *testing.T) {
osMock.EXPECT().MkdirAll("messages-export/testdisplayname", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname/testguid.txt").Return(chatFile, nil),
osMock.EXPECT().NewTxtOutFile(chatFile).Return(ofMocks[0]),
ofMocks[0].EXPECT().Flush(),
ofMocks[0].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[0].EXPECT().Flush(),
dbMock.EXPECT().GetMessageIDs(2),
osMock.EXPECT().MkdirAll("messages-export/testdisplayname", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname/testguid2.txt").Return(chatFile, nil),
osMock.EXPECT().NewTxtOutFile(chatFile).Return(ofMocks[1]),
ofMocks[1].EXPECT().Flush(),
ofMocks[1].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[1].EXPECT().Flush(),
)
},
},
Expand Down Expand Up @@ -228,8 +235,9 @@ func TestExportChats(t *testing.T) {
osMock.EXPECT().MkdirAll("messages-export/testdisplayname", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname/testguid.pdf").Return(chatFile, nil),
osMock.EXPECT().NewPDFOutFile(chatFile, gomock.Any(), false).Return(ofMocks[0]),
ofMocks[0].EXPECT().Flush(),
ofMocks[0].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[0].EXPECT().Flush(),
)
},
},
Expand Down Expand Up @@ -257,8 +265,9 @@ func TestExportChats(t *testing.T) {
osMock.EXPECT().MkdirAll("messages-export/testdisplayname", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname/testguid.pdf").Return(chatFile, nil),
osMock.EXPECT().NewPDFOutFile(chatFile, gomock.Any(), false).Return(ofMocks[0]),
ofMocks[0].EXPECT().Flush(),
ofMocks[0].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[0].EXPECT().Flush(),
)
},
},
Expand Down Expand Up @@ -287,8 +296,9 @@ func TestExportChats(t *testing.T) {
osMock.EXPECT().MkdirAll("messages-export/testdisplayname/attachments", os.ModePerm),
osMock.EXPECT().Create("messages-export/testdisplayname/testguid.txt").Return(chatFile, nil),
osMock.EXPECT().NewTxtOutFile(chatFile).Return(ofMocks[0]),
ofMocks[0].EXPECT().Flush(),
ofMocks[0].EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMocks[0].EXPECT().Flush(),
)
},
},
Expand Down
7 changes: 5 additions & 2 deletions internal/bagoup/write.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,18 @@ func (cfg *configuration) handleFileContents(outFile opsys.OutFile, messageIDs [
invalidCount++
}
}
imgCount, err := outFile.Flush()
imgCount, err := outFile.Stage()
if err != nil {
return errors.Wrapf(err, "flush chat file %q to disk", outFile.Name())
return errors.Wrapf(err, "stage chat file %q for writing", outFile.Name())
}
if openFilesLimit := cfg.OS.GetOpenFilesLimit(); imgCount*2 > openFilesLimit {
if err := cfg.OS.SetOpenFilesLimit(imgCount * 2); err != nil {
return errors.Wrapf(err, "chat file %q - increase the open file limit from %d to %d to support %d embedded images", outFile.Name(), openFilesLimit, imgCount*2, imgCount)
}
}
if err := outFile.Flush(); err != nil {
return errors.Wrapf(err, "flush chat file %q to disk", outFile.Name())
}
cfg.counts.files++
cfg.counts.messages += msgCount
cfg.counts.messagesInvalid += invalidCount
Expand Down
69 changes: 54 additions & 15 deletions internal/bagoup/write_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.txt"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush(),
ofMock.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock.EXPECT().Flush(),
)
},
wantJPGs: 1,
Expand All @@ -79,8 +80,9 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush(),
ofMock.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock.EXPECT().Flush(),
)
},
wantJPGs: 2,
Expand All @@ -107,9 +109,10 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush().Return(500, nil),
ofMock.EXPECT().Stage().Return(500, nil),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
osMock.EXPECT().SetOpenFilesLimit(1000),
ofMock.EXPECT().Flush(),
)
},
wantJPGs: 2,
Expand All @@ -136,8 +139,9 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.txt"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush(),
ofMock.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock.EXPECT().Flush(),
)
},
wantJPGs: 1,
Expand Down Expand Up @@ -165,8 +169,9 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2-1.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush(),
ofMock.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock.EXPECT().Flush(),
)
},
wantJPGs: 1,
Expand Down Expand Up @@ -244,7 +249,7 @@ func TestWriteFile(t *testing.T) {
wantErr: `write message "message2" to file "messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.txt": this is an outfile error`,
},
{
msg: "Flush error",
msg: "Staging error",
pdf: true,
setupMocks: func(dbMock *mock_chatdb.MockChatDB, osMock *mock_opsys.MockOS, ofMock *mock_opsys.MockOutFile) {
gomock.InOrder(
Expand All @@ -263,11 +268,11 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush().Return(0, errors.New("this is a staging error")),
ofMock.EXPECT().Stage().Return(0, errors.New("this is a staging error")),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
)
},
wantErr: `flush chat file "messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf" to disk: this is a staging error`,
wantErr: `stage chat file "messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf" for writing: this is a staging error`,
},
{
msg: "open files limit increase fails",
Expand All @@ -289,14 +294,42 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush().Return(500, nil),
ofMock.EXPECT().Stage().Return(500, nil),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
osMock.EXPECT().SetOpenFilesLimit(1000).Return(errors.New("this is a syscall error")),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
)
},
wantErr: `chat file "messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf" - increase the open file limit from 256 to 1000 to support 500 embedded images: this is a syscall error`,
},
{
msg: "Flush error",
pdf: true,
setupMocks: func(dbMock *mock_chatdb.MockChatDB, osMock *mock_opsys.MockOS, ofMock *mock_opsys.MockOutFile) {
gomock.InOrder(
osMock.EXPECT().MkdirAll("messages-export/friend", os.ModePerm),
osMock.EXPECT().Create("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf").Return(chatFile, nil),
osMock.EXPECT().NewPDFOutFile(chatFile, gomock.Any(), false).Return(ofMock),
dbMock.EXPECT().GetMessage(1, nil).Return("message1", true, nil),
ofMock.EXPECT().WriteMessage("message1"),
dbMock.EXPECT().GetMessage(2, nil).Return("message2", true, nil),
ofMock.EXPECT().WriteMessage("message2"),
osMock.EXPECT().FileExist("attachment1.heic").Return(true, nil),
osMock.EXPECT().HEIC2JPG("attachment1.heic").Return("attachment1.jpeg", nil),
ofMock.EXPECT().WriteAttachment("attachment1.jpeg"),
osMock.EXPECT().FileExist("attachment2.jpeg").Return(true, nil),
osMock.EXPECT().HEIC2JPG("attachment2.jpeg").Return("attachment2.jpeg", nil),
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock.EXPECT().Flush().Return(errors.New("this is a flush error")),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
)
},
wantErr: `flush chat file "messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf" to disk: this is a flush error`,
},
{
msg: "attachment file does not exist",
setupMocks: func(dbMock *mock_chatdb.MockChatDB, osMock *mock_opsys.MockOS, ofMock *mock_opsys.MockOutFile) {
Expand All @@ -315,8 +348,9 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.txt"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush(),
ofMock.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock.EXPECT().Flush(),
)
},
wantJPGs: 1,
Expand Down Expand Up @@ -421,8 +455,9 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.pdf"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush(),
ofMock.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock.EXPECT().Flush(),
)
},
wantJPGs: 1,
Expand Down Expand Up @@ -465,8 +500,9 @@ func TestWriteFile(t *testing.T) {
ofMock.EXPECT().WriteAttachment("attachment2.jpeg"),
ofMock.EXPECT().Name().Return("messages-export/friend/iMessage;-;friend@gmail.com;;;iMessage;-;friend@hotmail.com.txt"),
ofMock.EXPECT().ReferenceAttachment("att3transfer.png"),
ofMock.EXPECT().Flush(),
ofMock.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock.EXPECT().Flush(),
)
},
wantInvalid: 1,
Expand Down Expand Up @@ -538,8 +574,9 @@ func TestWriteFile(t *testing.T) {
osMock.EXPECT().MkdirAll("friend", os.ModePerm),
osMock.EXPECT().Create("friend/iMessage;-;heresareallylongemailaddress.heresareallylongemailaddress.heresareallylongemailaddress.heresareallylongemailaddress.heresareallylongemailaddress.heresareallylongemailaddress.heresareallylongemailaddress.heresareallylongemailaddress@gmail.c.txt").Return(chatFile, nil),
osMock.EXPECT().NewTxtOutFile(chatFile).Return(ofMock),
ofMock.EXPECT().Flush(),
ofMock.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock.EXPECT().Flush(),
)

cfg := configuration{OS: osMock}
Expand Down Expand Up @@ -579,8 +616,9 @@ func TestWriteFile(t *testing.T) {
}
mockCalls = append(
mockCalls,
ofMock1.EXPECT().Flush(),
ofMock1.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock1.EXPECT().Flush(),
osMock.EXPECT().Create("messages-export/friend/iMessage;-;friend@gmail.com.2.pdf").Return(chatFile2, nil),
osMock.EXPECT().NewPDFOutFile(chatFile2, gomock.Any(), false).Return(ofMock2),
)
Expand All @@ -595,8 +633,9 @@ func TestWriteFile(t *testing.T) {
}
mockCalls = append(
mockCalls,
ofMock2.EXPECT().Flush(),
ofMock2.EXPECT().Stage(),
osMock.EXPECT().GetOpenFilesLimit().Return(256),
ofMock2.EXPECT().Flush(),
)
gomock.InOrder(mockCalls...)

Expand Down
22 changes: 18 additions & 4 deletions opsys/mock_opsys/mock_outfile.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 2ec2203

Please sign in to comment.