~openerp-community/openobject-doc/ksa-openobject-doc-6.0

« back to all changes in this revision

Viewing changes to i18n/ru/source/contribute/15_guidelines/coding_guidelines_framework.rst

  • Committer: Don Kirkby
  • Date: 2011-02-21 20:46:11 UTC
  • mfrom: (433.1.53 openobject-doc)
  • Revision ID: donkirkby+launpd@gmail.com-20110221204611-1ykt6dmg4k3gh5dh
[MERGE] revisions 477 to 486 from the 5.0 branch.

Show diffs side-by-side

added added

removed removed

Lines of Context:
 
1
 
 
2
.. i18n: .. sectnum::
 
3
.. i18n:     :start: 2
 
4
..
 
5
 
 
6
.. sectnum::
 
7
    :start: 2
 
8
 
 
9
.. i18n: OpenERP Specific Guidelines
 
10
.. i18n: +++++++++++++++++++++++++++
 
11
..
 
12
 
 
13
OpenERP Specific Guidelines
 
14
+++++++++++++++++++++++++++
 
15
 
 
16
.. i18n: Bazaar is your historian
 
17
.. i18n: ------------------------
 
18
.. i18n: Do not comment out code if you want to disable it. The code is versioned in
 
19
.. i18n: Bazaar, and regardless of your opinion about Bazaar, it does not lose
 
20
.. i18n: the file history.
 
21
..
 
22
 
 
23
Bazaar is your historian
 
24
------------------------
 
25
Do not comment out code if you want to disable it. The code is versioned in
 
26
Bazaar, and regardless of your opinion about Bazaar, it does not lose
 
27
the file history.
 
28
 
 
29
.. i18n: Leaving comments just makes the code messier and harder to read. And don't
 
30
.. i18n: worry about making your changes obvious, that's the purpose of ``diff``::
 
31
.. i18n: 
 
32
.. i18n:     # no example for this one, code was really removed ;-)
 
33
..
 
34
 
 
35
Leaving comments just makes the code messier and harder to read. And don't
 
36
worry about making your changes obvious, that's the purpose of ``diff``::
 
37
 
 
38
    # no example for this one, code was really removed ;-)
 
39
 
 
40
.. i18n: Call your fish a fish
 
41
.. i18n: ---------------------
 
42
.. i18n: Give your variables a meaningful name all the time.
 
43
.. i18n: You may know what it is referring to now, but you won't in 2 months, and
 
44
.. i18n: others don't either. One-letter variables are acceptable only in lambda 
 
45
.. i18n: expressions and loop indices, or perhaps in pure maths expressions
 
46
.. i18n: (and even there it doesn't hurt to use a real name)::
 
47
.. i18n: 
 
48
.. i18n:     # unclear and misleading
 
49
.. i18n:     a = {}
 
50
.. i18n:     ffields = {}
 
51
.. i18n: 
 
52
.. i18n:     # better
 
53
.. i18n:     results = {}
 
54
.. i18n:     selected_fields = {}
 
55
..
 
56
 
 
57
Call your fish a fish
 
58
---------------------
 
59
Give your variables a meaningful name all the time.
 
60
You may know what it is referring to now, but you won't in 2 months, and
 
61
others don't either. One-letter variables are acceptable only in lambda 
 
62
expressions and loop indices, or perhaps in pure maths expressions
 
63
(and even there it doesn't hurt to use a real name)::
 
64
 
 
65
    # unclear and misleading
 
66
    a = {}
 
67
    ffields = {}
 
68
 
 
69
    # better
 
70
    results = {}
 
71
    selected_fields = {}
 
72
 
 
73
.. i18n: Do not bypass the ORM
 
74
.. i18n: ---------------------
 
75
.. i18n: You should never use the database cursor directly when the ORM can do the
 
76
.. i18n: same thing! By doing so you are bypassing all the ORM features,
 
77
.. i18n: possibly the transactions, access rights and so on.
 
78
..
 
79
 
 
80
Do not bypass the ORM
 
81
---------------------
 
82
You should never use the database cursor directly when the ORM can do the
 
83
same thing! By doing so you are bypassing all the ORM features,
 
84
possibly the transactions, access rights and so on.
 
85
 
 
86
.. i18n: And chances are that you are also making the code harder to read and
 
87
.. i18n: probably less secure (see also next guideline)::
 
88
.. i18n: 
 
89
.. i18n:     # very very wrong
 
90
.. i18n:     cr.execute('select id from auction_lots where auction_id in (' +
 
91
.. i18n:                ','.join(map(str,ids))+') and state=%s and obj_price>0',
 
92
.. i18n:                ('draft',))
 
93
.. i18n:     auction_lots_ids = [x[0] for x in cr.fetchall()]
 
94
.. i18n: 
 
95
.. i18n:     # no injection, but still wrong
 
96
.. i18n:     cr.execute('select id from auction_lots where auction_id in %s '\
 
97
.. i18n:                'and state=%s and obj_price>0',
 
98
.. i18n:                (tuple(ids),'draft',))
 
99
.. i18n:     auction_lots_ids = [x[0] for x in cr.fetchall()]
 
100
.. i18n: 
 
101
.. i18n:     # better
 
102
.. i18n:     auction_lots_ids = self.search(cr,uid,
 
103
.. i18n:                                    [('auction_id','in',ids),
 
104
.. i18n:                                     ('state','=','draft'),
 
105
.. i18n:                                     ('obj_price','>',0)])
 
106
..
 
107
 
 
108
And chances are that you are also making the code harder to read and
 
109
probably less secure (see also next guideline)::
 
110
 
 
111
    # very very wrong
 
112
    cr.execute('select id from auction_lots where auction_id in (' +
 
113
               ','.join(map(str,ids))+') and state=%s and obj_price>0',
 
114
               ('draft',))
 
115
    auction_lots_ids = [x[0] for x in cr.fetchall()]
 
116
 
 
117
    # no injection, but still wrong
 
118
    cr.execute('select id from auction_lots where auction_id in %s '\
 
119
               'and state=%s and obj_price>0',
 
120
               (tuple(ids),'draft',))
 
121
    auction_lots_ids = [x[0] for x in cr.fetchall()]
 
122
 
 
123
    # better
 
124
    auction_lots_ids = self.search(cr,uid,
 
125
                                   [('auction_id','in',ids),
 
126
                                    ('state','=','draft'),
 
127
                                    ('obj_price','>',0)])
 
128
 
 
129
.. i18n: No SQL injections, please!
 
130
.. i18n: --------------------------
 
131
.. i18n: Care must be taken not to introduce SQL injections vulnerabilities when using
 
132
.. i18n: manual SQL queries.  The vulnerability is present when user input is either
 
133
.. i18n: incorrectly filtered or badly quoted which allow an attacker to introduce
 
134
.. i18n: undesirable clauses to a SQL query (such as circumventing filters or executing
 
135
.. i18n: **UPDATE** or **DELETE** commands).
 
136
..
 
137
 
 
138
No SQL injections, please!
 
139
--------------------------
 
140
Care must be taken not to introduce SQL injections vulnerabilities when using
 
141
manual SQL queries.  The vulnerability is present when user input is either
 
142
incorrectly filtered or badly quoted which allow an attacker to introduce
 
143
undesirable clauses to a SQL query (such as circumventing filters or executing
 
144
**UPDATE** or **DELETE** commands).
 
145
 
 
146
.. i18n: The best way to be safe is to never, NEVER use Python string concatenation (+)
 
147
.. i18n: or string parameters interpolation (%) to pass variables to a SQL query string.
 
148
..
 
149
 
 
150
The best way to be safe is to never, NEVER use Python string concatenation (+)
 
151
or string parameters interpolation (%) to pass variables to a SQL query string.
 
152
 
 
153
.. i18n: The second reason, which is almost as important, is that it is the job of the
 
154
.. i18n: database abstraction layer (psycopg2) to decide how to format query parameters,
 
155
.. i18n: not your job!
 
156
.. i18n: For example psycopg2 knows that when you pass a list of values it needs to 
 
157
.. i18n: format them as a comma-separated list, enclosed in parentheses!
 
158
..
 
159
 
 
160
The second reason, which is almost as important, is that it is the job of the
 
161
database abstraction layer (psycopg2) to decide how to format query parameters,
 
162
not your job!
 
163
For example psycopg2 knows that when you pass a list of values it needs to 
 
164
format them as a comma-separated list, enclosed in parentheses!
 
165
 
 
166
.. i18n: .. code-block:: python
 
167
.. i18n: 
 
168
.. i18n:     # the following is very bad:
 
169
.. i18n:     #   - it's a SQL injection vulnerability
 
170
.. i18n:     #   - it's unreadable
 
171
.. i18n:     #   - it's not your job to format the list of ids
 
172
.. i18n:     cr.execute('select distinct child_id from account_account_consol_rel ' +
 
173
.. i18n:                'where parent_id in ('+','.join(map(str, ids))+')')
 
174
.. i18n: 
 
175
.. i18n:     # better
 
176
.. i18n:     cr.execute('SELECT DISTINCT child_id '\
 
177
.. i18n:                'FROM account_account_consol_rel '\
 
178
.. i18n:                'WHERE parent_id IN %s',
 
179
.. i18n:                (tuple(ids),))
 
180
..
 
181
 
 
182
.. code-block:: python
 
183
 
 
184
    # the following is very bad:
 
185
    #   - it's a SQL injection vulnerability
 
186
    #   - it's unreadable
 
187
    #   - it's not your job to format the list of ids
 
188
    cr.execute('select distinct child_id from account_account_consol_rel ' +
 
189
               'where parent_id in ('+','.join(map(str, ids))+')')
 
190
 
 
191
    # better
 
192
    cr.execute('SELECT DISTINCT child_id '\
 
193
               'FROM account_account_consol_rel '\
 
194
               'WHERE parent_id IN %s',
 
195
               (tuple(ids),))
 
196
 
 
197
.. i18n: This is very important, so please be careful also when refactoring, and most
 
198
.. i18n: importantly do not copy these patterns!
 
199
..
 
200
 
 
201
This is very important, so please be careful also when refactoring, and most
 
202
importantly do not copy these patterns!
 
203
 
 
204
.. i18n: Here is a `memorable example <http://www.bobby-tables.com>`_ to help
 
205
.. i18n: you remember what the issue is about (but do not copy the code there).
 
206
..
 
207
 
 
208
Here is a `memorable example <http://www.bobby-tables.com>`_ to help
 
209
you remember what the issue is about (but do not copy the code there).
 
210
 
 
211
.. i18n: Before continuing, please be sure to read the online documentation of pyscopg2
 
212
.. i18n: to learn of to use it properly:
 
213
..
 
214
 
 
215
Before continuing, please be sure to read the online documentation of pyscopg2
 
216
to learn of to use it properly:
 
217
 
 
218
.. i18n:  * `The problem with query parameters <http://initd.org/psycopg/docs/usage.html#the-problem-with-the-query-parameters>`_
 
219
.. i18n:  * `How to pass parameters with psycopg2 <http://initd.org/psycopg/docs/usage.html#passing-parameters-to-sql-queries>`_
 
220
.. i18n:  * `Advanced parameter types <http://initd.org/psycopg/docs/usage.html#adaptation-of-python-values-to-sql-types>`_
 
221
..
 
222
 
 
223
 * `The problem with query parameters <http://initd.org/psycopg/docs/usage.html#the-problem-with-the-query-parameters>`_
 
224
 * `How to pass parameters with psycopg2 <http://initd.org/psycopg/docs/usage.html#passing-parameters-to-sql-queries>`_
 
225
 * `Advanced parameter types <http://initd.org/psycopg/docs/usage.html#adaptation-of-python-values-to-sql-types>`_
 
226
 
 
227
.. i18n: Factor out the code
 
228
.. i18n: -------------------
 
229
.. i18n: If you are writing the same code over and over and it is more than one line,
 
230
.. i18n: then you must factor it out into a reusable function or method::
 
231
.. i18n: 
 
232
.. i18n:     # after writing this multiple times:
 
233
.. i18n:     terp = get_module_resource(module, '__openerp__.py')
 
234
.. i18n:     if not os.path.isfile(terp):
 
235
.. i18n:        terp = addons.get_module_resource(module, '__terp__.py')
 
236
.. i18n:        info = eval(tools.file_open(terp).read())
 
237
.. i18n: 
 
238
.. i18n:     # make a function out of it
 
239
.. i18n:     def _load_information_from_description_file(module):
 
240
.. i18n:         for filename in ['__openerp__.py', '__terp__.py']:
 
241
.. i18n:             description_file = addons.get_module_resource(module, filename)
 
242
.. i18n:             if os.path.isfile(description_file):
 
243
.. i18n:                 return eval(tools.file_open(description_file).read())
 
244
.. i18n:         raise Exception('The module %s does not contain a description file!')
 
245
..
 
246
 
 
247
Factor out the code
 
248
-------------------
 
249
If you are writing the same code over and over and it is more than one line,
 
250
then you must factor it out into a reusable function or method::
 
251
 
 
252
    # after writing this multiple times:
 
253
    terp = get_module_resource(module, '__openerp__.py')
 
254
    if not os.path.isfile(terp):
 
255
       terp = addons.get_module_resource(module, '__terp__.py')
 
256
       info = eval(tools.file_open(terp).read())
 
257
 
 
258
    # make a function out of it
 
259
    def _load_information_from_description_file(module):
 
260
        for filename in ['__openerp__.py', '__terp__.py']:
 
261
            description_file = addons.get_module_resource(module, filename)
 
262
            if os.path.isfile(description_file):
 
263
                return eval(tools.file_open(description_file).read())
 
264
        raise Exception('The module %s does not contain a description file!')
 
265
 
 
266
.. i18n: The infamous context
 
267
.. i18n: --------------------
 
268
.. i18n: Do not use mutable objects as default values for functions, because they are
 
269
.. i18n: created as constants (evaluated only once), so you will have possible
 
270
.. i18n: side-effects if you modify them.
 
271
.. i18n: The usual example of this is the ``context`` argument to all ORM methods::
 
272
.. i18n: 
 
273
.. i18n:     # bad, this could have side-effects
 
274
.. i18n:     def spam(eggs, context={}):
 
275
.. i18n:        setting = context.get('foo')
 
276
.. i18n:        #...
 
277
.. i18n: 
 
278
.. i18n:     # this is better if your need to use the context
 
279
.. i18n:     def spam(eggs, context=None):
 
280
.. i18n:        if context is None:
 
281
.. i18n:           context = {}
 
282
.. i18n:        setting = context.get('foo')
 
283
.. i18n:        #...
 
284
..
 
285
 
 
286
The infamous context
 
287
--------------------
 
288
Do not use mutable objects as default values for functions, because they are
 
289
created as constants (evaluated only once), so you will have possible
 
290
side-effects if you modify them.
 
291
The usual example of this is the ``context`` argument to all ORM methods::
 
292
 
 
293
    # bad, this could have side-effects
 
294
    def spam(eggs, context={}):
 
295
       setting = context.get('foo')
 
296
       #...
 
297
 
 
298
    # this is better if your need to use the context
 
299
    def spam(eggs, context=None):
 
300
       if context is None:
 
301
          context = {}
 
302
       setting = context.get('foo')
 
303
       #...
 
304
 
 
305
.. i18n: Also be careful with boolean tests on lists and maps, because an empty
 
306
.. i18n: dict, list or tuple will evaluate as ``False``::
 
307
.. i18n: 
 
308
.. i18n:     # bad, you shadow the original context if it's empty
 
309
.. i18n:     def spam(eggs, context=None):
 
310
.. i18n:        if not context:
 
311
.. i18n:           context = {}
 
312
.. i18n:        setting = context.get('foo')
 
313
.. i18n:        #...
 
314
..
 
315
 
 
316
Also be careful with boolean tests on lists and maps, because an empty
 
317
dict, list or tuple will evaluate as ``False``::
 
318
 
 
319
    # bad, you shadow the original context if it's empty
 
320
    def spam(eggs, context=None):
 
321
       if not context:
 
322
          context = {}
 
323
       setting = context.get('foo')
 
324
       #...
 
325
 
 
326
.. i18n: And it's okay if you only need to forward it, you can pass ``None`` and
 
327
.. i18n: let the downstream code handle it::
 
328
.. i18n: 
 
329
.. i18n:     # fine
 
330
.. i18n:     def spam(eggs, context=None):
 
331
.. i18n:         setting = get_setting(True, context=context)
 
332
..
 
333
 
 
334
And it's okay if you only need to forward it, you can pass ``None`` and
 
335
let the downstream code handle it::
 
336
 
 
337
    # fine
 
338
    def spam(eggs, context=None):
 
339
        setting = get_setting(True, context=context)
 
340
 
 
341
.. i18n: See also `launchpad bug 525808 <https://bugs.launchpad.net/openobject-server/+bug/525808>`_.
 
342
..
 
343
 
 
344
See also `launchpad bug 525808 <https://bugs.launchpad.net/openobject-server/+bug/525808>`_.
 
345
 
 
346
.. i18n: There is better than lambda, sometimes
 
347
.. i18n: --------------------------------------
 
348
.. i18n: Instead of writing trivial lambda expression to extract items or attributes
 
349
.. i18n: from a list of data structures, learn to use list comprehension
 
350
.. i18n: or ``operator.itemgetter`` and ``operator.attrgetter`` instead, which are
 
351
.. i18n: often more readable and faster::
 
352
.. i18n: 
 
353
.. i18n:     # not very readable
 
354
.. i18n:     partner_tuples = map(lambda x: (x['id'], x['name']), partners)
 
355
.. i18n: 
 
356
.. i18n:     # better with list comprehension for just one item/attribute
 
357
.. i18n:     partner_ids = [partner['id'] for partner in partners]
 
358
.. i18n: 
 
359
.. i18n:     # better with operator for many items/attributes
 
360
.. i18n:     from operator import itemgetter
 
361
.. i18n:     # ...
 
362
.. i18n:     partner_tuples = map(itemgetter('id', 'name'), partners)
 
363
..
 
364
 
 
365
There is better than lambda, sometimes
 
366
--------------------------------------
 
367
Instead of writing trivial lambda expression to extract items or attributes
 
368
from a list of data structures, learn to use list comprehension
 
369
or ``operator.itemgetter`` and ``operator.attrgetter`` instead, which are
 
370
often more readable and faster::
 
371
 
 
372
    # not very readable
 
373
    partner_tuples = map(lambda x: (x['id'], x['name']), partners)
 
374
 
 
375
    # better with list comprehension for just one item/attribute
 
376
    partner_ids = [partner['id'] for partner in partners]
 
377
 
 
378
    # better with operator for many items/attributes
 
379
    from operator import itemgetter
 
380
    # ...
 
381
    partner_tuples = map(itemgetter('id', 'name'), partners)
 
382
 
 
383
.. i18n: See also http://docs.python.org/library/operator.html#operator.attrgetter
 
384
..
 
385
 
 
386
See also http://docs.python.org/library/operator.html#operator.attrgetter
 
387
 
 
388
.. i18n: As of version 6.0 you can also use literal values as defaults for
 
389
.. i18n: your ORM columns, which means that you can stop writing these::
 
390
.. i18n: 
 
391
.. i18n:     # lots of trivial one-liners in 5.0
 
392
.. i18n:     _defaults = {
 
393
.. i18n:         'active': lambda *x: True,
 
394
.. i18n:         'state': lambda *x: 'draft',
 
395
.. i18n:     }
 
396
.. i18n: 
 
397
.. i18n:     # much simpler as of 6.0
 
398
.. i18n:     _defaults = {
 
399
.. i18n:         'active': True,
 
400
.. i18n:         'state': 'draft',
 
401
.. i18n:     }
 
402
..
 
403
 
 
404
As of version 6.0 you can also use literal values as defaults for
 
405
your ORM columns, which means that you can stop writing these::
 
406
 
 
407
    # lots of trivial one-liners in 5.0
 
408
    _defaults = {
 
409
        'active': lambda *x: True,
 
410
        'state': lambda *x: 'draft',
 
411
    }
 
412
 
 
413
    # much simpler as of 6.0
 
414
    _defaults = {
 
415
        'active': True,
 
416
        'state': 'draft',
 
417
    }
 
418
 
 
419
.. i18n: .. warning::
 
420
.. i18n: 
 
421
.. i18n:     Be careful with this, because non-callable defaults are only evaluated
 
422
.. i18n:     once! If you want to generate new default values for each
 
423
.. i18n:     record you really need to keep the ``lambda`` or make it a callable.
 
424
..
 
425
 
 
426
.. warning::
 
427
 
 
428
    Be careful with this, because non-callable defaults are only evaluated
 
429
    once! If you want to generate new default values for each
 
430
    record you really need to keep the ``lambda`` or make it a callable.
 
431
 
 
432
.. i18n: The most frequent error is with timestamps, as in the following example::
 
433
.. i18n: 
 
434
.. i18n:     # This will always give the server start time!
 
435
.. i18n:     _defaults = {
 
436
.. i18n:         'timestamp': time.strftime('%Y-%m-%d %H:%M:%S'),
 
437
.. i18n:     }
 
438
.. i18n: 
 
439
.. i18n:     # You need to keep it callable, e.g:
 
440
.. i18n:     _defaults = {
 
441
.. i18n:         'timestamp': lambda *x: time.strftime('%Y-%m-%d %H:%M:%S'),
 
442
.. i18n:     }
 
443
..
 
444
 
 
445
The most frequent error is with timestamps, as in the following example::
 
446
 
 
447
    # This will always give the server start time!
 
448
    _defaults = {
 
449
        'timestamp': time.strftime('%Y-%m-%d %H:%M:%S'),
 
450
    }
 
451
 
 
452
    # You need to keep it callable, e.g:
 
453
    _defaults = {
 
454
        'timestamp': lambda *x: time.strftime('%Y-%m-%d %H:%M:%S'),
 
455
    }
 
456
 
 
457
.. i18n: Keep your methods short/simple when possible
 
458
.. i18n: --------------------------------------------
 
459
.. i18n: Functions and methods should not contain too much logic: having a lot of small and simple methods is more advisable than having few
 
460
.. i18n: large and complex methods. A good rule of thumb is to split a method as soon as:
 
461
..
 
462
 
 
463
Keep your methods short/simple when possible
 
464
--------------------------------------------
 
465
Functions and methods should not contain too much logic: having a lot of small and simple methods is more advisable than having few
 
466
large and complex methods. A good rule of thumb is to split a method as soon as:
 
467
 
 
468
.. i18n:     * it has more than one responsibility (see http://en.wikipedia.org/wiki/Single_responsibility_principle)
 
469
.. i18n:     * it is too big to fit on one screen.
 
470
..
 
471
 
 
472
    * it has more than one responsibility (see http://en.wikipedia.org/wiki/Single_responsibility_principle)
 
473
    * it is too big to fit on one screen.
 
474
 
 
475
.. i18n: Also, name your functions accordingly: small and properly named functions are the starting point of readable/maintainable code and tighter documentation.
 
476
..
 
477
 
 
478
Also, name your functions accordingly: small and properly named functions are the starting point of readable/maintainable code and tighter documentation.
 
479
 
 
480
.. i18n: This recommendation is also relevant for classes, files, modules and packages. (See also http://en.wikipedia.org/wiki/Cyclomatic_complexity )
 
481
..
 
482
 
 
483
This recommendation is also relevant for classes, files, modules and packages. (See also http://en.wikipedia.org/wiki/Cyclomatic_complexity )
 
484
 
 
485
.. i18n: Never commit the transaction
 
486
.. i18n: ----------------------------
 
487
.. i18n: The OpenERP/OpenObject framework is in charge of providing the transactional context for all RPC calls.
 
488
.. i18n: The principle is that a new database cursor is opened at the begining of each RPC call, and commited
 
489
.. i18n: when the call has returned, just before transmitting the answer to the RPC client, approximately like this::
 
490
.. i18n: 
 
491
.. i18n:     def execute(self, db_name, uid, obj, method, *args, **kw):
 
492
.. i18n:         db, pool = pooler.get_db_and_pool(db_name)
 
493
.. i18n:         # create transaction cursor
 
494
.. i18n:         cr = db.cursor()
 
495
.. i18n:         try:
 
496
.. i18n:             res = pool.execute_cr(cr, uid, obj, method, *args, **kw)
 
497
.. i18n:             cr.commit() # all good, we commit
 
498
.. i18n:         except Exception:
 
499
.. i18n:             cr.rollback() # error, rollback everything atomically
 
500
.. i18n:             raise
 
501
.. i18n:         finally:
 
502
.. i18n:             cr.close() # always close cursor opened manually
 
503
.. i18n:         return res
 
504
..
 
505
 
 
506
Never commit the transaction
 
507
----------------------------
 
508
The OpenERP/OpenObject framework is in charge of providing the transactional context for all RPC calls.
 
509
The principle is that a new database cursor is opened at the begining of each RPC call, and commited
 
510
when the call has returned, just before transmitting the answer to the RPC client, approximately like this::
 
511
 
 
512
    def execute(self, db_name, uid, obj, method, *args, **kw):
 
513
        db, pool = pooler.get_db_and_pool(db_name)
 
514
        # create transaction cursor
 
515
        cr = db.cursor()
 
516
        try:
 
517
            res = pool.execute_cr(cr, uid, obj, method, *args, **kw)
 
518
            cr.commit() # all good, we commit
 
519
        except Exception:
 
520
            cr.rollback() # error, rollback everything atomically
 
521
            raise
 
522
        finally:
 
523
            cr.close() # always close cursor opened manually
 
524
        return res
 
525
 
 
526
.. i18n: If any error occurs during the execution of the RPC call, the transaction is rolled back atomically,
 
527
.. i18n: preserving the state of the system.
 
528
..
 
529
 
 
530
If any error occurs during the execution of the RPC call, the transaction is rolled back atomically,
 
531
preserving the state of the system.
 
532
 
 
533
.. i18n: Similarly, the system also provides a dedicated transaction during the execution of tests suites,
 
534
.. i18n: so it can be rolled back or not depending on the server startup options.
 
535
..
 
536
 
 
537
Similarly, the system also provides a dedicated transaction during the execution of tests suites,
 
538
so it can be rolled back or not depending on the server startup options.
 
539
 
 
540
.. i18n: The consequence is that if you manually call ``cr.commit()`` anywhere there is a very high chance
 
541
.. i18n: that you **will** break the system in various ways, because you will cause partial commits, and thus
 
542
.. i18n: partial and unclean rollbacks, causing among others:
 
543
..
 
544
 
 
545
The consequence is that if you manually call ``cr.commit()`` anywhere there is a very high chance
 
546
that you **will** break the system in various ways, because you will cause partial commits, and thus
 
547
partial and unclean rollbacks, causing among others:
 
548
 
 
549
.. i18n:  - inconsistent business data, usually data loss ;
 
550
.. i18n:  - workflow desynchronization, documents stuck permanently ;
 
551
.. i18n:  - tests that can't be rolled back cleanly, and will start polluting the database,
 
552
.. i18n:    and triggering error (this is true even if no error occurs during the transaction) ;
 
553
..
 
554
 
 
555
 - inconsistent business data, usually data loss ;
 
556
 - workflow desynchronization, documents stuck permanently ;
 
557
 - tests that can't be rolled back cleanly, and will start polluting the database,
 
558
   and triggering error (this is true even if no error occurs during the transaction) ;
 
559
 
 
560
.. i18n: Here is the very simple rule:
 
561
..
 
562
 
 
563
Here is the very simple rule:
 
564
 
 
565
.. i18n: .. warning:: **You should NEVER call cr.commit() yourself, UNLESS you have created your own 
 
566
.. i18n:    database cursor explicitly! And the situations where you need to do that are exceptional!**
 
567
..
 
568
 
 
569
.. warning:: **You should NEVER call cr.commit() yourself, UNLESS you have created your own 
 
570
   database cursor explicitly! And the situations where you need to do that are exceptional!**
 
571
 
 
572
.. i18n: And by the way if you did create your own cursor, then you need to handle error cases and proper
 
573
.. i18n: rollback, as well as properly close the cursor when you're done with it.
 
574
..
 
575
 
 
576
And by the way if you did create your own cursor, then you need to handle error cases and proper
 
577
rollback, as well as properly close the cursor when you're done with it.
 
578
 
 
579
.. i18n: And contrary to popular belief, you do *not* even need to call ``cr.commit()`` in the following
 
580
.. i18n: situations:
 
581
..
 
582
 
 
583
And contrary to popular belief, you do *not* even need to call ``cr.commit()`` in the following
 
584
situations:
 
585
 
 
586
.. i18n:  - in the ``_auto_init()`` method of an ``osv.osv`` object: this is taken care of by the addons
 
587
.. i18n:    initialization method, or by the ORM transaction when creating custom models ;
 
588
.. i18n:  - in reports: the ``commit()`` is handled by the framework too, so you can update the database
 
589
.. i18n:    even from within a report ;
 
590
.. i18n:  - within ``osv.osv_memory`` methods: these methods are called exactly like regular ``osv.osv``
 
591
.. i18n:    ones, within a transaction and with the corresponding ``cr.commit()``/``rollback()`` at the end ;
 
592
.. i18n:  - etc. (see general rule above if you have in doubt!)
 
593
..
 
594
 
 
595
 - in the ``_auto_init()`` method of an ``osv.osv`` object: this is taken care of by the addons
 
596
   initialization method, or by the ORM transaction when creating custom models ;
 
597
 - in reports: the ``commit()`` is handled by the framework too, so you can update the database
 
598
   even from within a report ;
 
599
 - within ``osv.osv_memory`` methods: these methods are called exactly like regular ``osv.osv``
 
600
   ones, within a transaction and with the corresponding ``cr.commit()``/``rollback()`` at the end ;
 
601
 - etc. (see general rule above if you have in doubt!)
 
602
 
 
603
.. i18n: And another very simple rule:
 
604
..
 
605
 
 
606
And another very simple rule:
 
607
 
 
608
.. i18n: .. warning:: **All cr.commit() calls outside of the server framework from now on must have an explicit
 
609
.. i18n:    comment explaining why they are absolutely necessary, why they are indeed correct, and why
 
610
.. i18n:    they do not break the transactions. Otherwise they can and will be removed!**
 
611
..
 
612
 
 
613
.. warning:: **All cr.commit() calls outside of the server framework from now on must have an explicit
 
614
   comment explaining why they are absolutely necessary, why they are indeed correct, and why
 
615
   they do not break the transactions. Otherwise they can and will be removed!**
 
616
 
 
617
.. i18n: Use the gettext method correctly
 
618
.. i18n: --------------------------------
 
619
..
 
620
 
 
621
Use the gettext method correctly
 
622
--------------------------------
 
623
 
 
624
.. i18n: OpenERP uses a GetText-like method named "underscore" ``_( )`` to indicate that a static
 
625
.. i18n: string used in the code needs to be translated at runtime using the language of the context.
 
626
.. i18n: This pseudo-method is accessed within your code by importing as follows::
 
627
.. i18n: 
 
628
.. i18n:     from tools.translate import _
 
629
..
 
630
 
 
631
OpenERP uses a GetText-like method named "underscore" ``_( )`` to indicate that a static
 
632
string used in the code needs to be translated at runtime using the language of the context.
 
633
This pseudo-method is accessed within your code by importing as follows::
 
634
 
 
635
    from tools.translate import _
 
636
 
 
637
.. i18n: A few very important rules must be followed when using it, in order for it to work and to
 
638
.. i18n: avoid filling the translations with useless junk.
 
639
..
 
640
 
 
641
A few very important rules must be followed when using it, in order for it to work and to
 
642
avoid filling the translations with useless junk.
 
643
 
 
644
.. i18n: Basically, this method should only be used for static strings written manually in the code,
 
645
.. i18n: it will not work to translate field *values*, such as Product names, etc. This must be
 
646
.. i18n: done instead using the ``translate`` flag on the corresponding field.
 
647
..
 
648
 
 
649
Basically, this method should only be used for static strings written manually in the code,
 
650
it will not work to translate field *values*, such as Product names, etc. This must be
 
651
done instead using the ``translate`` flag on the corresponding field.
 
652
 
 
653
.. i18n: The rule is very simple: calls to the underscore method should *always* be in the form ``_('literal string')``
 
654
.. i18n: and nothing else::
 
655
.. i18n: 
 
656
.. i18n:     # Good: plain strings
 
657
.. i18n:     error = _('This record is locked!')
 
658
.. i18n: 
 
659
.. i18n:     # Good: strings with formatting patterns included
 
660
.. i18n:     error = _('Record %s cannot be modified!') % record
 
661
.. i18n: 
 
662
.. i18n:     # OK too: multi-line literal strings
 
663
.. i18n:     error = _("""This is a bad multiline example
 
664
.. i18n:                  about record %s!""") % record
 
665
.. i18n:     error = _('Record %s cannot be modified' \
 
666
.. i18n:               'after being validated!') % record
 
667
.. i18n: 
 
668
.. i18n:     # BAD: tries to translate after string formatting 
 
669
.. i18n:     #      (pay attention to brackets!)
 
670
.. i18n:     # This does NOT work and messes up the translations!
 
671
.. i18n:     error = _('Record %s cannot be modified!' % record)
 
672
.. i18n: 
 
673
.. i18n:     # BAD: dynamic string, string concatenation, etc are forbidden!
 
674
.. i18n:     # This does NOT work and messes up the translations!
 
675
.. i18n:     error = _("'" + que_rec['question'] + "' \n")
 
676
.. i18n: 
 
677
.. i18n:     # BAD: field values are automatically translated by the framework
 
678
.. i18n:     # This is useless and will not work the way you think:
 
679
.. i18n:     error = _("Product %s is out of stock!") % _(product.name)
 
680
.. i18n:     # and the following will of course not work as already explained:
 
681
.. i18n:     error = _("Product %s is out of stock!" % product.name)
 
682
.. i18n: 
 
683
.. i18n:     # BAD: field values are automatically translated by the framework
 
684
.. i18n:     # This is useless and will not work the way you think:
 
685
.. i18n:     error = _("Product %s is not available!") % _(product.name)
 
686
.. i18n:     # and the following will of course not work as already explained:
 
687
.. i18n:     error = _("Product %s is not available!" % product.name)
 
688
.. i18n: 
 
689
.. i18n:     # Instead you can do the following and everything will be translated,
 
690
.. i18n:     # including the product name if its field definition has the
 
691
.. i18n:     # translate flag properly set:
 
692
.. i18n:     error = _("Product %s is not available!") % product.name
 
693
..
 
694
 
 
695
The rule is very simple: calls to the underscore method should *always* be in the form ``_('literal string')``
 
696
and nothing else::
 
697
 
 
698
    # Good: plain strings
 
699
    error = _('This record is locked!')
 
700
 
 
701
    # Good: strings with formatting patterns included
 
702
    error = _('Record %s cannot be modified!') % record
 
703
 
 
704
    # OK too: multi-line literal strings
 
705
    error = _("""This is a bad multiline example
 
706
                 about record %s!""") % record
 
707
    error = _('Record %s cannot be modified' \
 
708
              'after being validated!') % record
 
709
 
 
710
    # BAD: tries to translate after string formatting 
 
711
    #      (pay attention to brackets!)
 
712
    # This does NOT work and messes up the translations!
 
713
    error = _('Record %s cannot be modified!' % record)
 
714
 
 
715
    # BAD: dynamic string, string concatenation, etc are forbidden!
 
716
    # This does NOT work and messes up the translations!
 
717
    error = _("'" + que_rec['question'] + "' \n")
 
718
 
 
719
    # BAD: field values are automatically translated by the framework
 
720
    # This is useless and will not work the way you think:
 
721
    error = _("Product %s is out of stock!") % _(product.name)
 
722
    # and the following will of course not work as already explained:
 
723
    error = _("Product %s is out of stock!" % product.name)
 
724
 
 
725
    # BAD: field values are automatically translated by the framework
 
726
    # This is useless and will not work the way you think:
 
727
    error = _("Product %s is not available!") % _(product.name)
 
728
    # and the following will of course not work as already explained:
 
729
    error = _("Product %s is not available!" % product.name)
 
730
 
 
731
    # Instead you can do the following and everything will be translated,
 
732
    # including the product name if its field definition has the
 
733
    # translate flag properly set:
 
734
    error = _("Product %s is not available!") % product.name
 
735
 
 
736
.. i18n: Also, keep in mind that translators will have to work with the literal values that are passed
 
737
.. i18n: to the underscore function, so please try to make them easy to understand and keep spurious
 
738
.. i18n: characters and formatting to a minimum. Translators must be aware that formatting patterns such
 
739
.. i18n: as ``%s`` or ``%d``, newlines, etc. need to be preserved, but it's important to use these
 
740
.. i18n: in a sensible and obvious manner::
 
741
.. i18n: 
 
742
.. i18n:     # Bad: makes the translations hard to work with
 
743
.. i18n:     error = "'" + question + _("' \nPlease enter an integer value ")
 
744
.. i18n: 
 
745
.. i18n:     # Better (pay attention to position of the brackets too!)
 
746
.. i18n:     error = _("Answer to question %s is not valid.\n" \
 
747
.. i18n:               "Please enter an integer value.") % question
 
748
..
 
749
 
 
750
Also, keep in mind that translators will have to work with the literal values that are passed
 
751
to the underscore function, so please try to make them easy to understand and keep spurious
 
752
characters and formatting to a minimum. Translators must be aware that formatting patterns such
 
753
as ``%s`` or ``%d``, newlines, etc. need to be preserved, but it's important to use these
 
754
in a sensible and obvious manner::
 
755
 
 
756
    # Bad: makes the translations hard to work with
 
757
    error = "'" + question + _("' \nPlease enter an integer value ")
 
758
 
 
759
    # Better (pay attention to position of the brackets too!)
 
760
    error = _("Answer to question %s is not valid.\n" \
 
761
              "Please enter an integer value.") % question