PHP Laravel function to search for ads in MySQL

Question

I am new to the Laravel framework. I have recently written a function which gets called through a route. The function is created for two purposes:

1. Get ads from a particular city based on provided filter parameters with pagination
2. Search ads if search term is available in request parameters.

---

public function getAdsFeedWithFilters(Request $request) {
 $message = 'success';
 $code = ResponseCode::$SUCCESS;
 $cityId = $request->input('city_id');
 $postedWithin = $request->input("posted_within");
 $priceStartRange = $request->input('price_start_range');
 $priceEndRange = $request->input('price_end_range');
 $sortBy = $request->input('sort_by');
 $searchTerm = $request->input('search_term');
 $page = $request->input('page');
 $checkForPriceRange = false;
 if ($request->has(['price_start_range', 'price_end_range'])) {
 $checkForPriceRange = true;
 }
 $checkForPostedWithin = false;
 if ($request->has('posted_within')) {
 $checkForPostedWithin = true;
 }
 $sortByColumn = 'creation_date';
 $sortOrder = 'DESC';
 if ($sortBy == self::$SORT_BY_PRICE_HIGH_TO_LOW) {
 $sortByColumn = 'price';
 $sortOrder = 'DESC';
 } elseif ($sortBy == self::$SORT_BY_PRICE_LOW_TO_HIGH) {
 $sortByColumn = 'price';
 $sortOrder = 'ASC';
 } elseif ($sortBy == self::$SORT_BY_DATE_CREATED_NEW_TO_OLD) {
 $sortByColumn = 'creation_date';
 $sortOrder = 'DESC';
 } elseif ($sortBy == self::$SORT_BY_DATE_CREATED_OLD_TO_NEW) {
 $sortByColumn = 'creation_date';
 $sortOrder = 'ASC';
 }
 $termsRS = '';
 $checkForSearchTerm = false;
 if ($request->has('search_term')) {
 $terms = explode(" ", trim($searchTerm));
 $terms = MyUtils::removeIgnoredWords($terms);
 if (count($terms) != 0) {
 $checkForSearchTerm = true;
 $termsRS = implode(" ", $terms);
 }
 }
 $city = City::find($cityId);
 if (!$city) {
 $message = 'Invalid city id';
 $code = ResponseCode::$FAILURE;
 return ResponseFormatter::_format($code, $message);
 }
 $ads = $city->load(['ads' => function($query) use (
 $termsRS,
 $checkForSearchTerm,
 $checkForPostedWithin, $checkForPriceRange,
 $sortByColumn, $sortOrder, $postedWithin, $priceEndRange, $priceStartRange) {
 $query->where('status', '=', Ad::$STATUS_ACTIVE);
 if ($checkForPriceRange) {
 $query->where('price', '>=', $priceStartRange);
 $query->where('price', '<=', $priceEndRange);
 }
 if ($checkForPostedWithin) {
 $query->whereRaw('creation_date >= NOW() - INTERVAL ' . $postedWithin . ' DAY');
 }
 if ($checkForSearchTerm) {
 $query->whereRaw("MATCH (title) AGAINST (?)", [$termsRS]);
 }
 $query->orderBy($sortByColumn, $sortOrder);
 $query->paginate(Ad::$ADS_PER_PAGE);
 }]);
 $outputAds = array();
 foreach ($ads->ads as $ad) {
 $ad->details();
 $outputAds[] = $ad;
 }
 return ResponseFormatter::format($code, $message, $outputAds);
}

Ad Model

class Ad extends Model {
 public static $ADS_PER_PAGE = 5;
 public static $STATUS_ACTIVE = 1;
 protected $table = 'ad';
 public $timestamps = false;
 public function user() {
 return $this->belongsTo('Sello\User');
 }
 public function adImages() {
 return $this->hasMany('Sello\AdImage');
 }
 public function brand() {
 return $this->belongsTo('Sello\Brand');
 }
 public function category() {
 return $this->belongsTo('Sello\CategoryType');
 }
 public function details() {
 $this->adImages;
 $this->brand;
 $this->category;
 $this->user->city;
 }
}

I have created a FULLTEXT index on the title field of the Ad table so that the keyword search could be fast. There are only 100 records in the Ad table. For now, the method is working according to expectations but I want to optimize it so that in the future when the count of records goes above thousands or hundreds of thousands, the performance of this method remains good. Can any Laravel or SQL expert suggest to me some tips or code changes or query changes which can enhance the query performance even to the minute level?

---

Table - Ad enter image description here

Table - ad_images

enter image description here

Table - brand

enter image description here

Table - category

enter image description here

Table - city

enter image description here

Table - country

enter image description here

Table - user

enter image description here

• bumperbox: The performance of this will largely be based on your database table structure and indexes. Can you add that info to your question? Also if you are desperate for a performance increase don't use eloquent or query builder, just write a raw sql.
• Khawar Raza: Table structure is added.
• Conor Mancone: In case I don't have time to come back for a full code-review, I wanted to point out the very critical fact that (I believe) you have an SQL injection vulnerability for your 'postedWithin' query. You are passing unfiltered user input into your rawWhere() call.

Answer 1

It is unfortunate that this post has existed more than three years without a review. In that time Laravel has had multiple major version releases. You may have learned a few things about it and perhaps your code is different. However even if the information below is not helpful to you then perhaps it may help others.

When this code was posted the latest version of Laravel was 5.5 and the current version is 8.0. The links below will likely point to the latest documentation though one can likely switch back the the version used by their code.

Eager loading

The biggest thing that could simplify the code and reduce the number of queries performed is eager loading of relationships. It appears that the properties of the ad model are lazy loaded via the details() method. Eager loading should mean that method can be eliminated and $ads can be returned instead of needing to create $outputAds

SQL injection

As was mentioned in a comment, the query passes unsanitized input from the request in the raw SQL - e.g. $query->whereRaw('creation_date >= NOW() - INTERVAL ' . $postedWithin . ' DAY'); . It is wise to use bound parameters to avoid the possibility of injection attacks. The setBindings() method can be used to add parameters to a query - see this answer for an example.

Use Constants instead of static variables

I noticed there are static variables in all capitals that are referenced and used as if they are constants - e.g. $query->paginate(Ad::$ADS_PER_PAGE);, ResponseCode::$SUCCESS. While you may be the only one writing the code it may be the case that somebody else would be maintaining it. Know that those variables could be modified at run-time. If the intention is for those to be constants then declare them as constants with the const keyword

class Ad extends Model {
 public const int ADS_PER_PAGE = 5;
 public const int STATUS_ACTIVE = 1; 

_{Note: this example uses property type declarations, available with PHP 7.4+}

Then the value can not be re-assigned.