Back on Track!

After a nice spring break, and a good recharge it is time to get back on track. Our main project Drupal is buzzing and busy as always and took no rest, so I took a chance to review some of the bugs from our time line and see if anything has changed. Sure enough, one of the documentation bugs, Bug 1444650, was fixed by the community. They went ahead and tidied up the documentation. Then, Bug 1431512  was reclassified as a non-Drupal related issue and cannot be patched within Drupal itself.

A few lessons are learned from this experience. For one, I learned to be more assertive in tackling bugs. I should go ahead and be more forthright in assigning bugs to myself to prevent other users from patching before me. Also I must be more proactive in addressing bugs so that they are not fixed before I have a chance to look at them. Basically it is all a matter of time management. Bug fixing like all software engineering must be a well-timed and organized process. Without this focus in mind efficiency is sacrificed in both the project and project community.

Thankfully there was one more bug left of my three that had not been looked at yet. So for this post we will focus on this one. Bug 1438990 details a language error that occurs within Drupal. Essentially when a node contains page translations, if the translations are a subset of the entire sites languages then instead of using the default language for the page, the node will use the first language within the array subset. The bug reporter provides some good screen shots to demonstrate the problem.

Notice the array for the node; the language default is set to english:

https://i2.wp.com/drupal.org/files/screenshot11.png

Now notice that the language is not English but Bulgarian:

https://i0.wp.com/drupal.org/files/screenshot10_0.png

In my next post I will provide bug injection details and possible fixes (if time permits). Also I will be reviewing other possible bugs to add to my new revised list. Until then, keep on programming!

Advertisements

Challenges of the Software Age

This week, we got a chance to delve into some open software news sources. The task was to take two articles from opensource.com and post our personal response and reflect upon the articles argument or thesis. For my first article, I chose one about Louis C.K., the comedian. I am a lover of comedy, and Louis C.K. is one of my favorites. To be honest, I was little surprised to see an article about him even covered in a software magazine, but once I read the article I fully understood.

The article states its premise outright, “The answer to stabilizing content and price is letting artists retain greater control of their work.” This claim is not unheard of and is a sound premise. It is based off of Louis C.K. providing a download of his stand up special for five dollars a pop. This model was used instead of the production model to prove that ease of access will generate revenue, and that the current system of production is antiquated. Using this method, he spent approximately 250,000 dollars. Surprisingly, he generated 1,000,000 dollars in revenue. The author of the article is arguing that this electronic method provided better user-developer relations, and allowed Louis to make better quality comedy because he had control over the entire production.

This argument needs to be heard many times over, and not just in comedy. Software can benefit from this method of thinking as well. Over and over we hear of software losses due to theft through pirating software. However, as the author of the article is saying, the issue can be solved simply through ease of access and a reasonable pricing model. Another great example of this thought is in mobile application development. The rise of easy to afford, easy to install, and mobile apps demonstrates this key principle: price and production affect piracy. The current structure of the software world promotes attacking individuals for sharing files, and punishing paying users with inconvenient protection measures. In the end, removing this methodology helps customer relations by making the paying user feel less punished for choosing the right way.

Also mentioned in the article is price. Price for software can reach upwards of millions of dollars. How much of this cost is purely administrative? How much comes from unnecessary costs such as advertising and publishing? When you go to the store and look at the sixty dollar game, I can tell you a significant chunk of that price is going straight to the publisher, not the developer. By removing these middle men in the modern internet era, we can reduce the cost of software to a point where it is almost non-existent (open source anyone?). We can create better software by fostering a more direct relationship between the end user and the developer. We can create better software for a better price by being in greater control of our development process. This point is what I construed from the article. Developers must always be agile in the field of fast-paced technology. So why not start adapting now?

The second article I chose was called “A cure of the common troll”. By troll, they are referring to patent trolling. With the rising boom in software comes new technology and innovations. These new technologies can all be patented in order to protect the developer’s copyright. Some companies will arise and have risen, whose sole purpose is to collect patents and then sue infringers. This is the art of the patent troll. The results of this trolling are adverse. For one, patent trolling restricts innovation by preventing smaller companies from developing without being sued out of existence. Another notable reason is that many of these patents have been bought, sold, and traded. These patents are not the true inventors but people who buy or acquire these patents from the inventors for profit. By choosing to do so, they are going against the whole concept of a patent to begin with: to protect the inventor. Lastly, many of the patents are dealt with in an archaic manner. A common analogy is that it is like patenting the door knob or the wheel. These are basic universal components that just cannot be patented because they are so basic and necessary to software development.

The article suggests a way to deal with these trolls of the modern age:

“First, create a compulsory licensing mechanism for patents whose owners are not making competitive use of the technology in those patents. Patent owners should be required to declare the areas or products that incorporate the patented technology. All other non-practiced areas should be subject to a compulsory license fee. (A non-practiced “area” would be a market or technology sector or activity in which the patent owner is not using or licensing the invention rights, though the owner may be using the patent in other “areas.”) Licensing rates for patents could be set by patent classification or sub-classification based on industry average licensing rates for each such technology. Again, this would only apply to applications where the patent is not being practiced or voluntarily licensed by the patent owner.
Given the vast number of patents issued, an accused party should have a reasonable, set time after receiving notice of a patent within which to pay for the license going forward. Compulsory licenses are authorized by the treaties we have entered into, and we have significant experience with compulsory licensing of copyrighted works from which to develop an analogous patent mechanism. Uniform rates could be set.
Second, cap past damages for trolls at $1 million per patent and eliminate the possibility of obtaining injunctive relief for infringement of patents that are not in use, or are not used commercially, by the patent owner.
Third, a mandatory fee shifting provision should be put in place where the plaintiff is required to pay the defendant’s reasonable defense fees if the plaintiff does not obtain a better recovery than what was offered by the defendant. (Presently, there is such a cost shifting mechanism in place; however, the relevant costs typically are a tiny fraction of the legal fees in a case.)
Fourth, for U.S. domestic defendants, require that suits be brought in the venue where the defendant’s primary place of business is located.
Fifth, if a party wants more than limited discovery from the opposing side, particularly for electronically stored information (ESI), the requesting party should pay the cost of production. For large technology companies, ESI production alone can cost into the seven figures.”

I am a big supporter of all these concepts. I would also add to the list, that patents cannot be bought or sold, only inherited or renounced (made open to all). By doing so, patent companies would be insolvent and inviable. Each of the other suggestions from the author are great ideas and should be considered in updating our current system of patent application and distribution.

These two articles discussed some hot button issues in not just open source development, but also in all forms of software development. I particularly enjoyed this assignment and found the articles to be both informative and interesting. I look forward to reading more!

Exercises in Ronald McDonald House source code

To follow up the unit tests from earlier this week. We have been assigned two more exercises from our software development book. The questions are as follows:

5.7.a) Locate the module in RMH Homebase that displays a shift’s “notes” field for editing.

     b)Locate instances of ugly code within the module,

The calendarFam.php displays a shifts notes for editing. Ugly code within this module is similar to the code in calendar.php such as

if ($edit==true && !($days[6]->get_year()<$year || ($days[6]->get_year()==$year && $days[6]->get_day_of_year()<$day) ) && $_SESSION[‘access_level’]>=2)
echo “<p align=\”center\”><input type=\”submit\” value=\”Save changes to all notes\” name=\”submit\”>”;

 

c) Define a new function called predate(a,b) that performs the same computation and returns true or false if a predates b, respectively. Insert that function as a new feature of the shift class.

predate($a,$b)

return($a <= $b)

d) Replace each instance of ugly code with new function.

if ($edit==true && predate($days[6]->get_year(), $year ) && predate($days[6]->get_day_of_year(), $day) && $_SESSION[‘access_level’]>=2)

e) Test refactoring.

All tests were successful. The refactoring worked perfectly. The bug shown in figure 5.15 was also fixed with this refactored code.

 

Team Timeline

It has been one crazy week! I finally got a chance to sit down and update everyone on our team project. We have created a timeline to show our planned bug completions for Drupal. Here is a report detailing our current plans.

Timeline


Ideally, we would like to resolve all of the bugs listed below by the end of the semester, but the following gantt chart is a more realistic idea, yet still a challenge.

drupalGantt.png

 

Normal Priority Bugs


Bug 1057592 – Image Field with a default image shows default image even if an image is uploaded (Details here)

Bug 1085472 – Too long comments do not get formatted to fit the width of the screen (Details here)

Bug 1137280 – Image style saves name on add new effect, results in page not found (Details here)

Bug 1144372 – Pictures do not shift when the picture in the middle is removed. The last picture gets duplicated instead. (Details here)

Bug 1431512 – Language switcher links to default 404 page (Details here)

Bug 1439000 – Clearfix “content” value causes gap at page bottom (Details here)

Bug 1438990 – Language detection mistake (Details here)

Bug 1444650 – Make the path argument documentation more clear in theme_image_style for D7 (Details here)

Major or Critical Bugs


Bug 1423158 – Too many failed login attempts user login error/block even after following password reset instructions (Details here)

Feature Requests


Bug 835202 – Checkbox to allow all Class names (Details here)

Bug 1445858 – Highlight Flagged Content (Details here)

 

So our team definitely has our work cut out for us, but with this plan in place we should be able to get a significant amount done before the final presentations. Updates to come soon!

Exercises in Unit Testing

Today’s exercise is based of of chapter five in our software development book. I have to look at three exercises at the end of the chapter. Based on what I have seen and read, these exercises will be applications in testing and documentation within a FOSS project. Let’s get started.

5.1 Examine the RMH Homebase release 1.5 code base and accompying documentation. Identify at least one instance of the following:

a. Long Method
b. Too Few Comments
c. Data Clumps

This method comes from personEdit.php. It is over 140 lines long. The purpose of the function is to refine all the forms entered. As you can see in the code below. The code is effecting over 14 variables in this part of the code alone which could cause some hard to find side effects. There are few comments relating what each of these blocks effect, and the method tries to accomplish too much in this one function. There is so much going on that it could be its own php file. Also it appears every piece of data extraction is occurring in this function on the variable id. The constant use of id in so many different extractions leads to data clumping.

function process_form($id) {
//step one: sanitize data by replacing HTML entities and escaping the ‘ character
$first_name = trim(str_replace(‘\\\”,”,htmlentities(str_replace(‘&’,’and’,$_POST[‘first_name’]))));
$last_name = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘last_name’])));
$address = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘address’])));
$city = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘city’])));
$state = trim(htmlentities($_POST[‘state’]));
$zip = trim(htmlentities($_POST[‘zip’]));
$phone1 = trim(str_replace(‘ ‘,”,htmlentities($_POST[‘phone1’])));
$clean_phone1 = ereg_replace(“[^0-9]”, “”, $phone1);
$phone2 = trim(str_replace(‘ ‘,”,htmlentities($_POST[‘phone2’])));
$clean_phone2 = ereg_replace(“[^0-9]”, “”, $phone2);

$private_notes = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘private_notes’])));
$public_notes = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘public_notes’])));
$my_notes = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘my_notes’])));

$background_check = ”;
$shadow = ”;
$interview = ”;
if($_POST[‘background_check’]==’yes’) $background_check = ‘yes’;
if($_POST[‘interview’]==’yes’) $interview = ‘yes’;
if($_POST[‘shadow’]==’yes’) $shadow = ‘yes’;

$convictions = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘convictions’])));
$wherelived = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘wherelived’])));
$experience = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘experience’])));
$motivation = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘motivation’])));
$specialties = trim(str_replace(‘\\\”,’\”,htmlentities($_POST[‘specialties’])));
d. Speculative Generality

As for speculative generality, thew helpfooter.inc file clutters the code, and could have easily been made into a txt file for the user. Or better yet it could have been added to another method that deals with all footers, instead of being a repetitive include file.

5.2 For each of these “bad smells” refactor the code to reduce the size of the code base.

a. Long Method
b. Too Few Comments
c. Data Clumps

For these three I had to do a good chunk of work. I separated the process_form into different functions. This made the code more manageable and alleviated the Long Method and Data Clumps issues. This yielded four methods: process_address, process_notes, process_meeting, and process_background. Then I had to add comments to each explaining what they did and why they were there. This de-cluttered the code and turned a long method into a reasonable one.

d. Speculative Generality- I combined the helpfooter.inc with the footer.inc to remove one include file, and changed the reference to helpfooter.inc within the help.php file.

5.3 Perform unit tests

I ran unit tests on personEdit.php and help.inc that yielded all passes. Overall I am satisfied with the results of my refactoring though I will need to add in more test cases later to make sure I have checked absolutely everything necessary.

This weekend I and my team will be working on setting up a project schedule for the rest of the semester. See you all then!

Completion of first bug

Tomorrow our team’s first complete bug report is due. Our team met this past Saturday and looked at some bugs in Drupal, while I continued to complete the bug from our last team report. I managed to create a solid solution to the bug and made a team report to accompany our other solved bug. Here is a transcript of this report.

Bug 1425138
– node/[id]/Edit (with capital e) renders edit form in default theme (despite admin theme)

About the Bug:

Once we felt comfortable with the process of implementing a fix, we moved on to find a bug that we could fix ourselves and submit to the Drupal project. The following is the bug description:

“When I visit /node/42/edit the node edit form is displayed with the administration theme.
When I visit /node/42/Edit the node edit form is displayed with the default theme.
Happens also if you use eDiT or some other combination. Is also the case for /revisions, /translate etc.”

Solution:

After reviewing the Drupal API, we found that the admin paths are set higher in the class hierarchy. This method, as described by the api, matches the paths in Drupal using patterns (defined as $patterns). In the previous report the creation of these patterns was seen and the bug fix was attempted on that function. The problem is that this method was to clunky to use because one would have to define every single way to capitalize a class path, which creating hundreds of instances. How this function is supposed to be used is to just make a one array to be used in making a map in a higher function. Drupal makes this array of acceptable administrative class paths, then creates a map within the method below in order to avoid creating multiple arrays, saving time and space.

Code from /includes/path.inc, line 489
API source
/**

* Determine whether a path is in the administrative section of the site.

*
* By default, paths are considered to be non-administrative. If a path does not

* match any of the patterns in path_get_admin_paths(), or if it matches both
* administrative and non-administrative patterns, it is considered
* non-administrative.

*
* @param $path

* A Drupal path.

*
* @return

* TRUE if the path is administrative, FALSE otherwise.

*
* @see path_get_admin_paths()

* @see hook_admin_paths()
* @see hook_admin_paths_alter()

*/
function path_is_admin($path) {
$path_map = &drupal_static(FUNCTION);
if (!isset($path_map[‘admin’][$path])) {
$patterns = path_get_admin_paths();
$path_map[‘admin’][$path] = drupal_match_path($path, $patterns[‘admin’]);
$path_map[‘non_admin’][$path] = drupal_match_path($path, $patterns[‘non_admin’]);
}
return $path_map[‘admin’][$path] && !$path_map[‘non_admin’][$path];
}

To fix this bug all that has to be done is apply a strtolower function to the $path input variable. Here is a copy of the posted patch file to reflect the change:

— drupal/includes/path.inc 2012-02-01 17:03:14.000000000 -0500
+++ path.inc 2012-02-13 00:01:15.443036067 -0500
@@ -487,6 +487,7 @@

* @see hook_admin_paths_alter()

*/
function path_is_admin($path) {
+ $path = strtolower($path);
$path_map = &drupal_static(FUNCTION);
if (!isset($path_map[‘admin’][$path])) {
$patterns = path_get_admin_paths();

Results:
The bug was successfully fixed and tested on our team’s basic test cases.

Conclusion

The second patch above has been submitted for peer review on the drupal site here and is now in the testing queue. These bug fixes brought some great experience to our team. It was a good chance to get familiar with the tools available to us. These tools included the community in the form of development mailing lists and MIRC. It also included community documentation such as the api references. These first bugs were also an opportunity to just play around with Drupal. We saw several of its features and how they operate and interact with each other in the code. With this better understanding of Drupal function our team is better at discerning bugs we can fix and how well we can fix them, and most importantly, how long it will take to make a clean fix.

Next week our team will produce a full schedule of planned bug fixes for the rest of the semester. See you all then!

Bug Exercises Part II: Patching

Today’s exercises involve one of the more basic skills that all programmers should know: patching. In the open source world patching has become pretty standardized, except for certain minutiae in formatting. So for the exercise my job was to create some test patches and understand the test process using the Unix terminal.

7.2.1

This exercise was creating a basic diff output using none other than the diff command. The output of this diff corresponded to the books results and is reprinted below.

steve@ubuntu:~$ diff -u hello.c hello.c.punct

— hello.c 2012-02-09 23:50:18.659577870 -0500

+++ hello.c.punct 2012-02-09 23:51:37.780125196 -0500

@@ -5,6 +5,6 @@

#include

int main() {

– printf(“Hello, World.\n”);

+ printf(“Hello, World!\n”);

return 0;

}

7.2.2
Then the next exercise was to examine the differences made by not having a -u. By not having the -u we see that the formatting of the output has changed. There is less information and is more bare representation.

steve@ubuntu:~$ diff hello.c hello.c.punct

8c8

printf(“Hello, World!\n”);

7.8
This exercise was the creation of patch file containing the word bar. First I created the file “foo” containing “bar” and used diff on it with the null file provided by Unix. The resulting output was created in a file named “foopatch.patch”. Redundant I know. Just did not want to lose the foo.c

— foo.txt 2012-02-09 23:58:58.439593808 -0500
+++ /dev/null 2012-02-09 23:48:23.021831997 -0500
@@ -1 +0,0 @@
-bar

7.9
Finally this exercise had me making a patch file using a real program. The program is called caultelis. I took the echo.c file and changed a small snippet of code to compare to echo.c.reverse. The following output was created, which matches the output the book says I will get.

— src/echo.c.reverse 2012-02-10 00:06:29.018333108 -0500
+++ src/echo.c 2012-02-10 00:10:10.938330675 -0500
@@ -258,14 +258,14 @@
}
else
{
– while (argc > 0)
+ while (argc > 0)
{
– fputs (argv[0], stdout);
argc–;
– argv++;
+ fputs (argv[argc], stdout);
if (argc > 0)
putchar (‘ ‘);
}
+
}

if (display_return)

These exercises were a great experience. Considering we have our first bug fix due Monday I was glad to get this practice in for making a patch to my team’s Drupal project. This weekend I will update the results of creating my first official patch for Drupal.