WordPress.org

1. tabish
   Member
   Posted 10 months ago #

   Hi all,

   I was really happy when I downloaded this plugin and activated on my site. But my happiness couldn't last for long.

   As soon as you grow your categories, you will be able to see this plugin is strong enough to KILL your server. For example.. if you have 500 download categories then this plgin will run 800+ SQL queries to fetch even ONE download result on a single post page. If you are a bit familiar with the programming and server resources, then you can assume the BLUNDER.

   When I saw this this.. I was shocked and then I realized that we should think twice before activating any plugin like this.

   Anyway.. I spent few time with this plugin and customized it completely and now it is working the way I wanted.

   My advice would be.. DO NOT USE it.. tell the author to modify the coding ASAP.

   Regards

   http://wordpress.org/extend/plugins/download-monitor/

2. jolley_small
   Member
   Posted 10 months ago #

   Thats really clever. Rather than tell me, or contribute, you rant, thus reducing the user base, thus putting me off updating in the future. Genius.

3. tabish
   Member
   Posted 10 months ago #

   jolley_small

   No, I am not doing any clever trick here. I just warned about what I faced.

   As far as putting updates here is concerned, I have updated plugin for my use and i discarded so many things which i didn't need. So putting that update here will confuse people.

   If you want, you can tell the author to see the issues which I have pointed and then he can update it accordingly.

   This is a very serious issue and I am not sure why anyone didn't notice it so far.

   Just try to make 600 categories in it and then you will know what i mean. 600 data is nothing as far as PHP parsing and mysql load is concerned. But for this plugin 600 is like putting mote than 5 million data in the database. It will try to retrieve those categories in a way like it is trying to retrieve 5 millions entries from database.

   A BIG NO NO for this plugin.

   Regards
   Tabish

4. tabish
   Member
   Posted 10 months ago #

   Sorry jolley_small I didn't know you are the author.

   you can mail me so that i can tell you the issues.

   Regards

5. jolley_small
   Member
   Posted 10 months ago #

   Ill be looking into the issue shortly - I should be able to find it.

   Thanks

6. tabish
   Member
   Posted 10 months ago #

   Jolley_small

   Can you tell me what you are trying to achive here?

   // select all cats
   $query_select = sprintf("SELECT * FROM %s ORDER BY id;",
   $wpdb->escape($wp_dlm_db_cats));

   $cats = $wpdb->get_results($query_select);

   if (!empty($cats)) {

   $patts = array();
   $subs = array();

   foreach($cats as $c) {

   // Get downloads for cat and put in ul
   $links = '

   ';
   // Get list of cats and sub cats
   $the_cats = array();
   $the_cats[] = $c->id;
   $query = sprintf("SELECT id from %s WHERE parent IN (%s);",
   $wpdb->escape( $wp_dlm_db_cats ),
   $wpdb->escape( implode(",",$the_cats) ));
   $res = $wpdb->get_results($query);
   $b=sizeof($the_cats);
   if ($res) {
   foreach($res as $r) {
   if (!in_array($r->id,$the_cats)) $the_cats[]=$r->id;
   }
   }
   $a=sizeof($the_cats);
   while ($b!=$a) {
   $query = sprintf("SELECT id from %s WHERE parent IN (%s);",
   $wpdb->escape( $wp_dlm_db_cats ),
   $wpdb->escape( implode(",",$the_cats) ));
   $res = $wpdb->get_results($query);
   $b=sizeof($the_cats);
   if ($res) {
   foreach($res as $r) {
   if (!in_array($r->id,$the_cats)) $the_cats[]=$r->id;
   }
   }
   $a=sizeof($the_cats);
   }

   This you can get in wp_dlm_parse_downloads_cats() in wp_download_monitor.php file.

   What are you trying to archive in this loop dude?

   Regards
   Tabish

7. jolley_small
   Member
   Posted 10 months ago #

   Its a bad loop getting sub cats. I've improved efficiently in latest build.

8. tabish
   Member
   Posted 10 months ago #

   Good thing..

   But still it's as bas as it was before.

   Why are you using

   $query_select = "SELECT * FROM ".$wpdb->escape($wp_dlm_db_cats)." ORDER BY id;"; in wp_dlm_parse_downloads_cats() .. so again in

   foreach($cats as $c) {
   // Get downloads for cat and put in ul IF WE FIND IT IN THE DATA
   if ( strstr($data, "[download_cat#".$c->id."]" ) ) {

   That means again.. we have 800 categories.. it will run it 800 times..

   Why you are doing so.. i dont understand..

   My simple input for you would be:

   <?
   function wp_dlm_parse_downloads_cats($data)
   {
   if (substr_count($data,"[download_cat#")) {
   preg_match_all("|\[download_cat#(.*)\]|U",$data,$result,PREG_SET_ORDER);
   foreach ($result as $val)
   	{
   $query ="SELECT * FROM wp_DLM_DOWNLOADS WHERE category_id=$val[1] ORDER BY 'title'";
   		$downloads = $wpdb->get_results($query);
   // now here you can convert the keyowrds into links.. or whatever you want to do here
   }
    }
   return $data;
   }
   ?>

   This things can achieved by only two or three lines.. it can be more compact.. but I will have to use more mind.. which i cant do right now. :)

   Regards

9. jolley_small
   Member
   Posted 10 months ago #

   Its because we cannot run code directly in the post - we have to get the category ID's and check whether or not it is present in each post as a tag.

10. tabish
    Member
    Posted 10 months ago #

    Yes.. i had an Idea what you are doing..

    Did you get my code? it's doing the same thing without selecting all the categories.

    Regards
    Tabish

11. razorcreed
    Member
    Posted 10 months ago #

    i agree with tabish to many queries this thing kinda sucks it needs simplified

12. razorcreed
    Member
    Posted 10 months ago #

    sorry but after futher testing this thing sux worse than previously thought before boooooooooooooooooooo

13. tabish
    Member
    Posted 10 months ago #

    Yes.. razorcreed

    I tried to make the author (jolley_small) understand but it looks like he is being hurt if someone points his err. You can see his first reply to me.. it was real Rude.

    and now when I am giving him the solution, he looks like no more interested in any suggestions.

    I wish if Wordpress has some sort of "Check" for these kind of plugins.. which can easily kill your site and server.

    The worst thing is.. i have pointed only about one function.. even when you go to the Write Post page, download monitor will rum 800 SQL queries if you have more than 400 categories in it..

    So whole plugin needs to be re-written.

14. vektor
    Member
    Posted 10 months ago #

    Hi jolley_small/tabish,

    There's no denying that the concept for this plugin is a good one. Is there no way that you can sort you differences out and come up with a bullet proof solution? It wouldn't take too much surely?

    easy

    vektor.

15. jolley_small
    Member
    Posted 10 months ago #

    "I tried to make the author (jolley_small) understand but it looks like he is being hurt if someone points his err. You can see his first reply to me.. it was real Rude."

    FYI the topic title was discouraging anyone using the plugin, which I found rude considering the effort that went into it.

    May I remind everyone this is offered for free off of my own back so why complain and say it sucks?

    @tabish I appreciate your input, I'm trying to make it all work. The latest update was a temp improvement but obviously will need more work.
    Categories were thrown in at a later stage - thats why they are a bit messed up.

16. jolley_small
    Member
    Posted 10 months ago #

    Hi Tabish,

    If this works out well Ill mod all the queries in a similar fashion. Heres what I have come up with. I had to modify the query since we need to get the category and its sub-categories by traversing through.

    function wp_dlm_parse_downloads_cats($data) {
    	if (substr_count($data,"[download_cat#")) {
    
    		global $wpdb,$wp_dlm_root,$allowed_extentions,$max_upload_size,$wp_dlm_db,$wp_dlm_db_cats;
    
    		$patts = array();
    		$subs = array();
    
    		$url = get_option('wp_dlm_url');
    		$downloadurl = get_bloginfo('wpurl').'/'.$url;
    		if (empty($url)) $downloadurl = $wp_dlm_root.'download.php?id=';
    		$downloadtype = get_option('wp_dlm_type');
    
    		// Get cats and dig for sub cats
    		function get_sub_cats($the_cats) {
    			global $wpdb, $wp_dlm_db_cats;
    			$a=sizeof($the_cats);
    			$query = sprintf("SELECT id from %s WHERE parent IN (%s);",
    				$wpdb->escape( $wp_dlm_db_cats ),
    				$wpdb->escape( implode(",",$the_cats) ));
    			$res = $wpdb->get_results($query);
    			if ($res) {
    				foreach($res as $r) {
    					if (!in_array($r->id, $the_cats)) $the_cats[]=$r->id;
    				}
    			}
    			$b=sizeof($the_cats);
    			if ($a!=$b) {
    				$sub_cats = get_sub_cats($the_cats);
    				$the_cats = $the_cats + $sub_cats;
    			}
    			return $the_cats;
    		}
    
    		preg_match_all("|\[download_cat#(.*)\]|U",$data,$result,PREG_SET_ORDER);
    
    		foreach ($result as $val) {
    
    			// Traverse through categories to get sub-cats
    			$the_cats = array();
    			$the_cats[] = $val[1];
    			$the_cats = get_sub_cats($the_cats);
    
    			// Get downloads for category and sub-categories
    			$query ="SELECT * FROM $wp_dlm_db WHERE category_id IN (".implode(',',$the_cats).") ORDER BY 'title'";
    			$downloads = $wpdb->get_results($query);
    
    			// GENERATE LIST
    			$links = '<ul>';
    			if (!empty($downloads)) {
    				foreach($downloads as $d) {
    					switch ($downloadtype) {
    						case ("Title") :
    								$downloadlink = urlencode($d->title);
    						break;
    						case ("Filename") :
    								$downloadlink = $d->filename;
    								$link = explode("/",$downloadlink);
    								$downloadlink = end($link);
    						break;
    						default :
    								$downloadlink = $d->id;
    						break;
    					}
    					if (!empty($d->dlversion))
    						$links.= '<li><a href="'.$downloadurl.$downloadlink.'" title="'.__("Version","wp-download_monitor").' '.$d->dlversion.' '.__("downloaded","wp-download_monitor").' '.$d->hits.' '.__("times","wp-download_monitor").'" >'.$d->title.' ('.$d->hits.')</a></li>';
    					else $links.= '<li><a href="'.$downloadurl.$downloadlink.'" title="'.__("Downloaded","wp-download_monitor").' '.$d->hits.' '.__("times","wp-download_monitor").'" >'.$d->title.' ('.$d->hits.')</a></li>';									
    
    				}
    			} else {
    				$links .= '<li>No Downloads Found</li>';
    			}
    			$links .= '</ul>';
    			$patts[] = "[download_cat#" . $val[1] . "]";
    			$subs[] = $links;
    		}
    		$data = str_replace($patts, $subs, $data);
    	}
    	return $data;
    }

17. jolley_small
    Member
    Posted 10 months ago #

    Seems ok to me.

18. mosey
    Member
    Posted 10 months ago #

    I felt compelled to login when I read this post and put my thoughts into writing:

    I like this plugin. I appreciate the plugin author (Mike Jolley) for his hard work and time and dedication in creating something like this available for everyone to use, so thank you!

    Just to be clear I am not a coder and therefore from my perspective I cannot possibly analyse the efficiency of the code that powers this clearly very powerful plugin.

    It is great that Tabish has taken the time to detail his/her findings and hopefully help to improve the plugin in the long run, especially when he/she is clearly talking about hundreds of categories (I have personally never found the need to use more than 5 if at all) but just reading the title of the post, I can't help but feel that it is not the most appropriate description of the issue that has been identified!

    Most users probably don't extend the plugin as much as Tabish has done, so would never encounter this 'too many queries' issue. I'm sure a plugin author is almost always pleased that someone is pushing the boundaries, but
    

    "Think twice before using this plugin (WARNING)!!!"

    sounds more like scaremongering, akin to tabloid headlines, and it might be LESS confusing and LESS misleading to say specifically - in the title - something like
    

    'Too many SQL queries occur when setting up hundreds of categories'.

    Apologies if this is not the most appropriate title, and I hope I am being clear about what I am trying to say here. Ultimately, like vektor has said, I would love it if a solution could be worked out between both Mike Jolley and Tabish as I believe both will/have benefit, and non-power users (aka myself) will - unknowingly - also benefit, with thanks to developers like them, from a great plugin.

19. erigami
    Member
    Posted 10 months ago #

    Plugin authors put in hundreds of unpaid hours into their code. If you find a problem in a plugin, don't post "DO NOT USE it" publicly. As jolley_small states, this annoys the author, makes users less likely to install the plugin, and thereby makes the author less interested in working on the plugin.

    Just state what the problem is, and try to get the problem solved.

    We're operating in a gift economy here. Civility and friendliness go a long way.

20. tabish
    Member
    Posted 10 months ago #

    Ok.. No more arguments.. lets resolve the issue togather.

    jolley_small can you tell me buddy why we need to parse Sub Cats in this function? May be i am not getting this point here.. so pls explain this to me.

    Because The way I have customized your plugin.. i didn't use the sub cat thing and it's working perfectly right.

    Let me know so that we can finish these changes together.

    I truly appreciate your work.. what you have done.. is remarkable and I tried to see how to edit the title of my post but I couldn't.. so sorry for that.. may be i should have used a little more polite language.. my fault dude

    Regards

21. tdiaz
    Member
    Posted 10 months ago #

    800 freaking hundred categories? Holy downloads Batman!

    Just tell me.. should I use this if I'm expecting at most 20 categories sometime well in the future?

    Actually, I'm looking for a download manager that allows the category names only to be dynamically populated in the sidebar - because the Lester Chan one does not and a not that recent post says he's not gonna do it.

22. tabish
    Member
    Posted 10 months ago #

    yes Tdiaz .. you can use it for your 20 categories. :)

23. jolley_small
    Member
    Posted 10 months ago #

    @thanks mosey and erigami.

    @tabish - We check for sub cats because we output the downloads when showing a parent (the code I sent allowed this and used your method as much as possible).

    Example:

    A download has 1 category defined.

    Each category can have a parent. On the manage screen these are shown like this:

    2. Top Category - 3. Child - 4. Last child

    Lets say we had two in category 2 and one download in category 4. When we output category 2 we want to display its two downloads BUT ALSO grab the single download in the sub category, because technically its still in this category, just a sub cat.

    That's why we have to find child categories by querying the children.

    Make sense?

24. tabish
    Member
    Posted 10 months ago #

    Jolly,

    Technically displaying sub cats download in parent is not fine. Anyway.. Below is my coding which i customized for my site.. and the output of my code and yours are same.. even I dont use subcats select query..

    function wp_dlm_parse_downloads_cats($data)
    { // Our Function Starts
    global $table_prefix,$wpdb,$wp_dlm_root,$allowed_extentions,$max_upload_size,$wp_dlm_db,$wp_dlm_db_cats;
    if (substr_count($data,"[download_cat#")) {
    preg_match_all("|\[download_cat#(.*)\]|U",$data,$result,PREG_SET_ORDER);
    $url = get_option('wp_dlm_url');
    $downloadurl = get_bloginfo('wpurl').'/'.$url;
    if (empty($url)) $downloadurl = $wp_dlm_root.'download.php?id=';
    $count_result=count($result);
    $count_var=1;
    $ul_start="<!-- Starts Download UL --><ul id=\"downloadlinks\">";
    foreach ($result as $val)
    	{
    
    		$query ="SELECT * FROM wp_DLM_DOWNLOADS WHERE category_id=$val[1] ORDER BY 'title'";
    		$downloads = $wpdb->get_results($query);
    		//echo($query.'<br />');
    		if (!empty($downloads)) {
    		foreach($downloads as $d) {
    		$downloadlink = $d->id;
            //print_r($downloads);
    		$fileext=substr(strrchr(($d->filename), '.'), 1);
    		$links.= $ul_start.'<li><a href="'.$downloadurl.$downloadlink.'" title="'.__("Downloaded","wp-download_monitor").' '.$d->hits.' '.__("times","wp-download_monitor").'" >'.$d->title.'</a><BR /><span class="downloadedtimes">('.$d->hits.' downloads)</span> </li>';	
    
    		$ul_start='';
    		} // for each ends data one
    		if($count_result==$count_var){ $links.="</ul><!-- Ends Download UL -->"; }
    		$count_var++;
    		$data=str_replace($val[0], $links, $data);
    		$links='';
    		} // IF Ends
    	}
    
    } //IF ends.. download_cat matching
    	//echo($query);
    return $data;
    } // Our Functions Ends

    It's a complete code which i modified for my site. I am not using few of your default functions.. because I didnt need them.. (Like downloadtype case)

    Please run my code and make sub cats and all.. and then compare both codes.

    Let me know.
    Tabish

25. tabish
    Member
    Posted 10 months ago #

    As far as i could understood your Subcat logic.. you are saying that All the SubCat's download should show in Parent category?

    So that means if I have a category called:

    Pop Songs (20 download)
    then Sub cats of POP Songs:
    Madona
    Michel Jakcson
    Elvis
    and son on..

    So all these sub cats songs will be displayed in POP Songs category?

    IF this is so.. then it will be a big blunder for my site.. if i use sub cat function.

    Regards
    Tabish

26. jolley_small
    Member
    Posted 10 months ago #

    @tabish - blunder or not, thats what will happen, and what should happen.

    In your example, if you showed pop songs then it would make sense to show the subcats too because they are still pop songs... the sub cats just narrow down the results when displayed individually.

    So basically the code I posted is either the best it can be unless you have a better way of querying sub-categories...

27. jolley_small
    Member
    Posted 10 months ago #

    Ill change the patts/match bit in mine though to your $data=str_replace($val[0], $links, $data);

    Then thats about it.

28. jolley_small
    Member
    Posted 10 months ago #

    Maybe it will be better to find subcats differently. If i queried all cats at once with only 1 query and put them into a variable, this might be quicker. Ill experiment.

29. tabish
    Member
    Posted 10 months ago #

    Jolly

    I have a website, which allows users to download samples papers, and study materials.

    For one section, Our download monitor category structure is as follows:

    MBA -> School A -> School B -> School 3

    You can think about the mistake if it shows all the school's study material in MBA section while in MBA we have to put MBA fact sheets only and we dont want to display any school's material on that page.

    Anyway.. it's your plugin.. you do whatever you want. I simply tried to help you.

    Bye

30. jolley_small
    Member
    Posted 10 months ago #

    Well for that you would have to customise to your needs, but I doubt many would want the change.

    By the way, that structure is weird because your basically saying school b is within a - it looks like nesting categories was not needed.

    Ill post the fixes. Thank you for the assist.

Reply »

You must log in to post.