Opened 5 years ago

Closed 4 years ago

#4178 regression closed fixed (fixed)

Function singleSearchStep was renamed to _singleSearchStep not everywhere

Reported by: natta Owned by:
Priority: high Milestone: Twisted-10.0
Component: mail Keywords: mail imap4
Cc: exarkun Branch: branches/imap-search-fixes-4178
(diff, github, buildbot, log)
Author: exarkun, itamar Launchpad Bug:

Description

Function singleSearchStep was renamed to _singleSearchStep and one parameter added, but in search_OR and search_NOT fucntions there is still old version function invocation (i.e. without "_")
So every search query that contains OR or NOT keywords raises errors.

Attachments (1)

imap4.patch (454 bytes) - added by natta 5 years ago.
Patch that replaces singleSearchStep with _singleSearchStep in search_(NOT|OR)

Download all attachments as: .zip

Change History (12)

Changed 5 years ago by natta

Patch that replaces singleSearchStep with _singleSearchStep in search_(NOT|OR)

comment:1 Changed 5 years ago by natta

  • Owner changed from exarkun to natta
  • Status changed from new to assigned

comment:2 Changed 5 years ago by natta

  • Owner changed from natta to exarkun
  • Status changed from assigned to new

I can not commit changes, so i reassign it back

comment:3 Changed 5 years ago by exarkun

  • Type changed from defect to regression

Regression introduced by #1977.

comment:4 follow-up: Changed 5 years ago by exarkun

  • Cc exarkun added
  • Owner changed from exarkun to natta

Thanks for the patch, natta. Can you supply one which also includes unit tests for this fix? twisted.mail.test.test_imap.DefaultSearchTestCase may be a good case to extend.

comment:5 in reply to: ↑ 4 Changed 5 years ago by natta

Replying to exarkun:

Thanks for the patch, natta. Can you supply one which also includes unit tests for this fix? twisted.mail.test.test_imap.DefaultSearchTestCase may be a good case to extend.

OK. As so I am not a very experienced developer, but I'll try.

comment:6 Changed 5 years ago by exarkun

  • Milestone set to Twisted-10.0

comment:7 Changed 4 years ago by exarkun

  • Author changed from natta to exarkun, natta
  • Branch set to branches/imap-search-fixes-4178

(In [28242]) Branching to 'imap-search-fixes-4178'

comment:8 Changed 4 years ago by exarkun

  • Author changed from exarkun, natta to exarkun, itamar
  • Keywords review added
  • Owner natta deleted

Fixed and tested, please review.

comment:9 Changed 4 years ago by therve

  • Keywords imap4 added; imap4.py do_SEARCH review removed
  • Owner set to exarkun

So I would prefer another way to check if the lastSequenceId parameter is required. Something like that:

_requiresLastSequenceId = set(["NOT", "OR"])

if c in self._requiresLastSequenceId:
     result = f(query, id, msg, lastSequenceId)
else:
     result = f(query, id, msg)

The tests need to fit in 80 colums, too.

Please merge after that.

comment:10 Changed 4 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(In [28281]) Merge imap-search-fixes-4178

Author: exarkun, itamar
Reviewer: therve
Fixes: #4178

Fix the regression in search OR and NOT support in the IMAP4 server's fallback
search support.

comment:11 Changed 3 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.