Ibid

Merge lp:~ibid-dev/ibid/factoid-2-544493 into lp:~ibid-core/ibid/old-trunk-1.6

  1. factoid-2-544493
  2. Merge into old-trunk-1.6
Proposed by Stefano Rivera
Status: Merged
Approved by: Stefano Rivera
Approved revision: 1007
Merged at revision: 1007
Proposed branch: lp:~ibid-dev/ibid/factoid-2-544493
Merge into: lp:~ibid-core/ibid/old-trunk-1.6
Prerequisite: lp:~stefanor/ibid/regexp-595423
Diff against target: 234 lines (+82/-50)
1 file modified
ibid/plugins/factoid.py (+82/-50)
To merge this branch: bzr merge lp:~ibid-dev/ibid/factoid-2-544493
Reviewer Review Type Date Requested Status
Jonathan Hitchcock Approve
marcog (community) Approve
Max Rabkin Approve
Stefano Rivera Approve
Review via email: mp+47307@code.launchpad.net

Commit message

Use escape=# for LIKEs
Perform literal queries on all non-get Factoid operations. Return useful error if start index is too high
Substitute $arg for _% in search

To post a comment you must log in.
Revision history for this message
Stefano Rivera (stefanor) wrote :

There are probably lingering bugs overall, but I approve of Marco's part of this merge.

review: Approve
Revision history for this message
Max Rabkin (max-rabkin) wrote :

148 + # Hack: We replace $arg with _%, but this won't match a partial
149 + # "$args" string

the string is "$arg"

Note that this still needs another reviewer since Stefano is a part author of the branch.

review: Approve
Revision history for this message
Jonathan Hitchcock (vhata) wrote :

Merge artifacts!

review: Needs Fixing
Revision history for this message
marcog (marco-gallotta) :
review: Approve
Revision history for this message
Jonathan Hitchcock (vhata) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ibid/plugins/factoid.py'
2--- ibid/plugins/factoid.py 2011-01-25 07:20:39 +0000
3+++ ibid/plugins/factoid.py 2011-02-20 18:07:36 +0000
4@@ -78,11 +78,8 @@
5 def upgrade_6_to_7(self):
6 self.add_column(Column('wild', Boolean, PassiveDefault('0'),
7 nullable=False, index=True, default=False))
8- # http://www.sqlalchemy.org/trac/ticket/1400:
9- # We can't use .like() in MySQL
10 for row in self.upgrade_session.query(FactoidName) \
11- .filter('lower(name) LIKE lower(:pattern) ESCAPE :escape') \
12- .params(pattern='%\\_\\%%', escape='\\') \
13+ .filter(FactoidName.name.like('%#_#%%', escape='#')) \
14 .all():
15 row.wild = True
16 self.upgrade_session.save_or_update(row)
17@@ -210,18 +207,23 @@
18
19 action_re = re.compile(r'^\s*<action>\s*')
20 reply_re = re.compile(r'^\s*<reply>\s*')
21-escape_like_re = re.compile(r'([%_\\])')
22+escape_like_re = re.compile(r'([%_#])')
23
24 def get_factoid(session, name, number, pattern, is_regex, all=False,
25 literal=False):
26 """session: SQLAlchemy session
27 name: Factoid name (can contain arguments unless literal query)
28- number: Return factoid[number] (or factoid[number:] for literal queries)
29- pattern: Return factoids matching this pattern (cannot be used in conjuction with number)
30+ number: If not None, restrict to factoid[number] (or factoid[number:] for literal queries)
31+ pattern: If not None, restrict to factoids matching this pattern (cannot be used in conjuction with number)
32 is_regex: Pattern is a real regex
33 all: Return a random factoid from the set if False
34 literal: Match factoid name literally (implies all)
35+ if all or literal, a list is returned; otherwise a single factoid is returned
36+ if nothing is found, either an empty list or None is returned
37 """
38+ assert not (number and pattern), u'number and pattern cannot be used together'
39+ if literal:
40+ all = True # as mentioned in the docstring
41 # First pass for exact matches, if necessary again for wildcard matches
42 if literal:
43 passes = (False,)
44@@ -249,22 +251,22 @@
45 op = get_regexp_op(session)
46 query = query.filter(op(FactoidValue.value, pattern))
47 else:
48- pattern = '%%%s%%' % escape_like_re.sub(r'\\\1', pattern)
49- # http://www.sqlalchemy.org/trac/ticket/1400:
50- # We can't use .like() in MySQL
51- query = query.filter(
52- 'lower(value) LIKE lower(:pattern) ESCAPE :escape'
53- ).params(pattern=pattern, escape='\\')
54+ pattern = '%%%s%%' % escape_like_re.sub(r'#\1', pattern)
55+ query = query.filter(func.lower(FactoidValue.value)
56+ .like(pattern, escape='#'))
57
58- if number:
59+ if number is not None:
60+ number = max(0, int(number) - 1)
61 try:
62+ # The .all() is to ensure the result is a list.
63+ # It gets len()ed later
64 if literal:
65- return query.order_by(FactoidValue.id)[int(number) - 1:]
66+ return query.order_by(FactoidValue.id).all()[number:]
67 else:
68- factoid = query.order_by(FactoidValue.id)[int(number) - 1]
69+ factoid = query.order_by(FactoidValue.id).all()[number]
70 except IndexError:
71 continue
72- if all or literal:
73+ if all:
74 if factoid is not None:
75 return [factoid]
76 else:
77@@ -273,8 +275,9 @@
78 factoid = factoid or query.order_by(func.random()).first()
79 if factoid:
80 return factoid
81- if all:
82- return []
83+ if all:
84+ return []
85+ return None
86
87 class Utils(Processor):
88 usage = u'literal <name> [( #<from number> | /<pattern>/[r] )]'
89@@ -289,6 +292,15 @@
90 event.addresponse(u', '.join(u'%i: %s'
91 % (index + number, value.value)
92 for index, (factoid, name, value) in enumerate(factoids)))
93+ else:
94+ factoids = get_factoid(event.session, name, None, pattern, is_regex,
95+ literal=True)
96+ if factoids:
97+ event.addresponse(u"I only know %(number)d things about %(name)s", {
98+ u'number': len(factoids),
99+ u'name': name,
100+ })
101+
102
103 class Forget(Processor):
104 usage = u"""forget <name> [( #<number> | /<pattern>/[r] )]
105@@ -302,17 +314,17 @@
106 @match(r'^forget\s+(.+?)(?:\s+#(\d+)|\s+(?:/(.+?)/(r?)))?$')
107 @authorise(fallthrough=False)
108 def forget(self, event, name, number, pattern, is_regex):
109- factoids = get_factoid(event.session, name, number, pattern, is_regex, all=True)
110+ factoids = get_factoid(event.session, name, number, pattern, is_regex, all=True, literal=True)
111 if factoids:
112 factoidadmin = auth_responses(event, u'factoidadmin')
113 identities = get_identities(event)
114 factoid = event.session.query(Factoid).get(factoids[0][0].id)
115
116- if (number or pattern):
117- if len(factoids) > 1:
118- event.addresponse(u'Pattern matches multiple factoids, please be more specific')
119- return
120+ if len(factoids) > 1 and pattern is not None:
121+ event.addresponse(u'Pattern matches multiple factoids, please be more specific')
122+ return
123
124+ if number or pattern:
125 if factoids[0][2].identity_id not in identities and not factoidadmin:
126 return
127
128@@ -355,7 +367,14 @@
129
130 event.addresponse(True)
131 else:
132- event.addresponse(u"I didn't know about %s anyway", name)
133+ factoids = get_factoid(event.session, name, None, pattern, is_regex, all=True, literal=True)
134+ if factoids:
135+ event.addresponse(u"I only know %(number)d things about %(name)s", {
136+ u'number': len(factoids),
137+ u'name': name,
138+ })
139+ else:
140+ event.addresponse(u"I didn't know about %s anyway", name)
141
142 @match(r'^(.+)\s+is\s+the\s+same\s+as\s+(.+)$')
143 @authorise(fallthrough=False)
144@@ -415,35 +434,41 @@
145 pattern = m.group(1)
146 is_regex = bool(m.group(2))
147
148+ # Hack: We replace $arg with _%, but this won't match a partial
149+ # "$arg" string
150+ if is_regex:
151+ filter_op = get_regexp_op(event.session)
152+ name_pattern = pattern.replace(r'\$arg', '_%')
153+ else:
154+ filter_op = lambda x, y: x.like(y, escape='#')
155+ pattern = '%%%s%%' % escape_like_re.sub(r'#\1', pattern)
156+ name_pattern = pattern.replace('$arg', '#_#%')
157+
158 query = event.session.query(Factoid)\
159- .join(Factoid.names).add_entity(FactoidName)\
160- .join(Factoid.values)
161+ .join(Factoid.names).add_entity(FactoidName)\
162+ .join(Factoid.values)
163
164 if search_type.startswith('fact'):
165- filter_on = (FactoidName.name,)
166+ query = query.filter(filter_op(FactoidName.name, name_pattern))
167 elif search_type.startswith('value'):
168- filter_on = (FactoidValue.value,)
169- else:
170- filter_on = (FactoidName.name, FactoidValue.value)
171-
172- if is_regex:
173- filter_op = get_regexp_op(event.session)
174- else:
175- pattern = "%%%s%%" % escape_like_re.sub(r'\\\1', pattern)
176- filter_op = lambda x, y: x.like(y)
177-
178- if len(filter_on) == 1:
179- query = query.filter(filter_op(filter_on[0], pattern))
180- else:
181- query = query.filter(or_(filter_op(filter_on[0], pattern), filter_op(filter_on[1], pattern)))
182-
183- # Pre-evalute the iterable or the if statement will be True in SQLAlchemy 0.4 [Bug #383286]
184- matches = [match for match in query[start:start+limit]]
185-
186- if matches:
187+ query = query.filter(filter_op(FactoidValue.value, pattern))
188+ else:
189+ query = query.filter(or_(filter_op(FactoidName.name, name_pattern),
190+ filter_op(FactoidValue.value, pattern)))
191+
192+ # Pre-evalute the iterable or the if statement will be True in SQLAlchemy 0.4. LP: #383286
193+ matches = [match for match in query.all()]
194+
195+ bounded_matches = matches[start:start+limit]
196+ if bounded_matches:
197 event.addresponse(u'; '.join(
198 u'%s [%s]' % (fname.name, len(factoid.values))
199 for factoid, fname in matches))
200+ elif len(matches):
201+ event.addresponse(u"I could only find %(number)d things that matched '%(pattern)s'", {
202+ u'number': len(matches),
203+ u'pattern': origpattern,
204+ })
205 else:
206 event.addresponse(u"I couldn't find anything that matched '%s'" % origpattern)
207
208@@ -623,7 +648,7 @@
209 @authorise(fallthrough=False)
210 def append(self, event, name, number, pattern, is_regex, suffix):
211 name = strip_name(name)
212- factoids = get_factoid(event.session, name, number, pattern, is_regex, all=True)
213+ factoids = get_factoid(event.session, name, number, pattern, is_regex, all=True, literal=True)
214 if len(factoids) == 0:
215 if pattern:
216 event.addresponse(u"I don't know about any %(name)s matching %(pattern)s", {
217@@ -631,8 +656,15 @@
218 'pattern': pattern,
219 })
220 else:
221- event.addresponse(u"I don't know about %s", name)
222- elif len(factoids) > 1:
223+ factoids = get_factoid(event.session, name, None, pattern, is_regex, all=True)
224+ if factoids:
225+ event.addresponse(u"I only know %(number)d things about %(name)s", {
226+ u'number': len(factoids),
227+ u'name': name,
228+ })
229+ else:
230+ event.addresponse(u"I don't know about %s", name)
231+ elif len(factoids) > 1 and number is None:
232 event.addresponse(u"Pattern matches multiple factoids, please be more specific")
233 else:
234 factoidadmin = auth_responses(event, u'factoidadmin')

Subscribers

People subscribed via source and target branches

to all changes: