organized flames

Search

Refactoring a simple function in BIND 9: zone_settimer()

Posted on June 04, 2010 by Michael

Just like a garage, code needs an occasional cleanup. Without spending some time going back over existing code to make it better, shorter, and better tested, it will be filled with spiders, too. I mean bugs, my garage has the spiders.

As a practical example, I’ve taken a function in BIND 9 and rewritten it incrementally to be more testable, and I think easier to understand. This function, as we will learn later, sets a timer inside the zone to expire at the time of the next zone event. What the various events mean is beyond the scope of this example, but it’s unimportant also.

While the changes are incremental, I will include the full listing in each iteration. This will make this page unbelievably large, but it will make it easier to see the steps as we follow along.

Here’s the code as it exists in BIND 9.7.0’s version of lib/dns/zone.c:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121

static void
zone_settimer(dns_zone_t *zone, isc_time_t *now) {
  const char me[] = "zone_settimer";
  isc_time_t next;
  isc_result_t result;

  ENTER;
  REQUIRE(DNS_ZONE_VALID(zone));
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING))
    return;

  isc_time_settoepoch(&next);

  switch (zone->type) {
  case dns_zone_master:
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
      next = zone->notifytime;
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
      INSIST(!isc_time_isepoch(&zone->dumptime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->dumptime, &next) < 0)
        next = zone->dumptime;
    }
    if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING) &&
        !isc_time_isepoch(&zone->refreshkeytime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->refreshkeytime, &next) < 0)
        next = zone->refreshkeytime;
    }
    if (!isc_time_isepoch(&zone->resigntime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->resigntime, &next) < 0)
        next = zone->resigntime;
    }
    if (!isc_time_isepoch(&zone->keywarntime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->keywarntime, &next) < 0)
        next = zone->keywarntime;
    }
    if (!isc_time_isepoch(&zone->signingtime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->signingtime, &next) < 0)
        next = zone->signingtime;
    }
    if (!isc_time_isepoch(&zone->nsec3chaintime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->nsec3chaintime, &next) < 0)
        next = zone->nsec3chaintime;
    }
    break;

  case dns_zone_slave:
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
      next = zone->notifytime;
    /*FALLTHROUGH*/

  case dns_zone_stub:
    if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESH) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOMASTERS) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOREFRESH) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADING)) {
      INSIST(!isc_time_isepoch(&zone->refreshtime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->refreshtime, &next) < 0)
        next = zone->refreshtime;
    }
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED)) {
      INSIST(!isc_time_isepoch(&zone->expiretime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->expiretime, &next) < 0)
        next = zone->expiretime;
    }
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
      INSIST(!isc_time_isepoch(&zone->dumptime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->dumptime, &next) < 0)
        next = zone->dumptime;
    }
    break;

  case dns_zone_key:
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
      INSIST(!isc_time_isepoch(&zone->dumptime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->dumptime, &next) < 0)
        next = zone->dumptime;
    }
    if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING)) {
      if (isc_time_isepoch(&next) ||
          (!isc_time_isepoch(&zone->refreshkeytime) &&
          isc_time_compare(&zone->refreshkeytime, &next) < 0))
        next = zone->refreshkeytime;
    }
    break;

  default:
    break;
  }

  if (isc_time_isepoch(&next)) {
    zone_debuglog(zone, me, 10, "settimer inactive");
    result = isc_timer_reset(zone->timer, isc_timertype_inactive,
            NULL, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not deactivate zone timer: %s",
             isc_result_totext(result));
  } else {
    if (isc_time_compare(&next, now) <= 0)
      next = *now;
    result = isc_timer_reset(zone->timer, isc_timertype_once,
           &next, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not reset zone timer: %s",
             isc_result_totext(result));
  }
}

Document

At a first glance, I notice two things. One, there is no description of this function, so I have to read the code to really get what it does. Two, there is a lot of duplication. Let’s fix the documentation part first. I’m something of a C expert and I know well what things like DNSZONEFLGNEEDDUMP means, but if I did not, I would spend quite a bit of time here.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157

/*
 * A zone has one timer, but has events that occur at various times.
 * Go through the various events as needed for this zone type, and find
 * the one that will trigger next.  Set the zone's timer to this event's
 * time.
 *
 * If no events are scheduled to trigger, stop the timer.
 */
static void
zone_settimer(dns_zone_t *zone, isc_time_t *now) {
  const char me[] = "zone_settimer";
  isc_time_t next;
  isc_result_t result;

  ENTER;
  REQUIRE(DNS_ZONE_VALID(zone));
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING))
    return;

  isc_time_settoepoch(&next);

  switch (zone->type) {
  case dns_zone_master:
    /*
     * If we need to send a notify, set the timer.
     */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
      next = zone->notifytime;

    /* Do we need to dump to disk, and are not doing so? */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
      INSIST(!isc_time_isepoch(&zone->dumptime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->dumptime, &next) < 0)
        next = zone->dumptime;
    }

    /* Do we need to refresh the signatures? */
    if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING) &&
        !isc_time_isepoch(&zone->refreshkeytime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->refreshkeytime, &next) < 0)
        next = zone->refreshkeytime;
    }
    
    /* Do we need to re-sign any data? */
    if (!isc_time_isepoch(&zone->resigntime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->resigntime, &next) < 0)
        next = zone->resigntime;
    }
    
    /* Do we need to warn about expiring keys? */
    if (!isc_time_isepoch(&zone->keywarntime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->keywarntime, &next) < 0)
        next = zone->keywarntime;
    }
    
    /* Do we need to sign the zone? */
    if (!isc_time_isepoch(&zone->signingtime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->signingtime, &next) < 0)
        next = zone->signingtime;
    }
    
    /* Do we need build or rebuild nsec3 chains? */
    if (!isc_time_isepoch(&zone->nsec3chaintime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->nsec3chaintime, &next) < 0)
        next = zone->nsec3chaintime;
    }
    break;

  case dns_zone_slave:
    /* Do we need to send notifies? */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
      next = zone->notifytime;
    /*FALLTHROUGH*/

  case dns_zone_stub:
    /*
     * If we are not refreshing and can, we have masters,
     * and we are not loading then check if we should do
     * do that next.
     */
    if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESH) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOMASTERS) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOREFRESH) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADING)) {
      INSIST(!isc_time_isepoch(&zone->refreshtime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->refreshtime, &next) < 0)
        next = zone->refreshtime;
    }
    
    /* Check expire time if we are loaded */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED)) {
      INSIST(!isc_time_isepoch(&zone->expiretime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->expiretime, &next) < 0)
        next = zone->expiretime;
    }
    
    /* Dump if we need to, unless we already are */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
      INSIST(!isc_time_isepoch(&zone->dumptime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->dumptime, &next) < 0)
        next = zone->dumptime;
    }
    break;

  case dns_zone_key:
    /* Dump if we need to, unless we already are */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
      INSIST(!isc_time_isepoch(&zone->dumptime));
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->dumptime, &next) < 0)
        next = zone->dumptime;
    }
    
    /* Refresh our keys if needed */
    if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING)) {
      if (isc_time_isepoch(&next) ||
          (!isc_time_isepoch(&zone->refreshkeytime) &&
          isc_time_compare(&zone->refreshkeytime, &next) < 0))
        next = zone->refreshkeytime;
    }
    break;

  default:
    break;
  }

  if (isc_time_isepoch(&next)) {  /* No timers are scheduled to trigger */
    zone_debuglog(zone, me, 10, "settimer inactive");
    result = isc_timer_reset(zone->timer, isc_timertype_inactive,
            NULL, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not deactivate zone timer: %s",
             isc_result_totext(result));
  } else { /* Set the timer to be the soonest event we need to look at */
    if (isc_time_compare(&next, now) <= 0)
      next = *now;
    result = isc_timer_reset(zone->timer, isc_timertype_once,
           &next, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not reset zone timer: %s",
             isc_result_totext(result));
  }
}

Well, things did not get smaller, but larger! That’s OK though since readability and documentation improved. Just in case, do a compile to ensure we did not break things. If changing comments broke things, get more caffeine. Let’s get to actual code changes!

Factor out some duplicated code

The goal in this refactoring is to end up with a bit of code that could be more easily tested, is more readable, and is, well, less. Less duplicated code means less to test, less possible locations for bugs, and less code to drag along into the future. It is specifically not to change the behavior of the code, just its structure.

One thing that should immediately pop out is the repeated pattern:

1
2
3
4
5

    if (!isc_time_isepoch(&zone->resigntime)) {
      if (isc_time_isepoch(&next) ||
          isc_time_compare(&zone->resigntime, &next) < 0)
        next = zone->resigntime;
    }

This seems like a first place to start on our cleanup.

This particular code is tested by existing system-level tests. What a good test-driven coder would do here is write the tests for the code they wish they had. For this cleanup, I wish I had a function that would update an accumulator-type variable with a timer if it occurred sooner than what it already held. Go be a good TDD programmer and write that.

Good, got it written? Send it to ISC, please, in the form of a unit test! I’m just going to fumble along and rely on the system-level tests to check my work for this example.

Factoring out some of that repeated code, I came up with this helper function, and refactored the code to use it:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140

/*
 * Set "previous_time" to "new_time" if "new_time" is not the epoch
 * and "new_time" will happen sooner than "previous_time", or if
 * "previous_time" is the epoch.  In this case, epoch means
 * that timer is not set.
 */
static void
set_time_if_sooner(isc_time_t *previous_time, isc_time_t *new_time) {
  if (!isc_time_isepoch(new_time)) {
    if (isc_time_isepoch(previous_time) ||
        isc_time_compare(new_time, previous_time) < 0)
      *previous_time = *new_time;
  }
}

/*
 * A zone has one timer, but has events that occur at various times.
 * Go through the various events as needed for this zone type, and find
 * the one that will trigger next.  Set the zone's timer to this event's
 * time.
 *
 * If no events are scheduled to trigger, stop the timer.
 */
static void
zone_settimer(dns_zone_t *zone, isc_time_t *now) {
  const char me[] = "zone_settimer";
  isc_time_t next;
  isc_result_t result;

  ENTER;
  REQUIRE(DNS_ZONE_VALID(zone));
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING))
    return;

  isc_time_settoepoch(&next);

  switch (zone->type) {
  case dns_zone_master:
    /*
     * If we need to send a notify, set the timer.
     */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
      next = zone->notifytime;

    /* Do we need to dump to disk, and are not doing so? */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
      INSIST(!isc_time_isepoch(&zone->dumptime));
      set_time_if_sooner(&next, &zone->dumptime);
    }

    /* Do we need to refresh the signatures? */
    if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING))
      set_time_if_sooner(&next, &zone->refreshkeytime);
    
    /* Do we need to re-sign any data? */
    set_time_if_sooner(&next, &zone->resigntime);
    
    /* Do we need to warn about expiring keys? */
    set_time_if_sooner(&next, &zone->keywarntime);
    
    /* Do we need to sign the zone? */
    set_time_if_sooner(&next, &zone->signingtime);
    
    /* Do we need build or rebuild nsec3 chains? */
    set_time_if_sooner(&next, &zone->nsec3chaintime);

    break;

  case dns_zone_slave:
    /* Do we need to send notifies? */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
      next = zone->notifytime;
    /*FALLTHROUGH*/

  case dns_zone_stub:
    /*
     * If we are not refreshing and can, we have masters,
     * and we are not loading then check if we should do
     * do that next.
     */
    if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESH) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOMASTERS) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOREFRESH) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADING))
      set_time_if_sooner(&next, &zone->refreshtime);
    
    /* Check expire time if we are loaded */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED)) {
      INSIST(!isc_time_isepoch(&zone->expiretime));
      set_time_if_sooner(&next, &zone->expiretime);
    }
    
    /* Dump if we need to, unless we already are */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
      INSIST(!isc_time_isepoch(&zone->dumptime));
      set_time_if_sooner(&next, &zone->dumptime);
    }

    break;

  case dns_zone_key:
    /* Dump if we need to, unless we already are */
    if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
        !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
      INSIST(!isc_time_isepoch(&zone->dumptime));
      set_time_if_sooner(&next, &zone->dumptime);
    }
    
    /* Refresh our keys if needed */
    if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING))
      set_time_if_sooner(&next, &zone->refreshkeytime);

    break;

  default:

    break;
  }

  if (isc_time_isepoch(&next)) {  /* No timers are scheduled to trigger */
    zone_debuglog(zone, me, 10, "settimer inactive");
    result = isc_timer_reset(zone->timer, isc_timertype_inactive,
            NULL, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not deactivate zone timer: %s",
             isc_result_totext(result));
  } else { /* Set the timer to be the soonest event we need to look at */
    if (isc_time_compare(&next, now) <= 0)
      next = *now;
    result = isc_timer_reset(zone->timer, isc_timertype_once,
           &next, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not reset zone timer: %s",
             isc_result_totext(result));
  }
}

What we have now is a small helper function that we can test, and when it has proven itself we don’t need to worry about cut and paste typos nearly as much. We have less code to test, because we removed a lot of duplication.

Zone-type specific helper functions

Is there more we can do, you ask? There certainly is! I immediately notice that we have a switch statement, and each of its cases are rather long. Let’s make some more helper functions for each of these cases.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161

/*
 * Set "previous_time" to "new_time" if "new_time" is not the epoch
 * and "new_time" will happen sooner than "previous_time", or if
 * "previous_time" is the epoch.  In this case, epoch means
 * that timer is not set.
 */
static void
set_time_if_sooner(isc_time_t *previous_time, isc_time_t *new_time) {
  if (!isc_time_isepoch(new_time)) {
    if (isc_time_isepoch(previous_time) ||
        isc_time_compare(new_time, previous_time) < 0)
      *previous_time = *new_time;
  }
}

/* find next event for a master zone */
static void
check_master_zone_timers(isc_time_t *next, dns_zone_t *zone) {
  /*
   * If we need to send a notify, set the timer.
   */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
    *next = zone->notifytime;

  /* Do we need to dump to disk, and are not doing so? */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
    INSIST(!isc_time_isepoch(&zone->dumptime));
    set_time_if_sooner(next, &zone->dumptime);
  }

  /* Do we need to refresh the signatures? */
  if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING))
    set_time_if_sooner(next, &zone->refreshkeytime);
  
  /* Do we need to re-sign any data? */
  set_time_if_sooner(next, &zone->resigntime);
  
  /* Do we need to warn about expiring keys? */
  set_time_if_sooner(next, &zone->keywarntime);
  
  /* Do we need to sign the zone? */
  set_time_if_sooner(next, &zone->signingtime);
  
  /* Do we need build or rebuild nsec3 chains? */
  set_time_if_sooner(next, &zone->nsec3chaintime);
}

/* find next event for a slave zone */
static void
check_slave_zone_timers(isc_time_t *next, dns_zone_t *zone) {
  /* Do we need to send notifies? */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
    *next = zone->notifytime;
}

/* find next event for a stub (or slave) zone */
static void
check_stub_or_slave_zone_timers(isc_time_t *next, dns_zone_t *zone) {
  /*
   * If we are not refreshing and can, we have masters,
   * and we are not loading then check if we should do
   * do that next.
   */
  if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESH) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOMASTERS) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOREFRESH) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADING))
    set_time_if_sooner(next, &zone->refreshtime);
  
  /* Check expire time if we are loaded */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED)) {
    INSIST(!isc_time_isepoch(&zone->expiretime));
    set_time_if_sooner(next, &zone->expiretime);
  }
  
  /* Dump if we need to, unless we already are */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
    INSIST(!isc_time_isepoch(&zone->dumptime));
    set_time_if_sooner(next, &zone->dumptime);
  }
}

/* find next event for a key zone */
static void
check_key_zone_timers(isc_time_t *next, dns_zone_t *zone) {
  /* Dump if we need to, unless we already are */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
    INSIST(!isc_time_isepoch(&zone->dumptime));
    set_time_if_sooner(next, &zone->dumptime);
  }
  
  /* Refresh our keys if needed */
  if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING))
    set_time_if_sooner(next, &zone->refreshkeytime);
}

/*
 * A zone has one timer, but has events that occur at various times.
 * Go through the various events as needed for this zone type, and find
 * the one that will trigger next.  Set the zone's timer to this event's
 * time.
 *
 * If no events are scheduled to trigger, stop the timer.
 */
static void
zone_settimer(dns_zone_t *zone, isc_time_t *now) {
  const char me[] = "zone_settimer";
  isc_time_t next;
  isc_result_t result;

  ENTER;
  REQUIRE(DNS_ZONE_VALID(zone));
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING))
    return;

  isc_time_settoepoch(&next);

  switch (zone->type) {
  case dns_zone_master:
    check_master_zone_timers(&next, zone);
    break;

  case dns_zone_slave:
    check_slave_zone_timers(&next, zone);
    /*FALLTHROUGH*/

  case dns_zone_stub:
    check_stub_or_slave_zone_timers(&next, zone);
    break;

  case dns_zone_key:
    check_key_zone_timers(&next, zone);
    break;

  default:

    break;
  }

  if (isc_time_isepoch(&next)) {  /* No timers are scheduled to trigger */
    zone_debuglog(zone, me, 10, "settimer inactive");
    result = isc_timer_reset(zone->timer, isc_timertype_inactive,
            NULL, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not deactivate zone timer: %s",
             isc_result_totext(result));
  } else { /* Set the timer to be the soonest event we need to look at */
    if (isc_time_compare(&next, now) <= 0)
      next = *now;
    result = isc_timer_reset(zone->timer, isc_timertype_once,
           &next, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not reset zone timer: %s",
             isc_result_totext(result));
  }
}

Now we have even more bite-sized, testable functions which we can target and verify independently with unit tests. As before, a good TDD coder would have written unit tests for each of these functions before factoring them out.

Switching

Is there anything else that we might want to do? The switch statement itself should go into a helper function.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167

/*
 * Set "previous_time" to "new_time" if "new_time" is not the epoch
 * and "new_time" will happen sooner than "previous_time", or if
 * "previous_time" is the epoch.  In this case, epoch means
 * that timer is not set.
 */
static void
set_time_if_sooner(isc_time_t *previous_time, isc_time_t *new_time) {
  if (!isc_time_isepoch(new_time)) {
    if (isc_time_isepoch(previous_time) ||
        isc_time_compare(new_time, previous_time) < 0)
      *previous_time = *new_time;
  }
}

/* find next event for a master zone */
static void
check_master_zone_timers(isc_time_t *next, dns_zone_t *zone) {
  /*
   * If we need to send a notify, set the timer.
   */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
    *next = zone->notifytime;

  /* Do we need to dump to disk, and are not doing so? */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
    INSIST(!isc_time_isepoch(&zone->dumptime));
    set_time_if_sooner(next, &zone->dumptime);
  }

  /* Do we need to refresh the signatures? */
  if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING))
    set_time_if_sooner(next, &zone->refreshkeytime);
  
  /* Do we need to re-sign any data? */
  set_time_if_sooner(next, &zone->resigntime);
  
  /* Do we need to warn about expiring keys? */
  set_time_if_sooner(next, &zone->keywarntime);
  
  /* Do we need to sign the zone? */
  set_time_if_sooner(next, &zone->signingtime);
  
  /* Do we need build or rebuild nsec3 chains? */
  set_time_if_sooner(next, &zone->nsec3chaintime);
}

/* find next event for a slave zone */
static void
check_slave_zone_timers(isc_time_t *next, dns_zone_t *zone) {
  /* Do we need to send notifies? */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDNOTIFY))
    *next = zone->notifytime;
}

/* find next event for a stub (or slave) zone */
static void
check_stub_or_slave_zone_timers(isc_time_t *next, dns_zone_t *zone) {
  /*
   * If we are not refreshing and can, we have masters,
   * and we are not loading then check if we should do
   * do that next.
   */
  if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESH) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOMASTERS) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NOREFRESH) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADING))
    set_time_if_sooner(next, &zone->refreshtime);
  
  /* Check expire time if we are loaded */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADED)) {
    INSIST(!isc_time_isepoch(&zone->expiretime));
    set_time_if_sooner(next, &zone->expiretime);
  }
  
  /* Dump if we need to, unless we already are */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
    INSIST(!isc_time_isepoch(&zone->dumptime));
    set_time_if_sooner(next, &zone->dumptime);
  }
}

/* find next event for a key zone */
static void
check_key_zone_timers(isc_time_t *next, dns_zone_t *zone) {
  /* Dump if we need to, unless we already are */
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_NEEDDUMP) &&
      !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
    INSIST(!isc_time_isepoch(&zone->dumptime));
    set_time_if_sooner(next, &zone->dumptime);
  }
  
  /* Refresh our keys if needed */
  if (!DNS_ZONE_FLAG(zone, DNS_ZONEFLG_REFRESHING))
    set_time_if_sooner(next, &zone->refreshkeytime);
}

/* Find the next event based on zone type. */
static void
check_zone_timers(isc_time_t *next, dns_zone_t *zone) {
  switch (zone->type) {
  case dns_zone_master:
    check_master_zone_timers(next, zone);
    break;

  case dns_zone_slave:
    check_slave_zone_timers(next, zone);
    /*FALLTHROUGH*/

  case dns_zone_stub:
    check_stub_or_slave_zone_timers(next, zone);
    break;

  case dns_zone_key:
    check_key_zone_timers(next, zone);
    break;

  default:

    break;
  }
}

/*
 * A zone has one timer, but has events that occur at various times.
 * Go through the various events as needed for this zone type, and find
 * the one that will trigger next.  Set the zone's timer to this event's
 * time.
 *
 * If no events are scheduled to trigger, stop the timer.
 */
static void
zone_settimer(dns_zone_t *zone, isc_time_t *now) {
  const char me[] = "zone_settimer";
  isc_time_t next;
  isc_result_t result;

  ENTER;
  REQUIRE(DNS_ZONE_VALID(zone));
  if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING))
    return;

  isc_time_settoepoch(&next);

  check_zone_timers(&next, zone);

  if (isc_time_isepoch(&next)) {  /* No timers are scheduled to trigger */
    zone_debuglog(zone, me, 10, "settimer inactive");
    result = isc_timer_reset(zone->timer, isc_timertype_inactive,
            NULL, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not deactivate zone timer: %s",
             isc_result_totext(result));
  } else { /* Set the timer to be the soonest event we need to look at */
    if (isc_time_compare(&next, now) <= 0)
      next = *now;
    result = isc_timer_reset(zone->timer, isc_timertype_once,
           &next, NULL, ISC_TRUE);
    if (result != ISC_R_SUCCESS)
      dns_zone_log(zone, ISC_LOG_ERROR,
             "could not reset zone timer: %s",
             isc_result_totext(result));
  }
}

Now we have a simpler function that is easier to read. It is something of a personal preference if we should factor out the case statement. I can see both sides of this. Is the final function easier or harder to read? This is also something of a personal preference, but I believe most people would say it is easier.

Knowing when to stop

Whew, lots of changes so far. Anything else? I would say no, at this point. We could factor the timer setting / canceling code into another helper function, especially if that pattern repeats a lot elsewhere. However, for this code, I think it’s fine to leave it as it stands. There is still some duplication, such as checking for dumptime in multiple functions, but I think that’s fine. The check for the result of timer operations is duplicated as well, but that too seems simple enough as it stands.

Knowing when to stop refactoring is just as critical as identifying the need to refactor. This comes with experience, time, and patience. When the code becomes longer and harder to read by refactoring, it’s a mistake; when the code becomes longer but is more readable, it’s OK. Most of the length increase we added was not code but comments, whitespace, and function declarations. That’s quite OK.

Filed under: Programming | Tagged with:   | 0 comments

How I test Ruby on Rails with RSpec and Cucumber

Posted on November 19, 2009 by Michael

In any non-trivial application, I end up with several things in common.

  • Generic pages which anyone can see. (I usually make this the About controller)
  • Users
  • Things (DNS Zones, pictures, etc)
  • Access restrictions to those users and things
  • Tests to ensure access control to those users and things

When to use RSpec, and when to use Cucumber

I use RSpec for unit tests, and some low-level controller tests. I specifically do not use RSpec for “user experience” or multiple-step testing, so no views are tested using RSpec.

Cucumber is used for all things “user experience.” It will test that a user cannot access other’s pages, etc. but it doesn’t do anything that can’t easily be done by filling in forms or changing the URL.

Types of users

In my world, there are three kinds of users which appear over and over again.

  • Guests. These guys can look at anything in the About controller.
  • Logged-in users. These can look at anything they own and some things that they do not. They can edit anything they own (with security restrictions on some fields.)
  • Administrators. These are the ones who can look at and modify anything. I usually have an admin rights on/off toggle, and try hard to make what they see close to what users would see.

User tests

  • Tests to ensure that the public cannot do things.
  • Tests to ensure that a logged-in user cannot poke around in someone else’s business using usual page contents.
  • Tests to ensure the the public or a logged in user cannot use specially crafted requests to break things.
  • Tests to ensure that a user can change their own stuff.
  • Tests to ensure that an admin can do pretty much everything.

Most of these tests are a combination of RSpec and Cucumber. The “hack” type tests (like directly fiddling around with assignments which my normal forms may limit to only items the logged in user has access to) are best done with RSpec.

Examples

No blog post can possibly be useful without examples. They use Factory Girl to generate a user, which has an email, password, and an admin flag.

spec/spec_helper.rb snippit

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18

def login_user(options = {})
  @logged_in_user = Factory.create(:user, options)
  @controller.stub!(:current_user).and_return(@logged_in_user)
  @logged_in_user
end

def login_admin(options = {})
  options[:admin] = true
  @logged_in_user = Factory.create(:user, options)
  @controller.stub!(:current_user).and_return(@logged_in_user)
  @logged_in_user
end

def logout_user
  @logged_in_user = nil
  @controller.stub!(:current_user).and_return(@logged_in_user)
  @logged_in_user
end

My complete spec/controllers/userscontrollerspec.rb file

Sorry for the length of this part.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270

require 'spec_helper'

describe UsersController do
  setup :activate_authlogic

  before(:each) do
    logout_user
    @other_user = Factory.create(:user)
  end

  def mock_user(stubs={})
    @mock_user ||= mock_model(User, stubs)
  end

  def make_users
    users = [ @other_user ]
    users << @logged_in_user if @logged_in_user
    4.times do
      users << Factory.create(:user)
    end
    users  
  end

  describe "GET index" do
    it "assigns all users as @users (admin)" do
      login_admin
      users = make_users
      get :index
      assigns[:users].sort.should == users.sort
    end

    it "tells me to bugger off (not admin)" do
      login_user
      users = make_users
      get :index
      flash[:error].should match "You must be an administrator to access this page."
      response.should redirect_to(root_path)
    end

    it "tells me to bugger off (not logged in)" do
      users = make_users
      get :index
      flash[:error].should match "You must be an administrator to access this page."
      response.should redirect_to(root_path)
    end
  end

  describe "GET show" do
    it "assigns myself as @user (admin)" do
      login_admin
      get :show, :id => @logged_in_user.id
      assigns[:user].should == @logged_in_user
    end

    it "assigns me as @user (my data)" do
      login_user
      get :show, :id => @logged_in_user.id
      assigns[:user].should == @logged_in_user
    end

    it "assigns the requested user as @user (admin)" do
      login_admin
      get :show, :id => @other_user.id
      assigns[:user].should == @other_user
    end

    it "tells me to bugger off (not admin)" do
      login_user
      get :show, :id => @other_user.id
      flash[:error].should match "You must be an administrator to access this page."
    end

    it "redirects to root (not admin)" do
      login_user
      get :show, :id => @other_user.id
      response.should redirect_to(root_path)
    end

    it "tells me to log in (not logged in)" do
      get :show, :id => @other_user.id
      flash[:error].should match "Please log in to access this page."
    end

    it "redirects to root (not logged in)" do
      get :show, :id => @other_user.id
      response.should redirect_to(login_path)
    end
  end

  describe "GET new" do
    it "assigns a new user as @user (not logged in)" do
      User.stub!(:new).and_return(mock_user)
      get :new
      assigns[:user].should equal(mock_user)
    end
  end

  describe "GET edit" do
    it "assigns me as @user (admin)" do
      login_admin
      get :edit, :id => @logged_in_user.id
      assigns[:user].should == @logged_in_user
    end

    it "assigns the requested user as @user (admin)" do
      login_admin
      get :edit, :id => @logged_in_user.id
      assigns[:user].should == @logged_in_user
    end

    it "assigns me as @user (my data)" do
      login_user
      get :edit, :id => @logged_in_user.id
      assigns[:user].should == @logged_in_user
    end

    it "tells me to bugger off (not admin)" do
      login_user
      get :edit, :id => @other_user.id
      flash[:error].should match "You must be an administrator to access this page."
      end

    it "redirects to root (not admin)" do
      login_user
      get :edit, :id => @other_user.id
      response.should redirect_to(root_path)
    end

    it "tells me to log in (not logged in)" do
      get :edit, :id => @other_user.id
      flash[:error].should match "Please log in to access this page."
    end

    it "redirects to login (not logged in)" do
      get :edit, :id => @other_user.id
      response.should redirect_to(login_path)
    end
  end

  describe "PUT update" do
    describe "with valid params" do
      it "assigns me as @user (my data)" do
        login_user
        put :update, :id => @logged_in_user.id
        assigns[:user].should == @logged_in_user
      end

      it "tells me to bugger off (not admin)" do
        login_user
        put :update, :id => @other_user.id
        flash[:error].should match "You must be an administrator to access this page."
      end

      it "tells me to log in (not logged in)" do
        get :edit, :id => @other_user.id
        flash[:error].should match "Please log in to access this page."
      end

      it "assigns the requested user as @user (admin)" do
        login_admin
        put :update, :id => @logged_in_user.id
        assigns[:user].should == @logged_in_user
      end

      it "redirects to the user (admin)" do
        login_admin
        put :update, :id => @logged_in_user.id
        response.should redirect_to(user_url(@logged_in_user))
      end

      it "redirects to me (my data)" do
        login_user
        put :update, :id => @logged_in_user.id
        response.should redirect_to(user_url(@logged_in_user))
      end

      it "redirects to root (not admin)" do
        login_user
        put :update, :id => @other_user.id
        response.should redirect_to(root_path)
      end

      it "redirects to login (not logged in)" do
        put :update, :id => @other_user.id
        response.should redirect_to(login_path)
      end

      it "can edit anyone's data (admin)" do
        login_admin
        put :update, :id => @other_user.id, :user => { :email => "new_email@example.com"}
        response.should redirect_to(user_url(@other_user))
        assigns[:user].email.should == "new_email@example.com"
      end

      it "can edit my own data (not admin)" do
        login_user
        put :update, :id => @logged_in_user.id, :user => { :email => "new_email@example.com"}
        response.should redirect_to(user_url(@logged_in_user))
        assigns[:user].email.should == "new_email@example.com"
      end
    end

    describe "with invalid params" do
      it "updates the requested user (admin)" do
        login_admin
        User.should_receive(:find).with("37").and_return(mock_user)
        mock_user.should_receive(:update_attributes).with({'these' => 'params'})
        put :update, :id => "37", :user => {:these => 'params'}
      end

      it "assigns the user as @user (admin)" do
        login_admin
        User.stub!(:find).and_return(mock_user(:update_attributes => false))
        put :update, :id => "1"
        assigns[:user].should equal(mock_user)
      end

      it "re-renders the 'edit' template (admin)" do
        login_admin
        User.stub!(:find).and_return(mock_user(:update_attributes => false))
        put :update, :id => "1"
        response.should render_template('edit')
      end
    end

  end

  describe "DELETE destroy" do
    it "destroys the requested user (admin)" do
      login_admin
      User.should_receive(:find).with("37").and_return(mock_user)
      mock_user.should_receive(:destroy)
      delete :destroy, :id => "37"
    end

    it "redirects to the users list (admin)" do
      login_admin
      User.stub!(:find).and_return(mock_user(:destroy => true))
      delete :destroy, :id => "1"
      response.should redirect_to(users_url)
    end

    it "destroys me (my data)" do
      login_user
      delete :destroy, :id => @logged_in_user.id
    end

    it "tells me to bugger off (not admin)" do
      login_user
      delete :destroy, :id => @other_user.id
      flash[:error].should match "You must be an administrator to access this page."
    end

    it "tells me to log in (not logged in)" do
      delete :destroy, :id => @other_user.id
      flash[:error].should match "Please log in to access this page."
    end

    it "redirects to root (not admin)" do
      login_user
      delete :destroy, :id => @other_user.id
      response.should redirect_to(root_path)
    end

    it "redirects to login (not logged in)" do
      delete :destroy, :id => @other_user.id
      response.should redirect_to(login_path)
    end
  end
end

Filed under: Programming | Tagged with:   | 0 comments

attr_accessible vs accepts_nested_attributes_for

Posted on November 10, 2009 by Michael

First, I just have to say that I never thought I’d be in Japan, let along blogging from here.

Today I ran into a little gotcha with restricting access to models when trying out the Rails 2.3 method of nested models. In my model, I have:

1
2
3
4
5
6

  has_many :dnskeys, :dependent => true
  accepts_nested_attributes_for :dnskeys,
    :allow_destroy => true,
    :reject_if => proc { |attrs| attrs.all? { |k, v| v.blank? }}

  attr_accessible :name

I was surprised, although I should not have been, that this prevented me from posting a zone and its dnskeys in the same action. Adding the proper attr_accessible line made the magic happen again:



  attr_accessible :dnskeys_attributes

Filed under: Programming | Tagged with:   | 1 comment

Experiments with HAML

Posted on November 05, 2009 by Michael

I decided to try HAML today, rather than the standard ERB templates. The jury is still out on if it is better or worse, but like many things Rails, it is better to just dive in and try it.

I have a little project (not yet released) which I’m converting over to use HAML. Since I also use rspec for testing, along with Cucumber and Webrat, I thought I had a long day ahead of me.

It turns out most of it can be made close to automatic, with just a little cleanup afterwards.

Using the rake task below, you can issue these two commands to convert your existing *.html.erb views into *.html.haml. The second target will convert the rspec view tests to expect haml. Renaming the spec tests is not required, but I like sanity.

1
2

  $ rake haml:convert:html
  $ rake haml:convert:spec

If you use a SCM (I still like Subversion best) you’ll want to remove the old files and add the new ones. For Subversion, I do:

1
2

  $ svn rm app/views/*/*.html.erb spec/views/*/*.html.erb_spec.rb
  $ svn add app/views/*/*.html.haml spec/views/*/*.html.haml_spec.rb

The main problems were that not all of the templates were properly indented, and “- end” lines appeared in the HAML templates. Fixing this was fast, as I really don’t have much in my templates yet as this is a new project. This step too could probably be done in an automated way. However, the indentation is probably going to require a bit of manual work.

While there, I also fixed up multi-line constructs generated with a much nicer compact format:

1
2
3
4
5
6
7
8

  %h1
    Title
  %tr
    %th
      Heading
    %th
      Heading
...

Compare with this:

1
2
3
4
5
6

  %h1 Title

  %tr
    %th Heading
    %th Heading
...

And now for the rake task:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33

# RAILS_ROOT/lib/tasks/haml_convert.rake

require 'haml'
require 'haml/exec'
 
namespace :haml do
  namespace :convert do
    desc "Convert all *.html.erb files to *.html.haml"
    task :html do
      Dir.glob("app/views/**/*.html.erb").each do |html|
        puts html + "..."
        Haml::Exec::HTML2Haml.new([html, html.gsub(".html.erb", ".html.haml")]).process_result
        File.delete(html)
      end
    end

    desc "Convert all *.html.erb_spec.rb files to *.html.haml_spec.rb"
    task :spec do
      Dir.glob("spec/views/**/*.html.erb_spec.rb").each do |oldname|
        puts oldname + "..."
        newname = oldname.gsub(".html.erb", ".html.haml")
        nf = File.open(newname, "w")
        File.open(oldname) do |of|
          of.each_line do |line|
            nf.puts line.gsub(".html.erb", ".html.haml")
          end
        end
        nf.close
        #File.delete(oldname)
      end
    end
  end
end

Filed under: Programming | Tagged with:   | 0 comments

ActiveResource and Shallow Nested Routes

Posted on November 04, 2009 by Michael

I recently tried to dive into ActiveResource, which allows a Ruby (usually Ruby on Rails) application to connect to a remote RESTful API and treat it almost as if it were local. There were some problems with my API from ActiveResource’s point of view.

Firstly, I use nested routes. This isn’t really a problem as it is possible to specify a prefix to add to the path, and even to substitution on this path. However, Rails added the concept of a “shallow nested route” a while back.

A shallow nested route is a short-hand notation which allows one to appear to scope out collections (that is, a list of things) from the actual thing itself. For example, in my application (https://dlv.isc.org/) I have:

/users/123 <– specific userid

/users/123/zones <— list of all zones for user 123

/zones/456 <— specific zone

This allows me to look at a specific user’s zones (or them, their own zones) without having to do some sort of special back-end processing which relies on something not in the path, such as the @current_user instance variable. After all, how would an admin list a user’s zones if /zones returned the current user’s zones only? Through an admin interface probably, but that seems messy. Shallow routes are seemingly more clean.

However, they break ActiveResource’s concept of the world.

With a little trickery, however, I managed to get shallow nested routes to work without having to do much additional work. I did have to pass in an additional argument to the collection list(s) however.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26

class Zone < ActiveResource::Base
  self.site = SITE
  self.user = USERNAME
  self.password = PASSWORD
  
  def self.collection_path(prefix_options = {}, query_options = nil)
    prefix_options, query_options = split_options(prefix_options) if query_options.nil?
    "/users/#{USERID}#{prefix(prefix_options)}#{collection_name}.#{format.extension}#{query_string(query_options)}"
  end
end

class Dnskey < ActiveResource::Base
  self.site = SITE
  self.user = USERNAME
  self.password = PASSWORD
  
  def self.collection_path(prefix_options = {}, query_options = nil)
    prefix_options, query_options = split_options(prefix_options) if query_options.nil?
    z = ''
    if query_options.has_key?(:zone_id)
      z = "/zones/#{query_options[:zone_id]}"
      query_options.delete(:zone_id)
    end
    "#{z}#{prefix(prefix_options)}#{collection_name}.#{format.extension}#{query_string(query_options)}"
  end
end

In my case, I used a constant USERID to the Zone’s collection, but left the specific item (aka “elementpath”) alone. For Dnskeys, where I needed to pass in different zoneid values, I had to do a small trick.

I call this as:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

all_zones = Zones.find :all # returns a list of all zones for the USERID
z = Zone.find(1) # returns only Zone with the id of 1
z.destroy # destroys the zone, uses the element_path()

z = Zone.find(2)
keys = Dnskey.find(:all, { :zone_id => z.id }) # disgusting, but works

#
# This is a hack.  I want to use Dnskey.create here, but it won't work since I cannot
# pass the zone_id along, and there are no association hints so I can't use
# zone.dnskeys.create() like I can with ActiveRecord.
#
d = Dnskey.new(:flags => key.flags, :algorithm => key.algorithm.to_i, ...)
d.prefix_options[:zone_id] = zone.id
d.save

Filed under: Programming | Tagged with:   | 0 comments

A Real XML API and Rails

Posted on March 06, 2009 by Michael

I recently implemented an XML API that I intended to be used outside of a web browser. Much of the words others have written on the topic are ways to get a Javascript framework to use the authentication_token magic. Some others show the GET side and mention the DELETE but omit the PUT and POST methods.

Here are things I’ve learned:

  • To ensure XML data is returned, use the correct HTTP header: Accept: application/xml
  • Rails assumes that multipart forms and url-encoded forms are from browsers. You can’t use them in a default Rails setup if you want to avoid the authentication_token check.
  • To POST or PUT, use a header of Content-Type: application/xml and include XML data.

It is rather unfortunate that Rails assumes that the encoding ties into a browser. It should be possible to use any encoding so long as the XML data types in the headers are correct. This is probably a bug, but it might be that if you receive XML through an API, you should send XML too.

Example

1
2
3
4
5
6
7
8

curl --user user:password -H 'Accept: application/xml' \
  -H 'Content-Type: application/xml' \
  -X POST -d '<?xml version="1.0" encoding="UTF-8"?>
<item>
  <name>foo</name>
  <description>bar</description>
  <price>100</price>
</item>' http://localhost:3000/items/create

Tagged with:   | 0 comments

Ruby and OpenSSL

Posted on February 28, 2009 by Michael

I recently had to do some DNSSEC-type (somewhat low-level) cryptography work, and found the seeming lack of Ruby OpenSSL documentation a big pain. I found numerous examples of how OpenSSL is commonly used with PEM-encoded keys, but precious little information on low-level key loading. To save others the trouble of having to dig up some of this, I’ve collected some short examples of how to do low-level RSA and DSA building from a lower level than most use.

This table summarizes the variables which need to be set to use an RSA public and private key.

RSA Keys

Key TypeItemDescription
RSA PublicePublic Exponent
RSAnModulus
RSA PrivatedPrivate Exponent
RSA PrivatepPrime 1
RSA PrivateqPrime 2
RSA Privatedmq1Exponent 1
RSA Privatedmp1Exponent 2
RSA PrivateiqmpCoefficient

Thus, in order to make a working RSA public key (so the method key.publicencrypt() or key.publicdecrypt() work) you must set at least n and e. For a working private key, you would need to load all of the items. Exposing any of the items marked as “RSA Private” above will cause a key compromise.

RSA Example

In this example, a 128-bit RSA key is loaded from numerical values. In DNSSEC, the public key is stored in the DNSKEY record for the zones. Don’t use these numbers for real crypto; the short key length is used only to make the numbers short enough to fit in the screen width. For real work, 1024 is probably a reasonable minimum length for short-lived uses, and 2048 for longer-term use.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31

require 'openssl'

#
# Build a RSA public key.  We only need to load two things
# here in order to use the public key to use it to encrypt,
# sign, or verify.
#
pub = OpenSSL::PKey::RSA::new
pub.e = 65537
pub.n = 216457604585180710748301099018726389113

# At this point, this will work:
crypted = pub.public_encrypt("test")

#
# Build an RSA private key.  For the private key to work, we need
# to load the entire key, private and public components.  As we
# should have access to both, this is not really a problem.
#
prv = OpenSSL::PKey::RSA::new
prv.e = 65537
prv.d = 178210827022942698143906513631075003381
prv.n = 216457604585180710748301099018726389113
prv.p = 15294921647876231099
prv.q = 14152253249053866587
prv.dmp1 = 6806715058393856237
prv.dmq1 = 637679537428568107
prv.iqmp = 6672106206837437412

# Now we have a working private key.
puts prv.private_decrypt(crypted) # prints "test"

DSA keys

A DSA key is more or less the same, just with different variable names. It is also split into a public and private part, and the key can be loaded from individual components just as easily.

Key TypeItemDescription
DSA Publicpub_keyPublic Key
DSAqPrime 1
DSApPrime 2
DSAgMultiplicative order modulo p is q
DSA Privatepriv_keyPrivate Key

DSA Example

Unfortunately, this example has some numbers which are too long to display nicely. I have used a trick to convert them from strings into integers so they will fit here. Normally you would not need to do this.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46

require 'openssl'

#
# Build an DSA private key.  For the private key to work, we need
# to load the entire key, private and public components.  As we
# should have access to both, this is not really a problem.
#
prv = OpenSSL::PKey::DSA::new
prv.pub_key = ("899167044393666062859565588228279347268072456516837337" +
               "963353916587148226144760114643916732975837345856985656" +
               "3340384802383806137452386519280693373122367959").to_i
prv.p = ("952649509730281181203079535805855260554748337655197352471196" +
         "869232197576949258404031665397657842790773780623545384978542" +
         "6685417827665656974405272756289291").to_i
prv.q = 903197981571669745498020976355730183999507610553
prv.g = ("535694721480531756072717909769318961974692885092552247120424" +
         "749877864650255208980198391972633196543370921493242375015765" +
         "755160911031468160738717891191998").to_i
prv.priv_key = 557886499717422048101097620625259920363848888840

# At this point, this will work:
signature = prv.sign(OpenSSL::Digest::DSS1.new, "test")

#
# Build a DSA public key.  We only need to load two things
# here in order to use the public key to use it to encrypt,
# sign, or verify.
#
pub = OpenSSL::PKey::DSA::new
pub.pub_key = ("899167044393666062859565588228279347268072456516837337" +
               "963353916587148226144760114643916732975837345856985656" +
               "3340384802383806137452386519280693373122367959").to_i
pub.p = ("952649509730281181203079535805855260554748337655197352471196" +
         "869232197576949258404031665397657842790773780623545384978542" +
         "6685417827665656974405272756289291").to_i
pub.q = 903197981571669745498020976355730183999507610553
pub.g = ("535694721480531756072717909769318961974692885092552247120424" +
         "749877864650255208980198391972633196543370921493242375015765" +
         "755160911031468160738717891191998").to_i

# Now we have a working private key.  Verify the signature
if pub.verify(OpenSSL::Digest::DSS1.new, signature, "test")
  puts "Signature verified."
else
  puts "Signature verification failed."
end

Tagged with:   | 0 comments

Find conditions from params[]

Posted on February 28, 2009 by Michael

I have often wished to add a trivial search capability to my controllers which would allow searching on different fields. Without getting into a mess of many different if statements each of which has a different set of conditions, I use something much like the following:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17

def index
  cond_hash = {}
  cond_strings = []

  if params[:search]
    cond_hash[:login] = "%#{params[:search]}%"
    cond_hash[:email] = "%#{params[:search]}%"
    cond_strings << "(login ILIKE :login OR email ILIKE :email)"
  end
  if params[:search_address]
    cond_hash[:address] = params[:search_address]
    cond_strings << "(address == :address)"
  end

  conditions = cond_strings.join(" AND ")
  @users = User.all :conditions => [ conditions, cond_hash ]
end

It may be safer to use users.login, users.email, and users.address in the SQL-like strings above.

Tagged with:   | 0 comments

Rails 2.0 and cool error handling

Posted on February 28, 2009 by Michael

Rails is a great framework, but it has gained something of a bad reputation in terms of error reporting. Everyone has seen them – those ugly 500-status “we’re sorry, but something has gone wrong” messages.

In a finished application, this just doesn’t cut it. It used to be necessary to trap the error using something like this:

1
2
3
4
5
6
7
8

def rescue_action_in_public(exception)
  case exception
  when ::ActiveRecord::RecordNotFound
    render :file => "#{Rails.public_path}/404.html", :status => 404
  else
    render :file => "#{Rails.public_path}/500.html", :status => 500
  end
end

The problem is that, as error causes begin to pile up, this method gets messy. Plus, you might want to render something different in one controller than in another, and would either need to duplicate all the logic, put controller-specific cases in the ApplicationController, or call ApplicationController::rescueactionin_public directly.

Enter Rails 2.x error handling. Drop this in your controller or ApplicationController.

1
2
3

rescue_from(ActiveRecord::RecordNotFound) do |e|
  render :file => "#{Rails.public_path}/404_record_not_found.html", :status => 404
end

By building up a rich set of rescue_from handlers in the ApplicationController, and only overriding the ones you wish to make per-controller, you have fine-grained control. And they work exactly the same in development and testing as they do in production.

Tagged with:   | 0 comments

Ruby Regular Expression Gotchas

Posted on February 26, 2009 by Michael

I love Ruby. I love Ruby on Rails. Rarely have I found a language or a framework that just works.

However, you still have to know the finer details sometimes. I recently made a model for a DNS zone. The name in the model is the “front part” of a fully qualified domain name. For instance, if zone.name = “foo” then I would write the name into my name server’s configuration files as “foo.example.com.”

Knowing that people were evil, I saw that if a user put a string in like “example.com. NS hackerz-will-someday-rule-the-earth.ru.\nfoo” I would happily write out two strings, one being rather bad.

Knowing how easy this sort of data validation is in Rails, I made my model look like:

1
2
3
4
5
6
7

class Zone < ActiveRecord::Base
  validates_presence_of :name
  validates_uniqueness_of :name
  validates_format_of :name,
    :with => /^[a-zA-Z0-9\-\_\.]+$/,
    :message => "contains invalid characters."
end

Happy, I ran a few tests using my browser and found that I could not insert names with spaces, colons, tabs, etc. Then, several days later, I decided it was time to write tests for this.

1
2
3
4
5
6
7
8
9
10
11
12
13
14

require 'test_helper'
class ZoneTest < ActiveSupport::TestCase
  def test_name_with_newline_fails
    z = Zone.new(:name => "test\nzone")
    assert !z.valid?
    assert z.errors.on(:name)
  end

  def test_name_with_space_fails
    z = Zone.new(:name => "test zone")
    assert !z.valid?
    assert z.errors.on(:name)
  end
end

Imagine my surprise when testnamewithspacefails() passed, and the one I was most worried about, testnamewithnewlinefails(), did not!

Not all regular expressions are alike.

The problem is in what I thought ^ and $ actually matched. I thought these meant “match the beginning and ending of the string.” However, it turns out it means “match the beginning and ending of each line contained in the string,” where lines are divided by newlines. Ooops.

Changing ^ into \A and $ into \Z fixed this problem. Now I’m auditing all the code in this application to see if there are other problems like this.

This is just one thing to add to an ever-growing security checklist for my Rails work. It’s also a very typical security hole: programmer error.

Tagged with:   | 0 comments

String#bitruncate

Posted on February 21, 2009 by Michael

And that’s ‘“bi-truncate” not “bit-uncate”.

What’s it do?

1
2

"This is a test".bitruncate(:length => 6) ==> "Thi...est"
"This is a test".bitruncate(:elength => 6) ==> "...a test"

The default options are { :length => 30 } which will produce 15 characters from the front and 15 from the end, putting … marks in the middle.

For rails, I put this in my lib/core_extensions.rb file.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74

#
# Copyright (c) 2009 Michael Graff.  All rights reserved.
#
# Redistribution and use in source and binary forms, with or
# without modification, are permitted provided that the following
# conditions are met:
# 1. Redistributions of source code must retain the above copyright
#    notice, this list of conditions and the following disclaimer.
# 2. Redistributions in binary form must reproduce the above
#    copyright notice, this list of conditions and the following
#    disclaimer in the documentation and/or other materials provided
#    with the distribution.
# 3. The name of Michael Graff may not be used to endorse or promote
#    products derived from this software without specific prior
#    written permission.
#
# THIS SOFTWARE IS PROVIDED BY Michael Graff ``AS IS'' AND ANY
# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
# PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL Micahel Graff
# BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
# TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY
# OF SUCH DAMAGE.
#

class String
  #
  # Truncate from both ends of a string.  The :length parameter, which defaults
  # to 30, will return the first 15 and the last 15 characters from a string
  # if it is longer than 30 characters.  If it is shorter, the entire string
  # is returned.
  #
  # Another way to specify the front and back portions are with :flength and
  # :elength.  If you specify one of these but not the other then you will
  # not get the missing part.  e.g., :flength => 10 alone will return only
  # the first 10 charcters of the string.  This is the same as the standard
  # truncate(s, :length => 10) helper.
  #
  # If a :length parameter is provided it will override any other lengths
  # specified.
  #
  def bitruncate(options = {})
    maxlength = options[:length] || 0
    flength = options[:flength] || 0
    elength = options[:elength] || 0
    omission = options[:omission] || '...'

    if maxlength == 0 && flength == 0 && elength == 0
      maxlength = 30
    end
    if maxlength != 0
      flength = maxlength / 2
      elength = maxlength / 2
    end

    return self if length <= (flength + elength)

    front = ''
    back = ''
    if flength > 0
      front = self[0..(flength - 1)]
    end
    if elength > 0
      back = self[(length - elength)..(length)]
    end

    front + omission + back
  end
end

Filed under: Programming | Tagged with:   | 0 comments

Checking Credit Card Numbers in Ruby

Posted on March 24, 2008 by Michael

This is not meant to be an exhaustive list of all possible numbers, nor the only or best method to verify that they pass the “checksum” test, but here’s what I came up with.

I wrote this mostly to link a Ruby version of the code to Wikipedia’s article on Luhn checksum validation, since nearly every other language in use was listed, but Ruby was sadly missing.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101

#!/usr/bin/env ruby

#
# Copyright (c) 2008 Michael Graff.  All rights reserved.
#
# Redistribution and use in source and binary forms, with or
# without modification, are permitted provided that the following
# conditions are met:
# 1. Redistributions of source code must retain the above copyright
#    notice, this list of conditions and the following disclaimer.
# 2. Redistributions in binary form must reproduce the above
#    copyright notice, this list of conditions and the following
#    disclaimer in the documentation and/or other materials provided
#    with the distribution.
# 3. The name of Michael Graff may not be used to endorse or promote
#    products derived from this software without specific prior
#    written permission.
#
# THIS SOFTWARE IS PROVIDED BY Michael Graff ``AS IS'' AND ANY
# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
# PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL Micahel Graff
# BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
# TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY
# OF SUCH DAMAGE.
#


class Luhn
  public
  def self.check_luhn(s)
    s.gsub!(/[^0-9]/, "")
    ss = s.reverse.split(//)

    alternate = false
    total = 0
    ss.each do |c|
      if alternate
        total += double_it(c.to_i)
      else
        total += c.to_i
      end
      alternate = !alternate
    end
    (total % 10) == 0
  end

  private
  def self.double_it(i)
    i = i * 2
    if i > 9
      i = i % 10 + 1
    end
    i
  end

end

if $0 == __FILE__
  def test_valid(s)
    result = Luhn::check_luhn(s)
    if result
      puts "VALID: #{s}"
    else
      puts "INVALID: #{s} (should be valid)"
    end
  end

  test_valid('5105 1051 0510 5100') # Mastercard
  test_valid('5555 5555 5555 4444') # Mastercard

  test_valid('4222 2222 2222 2')    # Visa
  test_valid('4111 1111 1111 1111') # Visa
  test_valid('4012 8888 8888 1881') # Visa

  test_valid('3782 8224 6310 005')  # American Express
  test_valid('3714 4963 5398 431')  # American Express
  test_valid('3787 3449 3671 000')  # American Express Corporate
  test_valid('3782 8224 6310 005')  # Amex
  test_valid('3400 0000 0000 009')  # Amex
  test_valid('3700 0000 0000 002')  # Amex

  test_valid('38520000023237')      # Diners Club (14 digits)
  test_valid('30569309025904')      # Diners Club (14 digits)

  test_valid('6011111111111117')    # Discover (16 digits)
  test_valid('6011 0000 0000 0004') # Discover
  test_valid('6011 0000 0000 0012') # Discover
  test_valid('6011000990139424')    # Discover (16 digits)
  test_valid('6011601160116611')    # Discover (16 digits)

  test_valid('3530111333300000')    # JCB (16 digits)
  test_valid('3566002020360505')    # JCB (16 digits)

  test_valid('5431111111111111')    # Mastercard (16 digits)
end

Filed under: Programming | Tagged with:   | 0 comments

RailsMode (not ENV['RAILS_ENV'])

Posted on October 27, 2007 by Michael

RailsMode

I’ve been spreading things like this all around my code, where I wanted to do something differently in production vs. development mode. Previously, I’d write something like this:

1
2
3

if ENV['RAILS_ENV'] == 'production'
  ... perform magic ...
end

While this is pretty simple, it just didn’t feel very DRY. So, I decided to use this as a reason to learn about modules and mixins.

I have a generic plug-in in vendor/plugins that I put small bits of code like this. You might as well, but if you don’t, you can drop this in a helper.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

module RailsMode
  def railsmode(*list)
    list.map! do |item|
      item.to_s
    end

    if block_given?
      if list.include?(ENV['RAILS_ENV'])
        yield
      end
    else
      return list.include?(ENV['RAILS_ENV'])
    end
  end
end

I also put this line in my environment.rb file:



include RailsMode

This mixes the module into the current class. Doing this in environment.rb makes it available everywhere in rails. Putting that line in a specific file would also work, such as a controller, or a helper.

With this, I can now write:

1
2
3

if railsmode(:production)
  ... perform magic ...
end

I can also check for multiple modes at once:

1
2
3

if railsmode(:production, :development)
  ... perform magic ...
end

And of course, who needs an if when I can pass in a block:

1
2
3

railsmode(:production) do
  ... perform magic ...
end

Filed under: Programming | Tagged with:   | 1 comment