CodeSOD: Lines and Lines and Lines of Order Lines

Darlene's company uses Siebel for managing their enterprise. Like most enterprise software packages, it's complicated, incomprehensible, and any significant maintenance depends on very expensive consultants.

During an upgrade, one of those Highly Paid Consultants caught a new requirement: customers wanted to be able to change an order, replacing one product code with another, all the way up until the order went into fulfillment. Now, the logical thing would have been to cancel the changed order line and create a new one, but our HPC couldn't quite figure out how to cancel an individual line item, so he just decided to delete it instead.

This is the eScript (Siebel's proprietary version of JavaScript) code he implemented:

var orderLineItemsBO = TheApplication().GetBusObject("Order Entry - Line Items");var orderLineItemsBC = orderLineItemsBO.GetBusComp("Order Entry - Line Items");orderLineItemsBC.ClearToQuery();orderLineItemsBC.SetSearchSpec("Order Id", myOrderId);orderLineItemsBC.SetSearchSpec("Product", "Garbage Product"); //"Garbage Product" is an example name of what a product code might be.orderLineItemsBC.ExecuteQuery(ForwardOnly);var record = orderLineItemsBC.FirstRecord();while(record){orderLineItemsBC.DeleteRecord();record = orderLineItemsBC.FirstRecord();}

Now, this code doesn't look terrible, aside from the lack of indenting, but immediately after this went into test, the QA team started complaining about performance. "Editing an order takes an extremely long time." There were other, bigger requirements, editing an order wasn't a commonly used feature, and eventually the project managers just said, "It's good enough," and shipped it into production.

And that's when all hell broke loose. People would place orders for product code, say, PQ1236, but before their order shipped, that line would be removed from their order without them doing anything.

The culprit was this specific line: orderLineItemsBC.SetSearchSpec("Order Id", myOrderId);.

Now, you might not know what's wrong with this, but you're not a highly paid Siebel consultant. The issue is that myOrderId isn't validated anywhere in the function, and if a null gets passed in, Siebel decides that you must want a wildcard search, and returns every order in the system.

That's why performance was terrible, and it also highlighted a failure in their testing methodology: no one ever looked at old orders during the test process, otherwise they might have noticed the issue.

[Advertisement] Universal Package Manager - ProGet easily integrates with your favorite Continuous Integration and Build Tools, acting as the central hub to all your essential components. Learn more today!