Skip to content

gmail: Use Label Get() in order to get better progress estimate. #6

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.

Sign up for GitHub

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

Open
canet-benoit wants to merge 1 commit into
base: master
Choose a base branch
from
Open

gmail: Use Label Get() in order to get better progress estimate. #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
105 changes: 97 additions & 8 deletions lib/gmail/gmail.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,87 @@ func shardForMsgId(id string) int {
return int(shard)
}

// This function return true if the given label parameter

@danmarg danmarg Mar 19, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
// This function return true if the given label parameter
// This function returns true if the given label parameter

// is not an extra (decorating) label.
func isToBeAccounted(label string) bool {

@danmarg danmarg Mar 19, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think it might be clearer to just declare a "const" (not really, since maps cannot be const) map at the top of this file and directly check for membership, a la:

var countableLabels = map[string]bool{
  "DRAFT": true,
  "INBOX": true,
  "SENT": true,
  "TRASH": true,
}

And then for usage, replace the function call with countableLabels[label]. I think the function indirection doesn't buy us much.

// The following build a set of the real mails labels.
// Only these mails types are to take into account when
// retrieving a full mailbox without filtering by a given
// label.
type void struct{}

@danmarg danmarg Mar 19, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

As I noted above, I think I'd just declare a map and access it directly. In addition, given the small size of this map, the optimization of using empty struct members seems to me not worth the slight loss in readability.

In any event, this function could be rewritten as:

Suggested change
type void struct{}
func isCounted(label string) bool {
_, r := map[string]struct{}{
"INBOX": struct{}{},
"DRAFT": struct{}{},
"SENT": struct{}{},
"TRASH": struct{}{}}[label]
return r
}

var member void
toAccountLabels := make(map[string]void)
toAccountLabels["DRAFT"] = member
toAccountLabels["INBOX"] = member
toAccountLabels["SENT"] = member
toAccountLabels["TRASH"] = member
_, toBeAccounted := toAccountLabels[label]
return toBeAccounted
}

// This function takes a list of labels and count
// the number of real labels discarding extra labels.

@danmarg danmarg Mar 19, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
// the number of real labels discarding extra labels.
// the number of countable labels discarding extra labels.

func toBeAccountedLabelsCount(g *Gmail, labels []string) int64 {
// If a label was specified on the command line
// then we are interested only on this given label
// and we return 1.
if len(g.label) != 0 {
return 1
}
// No label was specified so return the relevant labels
// count.
count := int64(0)
for _, label := range labels {
if isToBeAccounted(label) {
count += 1
}
}
return count
}

func getLabelsCount(g *Gmail) (int64, error) {

@danmarg danmarg Mar 19, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm afraid I don't understand the algorithm here. It seems like we are doing two things

  1. Getting the message count per labels for which we will download messages (which is either all "toBeAccounted" labels, or the label specified on the command line?).
  2. Getting the total count for all toBeAccounted labels.

And then displaying progress as "(1) divided by (2)".

I don't understand how this is correct? Isn't (1) the total number of matching messages?

Given the code that is updating the progress meter knows its current download counts, I thought the only error in displaying progress was that the denominator (i.e. returned from (2)) was wrong (prior to this PR).

// Get a list of partially filled labels.
// In order to get the message count of the resulting
// labels another call to UsersLabelsService.Get() must be
// done later per label.
labelsResponse, err := g.svc.GetLabels()
if err != nil {
return 0, err
}

totalCount := int64(0)
for _, label := range labelsResponse.Labels {
// Get the complete data for a specific label
fullLabel, err := g.svc.GetLabel(label.Id)
if err != nil {
return 0, err
}

toAccount := int64(fullLabel.MessagesTotal)
// 1) If the current label is TRASH and no label was
// specified then it's message count must be
// substracted from the totalCount if we are
// in 2) below.
if label.Name == "TRASH" {
toAccount *= -1
}

// No label was specified when calling the NewGmail()
// constructor: add to the total account of labels
// matching this loop's label excepted if the are in 1)
// (like above) then we must withdraw the account.
if len(g.labelId) == 0 && isToBeAccounted(label.Name) {
totalCount += toAccount
} else if len(g.labelId) != 0 && fullLabel.Id == g.labelId {
// If we are looking for a specific label then
// simply return the message count of this label.
return fullLabel.MessagesTotal, nil
}
}

return totalCount, nil
}

func (g *Gmail) incremental(historyId uint64) error {
log.Println("Performing incremental sync.")
page := ""
Expand Down Expand Up @@ -406,7 +487,11 @@ func (g *Gmail) incremental(historyId uint64) error {
close(ops)
}()

t := uint(0) // Total count, for progress reporting.
t, err := getLabelsCount(g) // Total count, for progress reporting
if err != nil {
return err
}

go func() {
for true {
r, err := g.svc.GetHistory(historyId, g.labelId, page)
Expand All @@ -419,7 +504,6 @@ func (g *Gmail) incremental(historyId uint64) error {
return
}
page = r.NextPageToken
t += uint(len(r.History))
for _, m := range r.History {
if m.Id > historyId {
historyId = m.Id
Expand Down Expand Up @@ -474,13 +558,13 @@ func (g *Gmail) incremental(historyId uint64) error {
close(h)
}
}()
i := uint(0)
i := int64(0)
for o := range ops {
// Update progress bar.
if g.progress != nil {
g.progress <- lib.Progress{Current: i, Total: t}
}
i++
i += toBeAccountedLabelsCount(g, o.Labels)
if o.Error != nil {
return o.Error
}
Expand Down Expand Up @@ -533,7 +617,12 @@ func (g *Gmail) full() error {
close(ops)
}()
seen := make(map[string]struct{}) // Used to compute deletes.
t := uint(0) // Total count, for progress reporting.

t, err := getLabelsCount(g) // Total count, for progress reporting
if err != nil {
return err
}

go func() {
defer close(newMsgs)
page := ""
Expand All @@ -544,7 +633,7 @@ func (g *Gmail) full() error {
return
}
page = r.NextPageToken
t += uint(r.ResultSizeEstimate)

for _, m := range r.Messages {
newMsgs <- m.Id
seen[m.Id] = struct{}{}
Expand All @@ -555,13 +644,13 @@ func (g *Gmail) full() error {
}
}()
historyId := uint64(0)
i := uint(0) // For updating progress bar.
i := int64(0) // For updating progress bar.
for o := range ops {
// Update progress bar.
if g.progress != nil {
g.progress <- lib.Progress{Current: i, Total: t}
}
i++
i += toBeAccountedLabelsCount(g, o.Labels)
if o.Error != nil {
return o.Error
}
Expand Down
11 changes: 11 additions & 0 deletions lib/gmail/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type gmailService interface {
GetRawMessage(id string) (string, error)
GetMetadata(id string) (*gmail.Message, error)
GetLabels() (*gmail.ListLabelsResponse, error)
GetLabel(id string) (*gmail.Label, error)
GetHistory(historyIndex uint64, label, page string) (*gmail.ListHistoryResponse, error)
GetMessages(q, page string) (*gmail.ListMessagesResponse, error)
}
Expand Down Expand Up @@ -79,6 +80,16 @@ func (s *restGmailService) GetLabels() (*gmail.ListLabelsResponse, error) {
return r, err
}

func (s *restGmailService) GetLabel(id string) (*gmail.Label, error) {
var r *gmail.Label
var err error
err = s.limiter.DoWithBackoff(func() (error, bool) {
r, err = s.svc.Labels.Get("me", id).Do()
return isRateLimited(err)
})
return r, err
}

func (s *restGmailService) GetHistory(historyIndex uint64, labelId, page string) (*gmail.ListHistoryResponse, error) {
hist := s.svc.History.List("me").StartHistoryId(historyIndex)
if labelId != "" {
Expand Down
4 changes: 2 additions & 2 deletions lib/progress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ package lib

// Progress represents a simple "done xxx out of yyy"-style progress report.
type Progress struct {
Current uint
Total uint
Current int64
Total int64
}
5 changes: 4 additions & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,14 @@ func main() {
}
progress := make(chan lib.Progress)
go func() {
// Given how the label mail counting work we are only able to render the progress
Comment thread
danmarg marked this conversation as resolved.
// relative to the total number of mail label processed.
// So we specify below that the progress is done against labels and not mails.
l := time.Time{}
for p := range progress {
if time.Since(l).Seconds() > progressUpdateFreqSecs {
l = time.Now()
fmt.Printf("\r%d / %d %.2f%% ", p.Current, p.Total, float32(p.Current)/float32(p.Total)*100)
fmt.Printf("\r%d / %d labels : %.2f%% ", p.Current, p.Total, float32(p.Current)/float32(p.Total)*100)
}
}
fmt.Println()
Expand Down