[PATCH 06/10] parser: Use full regexps for delegation rules paths

[Laurent Pinchart]

Hi Stephen,

On Thursday 24 December 2015 14:06:58 Finucane, Stephen wrote:
> On 28 Nov 10:14, Mauro Carvalho Chehab wrote:
> > From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > 
> > Paths are validated by trying to compile it as a regexp using a custom
> > validator.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab at osg.samsung.com>
> 
> Some small nits that I can fix myself. Other than that,
> 
> Acked-by: Stephen Finucane <stephen.finucane at intel.com>
> 
> > ---
> > 
> >  patchwork/bin/parsemail.py | 19 +++++++++++++++++--
> >  patchwork/models.py        |  9 ++++++++-
> >  2 files changed, 25 insertions(+), 3 deletions(-)
> > 
> > diff --git a/patchwork/bin/parsemail.py b/patchwork/bin/parsemail.py
> > index 4f22c7f2d6a0..5d6ddf45d264 100755
> > --- a/patchwork/bin/parsemail.py
> > +++ b/patchwork/bin/parsemail.py
> > @@ -339,18 +339,33 @@ def get_state(state_name):
> >              pass
> >      return get_default_initial_patch_state()
> > 
> > +class Rule(object):
> > +    pass
> > +
> 
> nit: a defaultdict ({user: [path, path...]}) would be more semantic
> here. I can change this, unless you've any reason I shouldn't.

No reason, please go ahead. I didn't know about defaultdict, thanks for the 
tip.

> >  def auto_delegate(project, filenames):
> >      if not filenames:
> >          return None
> > 
> > -    rules = list(DelegationRule.objects.filter(project = project))
> > +    # Precompile the path regexps
> > +    rules = []
> > +    for rule in DelegationRule.objects.filter(project = project):
> > +        r = Rule()
> > +        r.user = rule.user
> > +
> > +        try:
> > +            r.path = re.compile(rule.path)
> > +        except:
>
> I'm going to make this capture 're.error' only, if that's OK.

No issue with that.

> > +            print '%s is not a valid regular expression' % rule.path
> > +            continue
> > +
> > +        rules.append(r)
> > 
> >      patch_delegate = None
> >      
> >      for filename in filenames:
> >          file_delegate = None
> >          for rule in rules:
> > -            if fnmatch(filename, rule.path):
> > +            if rule.path.match(filename):
> >                  file_delegate = rule.user
> >                  break;

-- 
Regards,

Laurent Pinchart