Crone task run only once.

Discussion forum for Extension Writers regarding Extension Development.
Post Reply
andreask
Registered User
Posts: 623
Joined: Fri Feb 27, 2009 6:13 pm
Name: Andreas

Crone task run only once.

Post by andreask »

I was finally lucky and my crone task is starting...
But there is always a but :)
So It starts it runs (fine) but it does not run again.
In cron status it goes red and never goes to ready to run again.
And if I want to run it again I have to reinstall it and clear the cache.

The cron checks are done properly, is_runnable and should_run on first run return true, and then when the interval passes ~30 minutes (1800 seconds) should_run again turns from false to true

The result of my run is also fine, meaning it does what it is suppose to do without errors (none that I have seen at least)

config/services.yml

Code: Select all

services:
    andreask.ium.cron.send_reminder:
        class: andreask\ium\cron\send_reminder
        arguments:
            - @config
            - @log
            - @user
            - @service_container
        calls:
            - [set_name, [cron.task.ium_send_reminder]]
        tags:
            - { name: cron.task }
    andreask.ium.classes.reminder:
        class: andreask\ium\classes\reminder
        arguments:
            - @config
            - @dbal.conn
            - @user_loader
            - @log
            - %core.table_prefix%
            - %core.root_path%
            - %core.php_ext%
cron/send_reminder.php

Code: Select all

<?php
/* Put your header comments here. */

namespace andreask\ium\cron;

use Symfony\Component\DependencyInjection\ContainerInterface;

class send_reminder extends \phpbb\cron\task\base
{
    protected $config;
    protected $user;
	protected $container;
    protected $log;

    /**
     * Constructor.
     *
     * @param \phpbb\config\config $config The config
     */

    public function __construct(\phpbb\config\config $config, \phpbb\log\log $log, \phpbb\user $user, ContainerInterface $container)
    {
        $this->config = $config;
        $this->user = $user;
        $this->container = $container;
        $this->log = $log;
    }

    /**
     * Runs this cron task.
     *
     * @return null
     */
    public function run()
    {
        // Do not forget to update the configuration variable for last run time.
		$this->log->add('admin', 54 , '127.0.0.1', 'In the cron run before sending reminders', time());

		$reminder = $this->container->get('andreask.ium.classes.reminder');
		$reminder->send((int) $this->config['andreask_ium_email_limit']);

		$this->config->set('send_reminder_last_gc', time());
		// for now add a line to logs.
		$this->log->add('admin', 54 , '127.0.0.1', 'Sent reminders', time());
    }

    /**
     * Returns whether this cron task should run now, because enough time
     * has passed since it was last run.
     *
     * @return bool
     */

    public function should_run()
    {
    	$run = (bool) ($this->config['send_reminder_last_gc'] < (time() - $this->config['send_reminder_gc']));
		$this->log->add('admin','54','127.0.0.1',"Do I need to run? (".$run. ") last_gc ".date("Y-m-d H:i:s",$this->config['send_reminder_last_gc'])." now :" .date("Y-m-d H:i:s",time())." - ". $this->config['send_reminder_gc'] ." Seconds", time());
        return (bool) $this->config['send_reminder_last_gc'] < strtotime('10 minutes ago');
    }

	/**
	 * @return bool enable cron task if
	 */

	public function is_runnable()
    {
    	$test = $this->config['andreask_ium_enable'];
    	$this->log->add('admin','54','127.0.0.1',"Can I run? : $test ", time());
        return (bool) $this->config['andreask_ium_enable'];
    }
}
classes/reminder.php

Code: Select all

<?php

namespace andreask\ium\classes;

class reminder
{

    /**
     *
     * This file is part of the phpBB Forum Software package.
     *
     * @copyright (c) phpBB Limited <https://www.phpbb.com>
     * @license GNU General Public License, version 2 (GPL-2.0)
     *
     * For full copyright and license information, please see
     * the docs/CREDITS.txt file.
     *
     */

    protected   $inactive_users = [];
	protected   $config;
	protected   $db;
	protected   $user;
	protected 	$log;
	protected   $table_prefix;
	protected   $phpbb_root_path;
	protected   $php_ext;
	protected	$table_name;

	public function __construct(\phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, \phpbb\user_loader $user,\phpbb\log\log $log ,$table_prefix, $phpbb_root_path, $php_ext)
	{
		$this->config = $config;
		$this->db = $db;
		$this->user = $user;
		$this->log	= $log;
		$this->table_prefix = $table_prefix;
		$this->php_ext = $php_ext;
		$this->phpbb_root_path = $phpbb_root_path;
		$this->table_name = 'ium_reminder';
	}

	/**
	 *
	 * Send email depending on the list of $inactive_users
	 *
	 */
    public function send($limit)
    {
		$this->log->add('admin', 54 , '127.0.0.1', 'Preparing to send reminders to users...', time());
		$this->get_users($limit);
    	if ($this->has_users())
	    {
			if (!class_exists('messenger'))
		    {
			    include($this->phpbb_root_path . 'includes/functions_messenger.' . $this->php_ext);
		    }

		    foreach ($this->inactive_users as $sleeper)
			{
				$this->log->add('admin', 54 , '127.0.0.1', '-------------- START ----------------------', time());
				$this->log->add('admin', 54 , '127.0.0.1', 'Sending to '. $sleeper['username'], time());

				// dirty fix for now, need to find a way for the templates.
				$lang = ($sleeper['user_lang'] == 'en') ? $sleeper['user_lang'] : 'en';

				// add template variablies
				$template_ary = array(
					'USERNAME'   	=> htmlspecialchars_decode($sleeper['username']),
					'REG_DATE'		=> date($sleeper['user_dateformat'], $sleeper['user_regdate']),
					'LAST_VISIT' 	=> date($sleeper['user_dateformat'], $sleeper['user_lastvisit']),
					'ADMIN_MAIL' 	=> $this->config['board_contact'],
					'SITE_NAME'  	=> htmlspecialchars_decode($this->config['sitename']),
					'SIGNATURE'	 	=> $this->config['board_email_sig'],
					'URL'        	=> generate_board_url(),
				);
				$messenger = new \messenger(false);
				$messenger->to($sleeper['user_email'], $sleeper['username']);
				$messenger->from($this->config['board_contact']);

				// Load template depending on the user

				if ($sleeper['user_lastvisit'] != 0)
				{
					// User never came back after registration...
					$this->log->add('admin', 54 , '127.0.0.1', 'User has visited us before' , time());
					$messenger->template('@andreask_ium/sleeper', $lang);
					$messenger->subject('We\'ve missed you!');
					$messenger->assign_vars($template_ary);
				}
				else
				{
					// User has forsaken us! :(
					$this->log->add('admin', 54 , '127.0.0.1', 'Users has never visited us before' , time());
					$messenger->template('@andreask_ium/inactive', $lang);
					$messenger->subject('Hello!');
					$messenger->assign_vars($template_ary);
				}

				// Send mail...
				$messenger->send();
//				$messenger->save_queue();
				// Update ext's table...
				$this->update_ium_reminder($sleeper);

				$this->log->add('admin', 54 , '127.0.0.1', '-------------- END ----------------------', time());
			}
		}
		// release the user list.
		unset($this->inactive_users);
    }

	/**
	 * @param null $limit optional parameter to limit the amount of results. (need to fix this)
	 */

	private function get_users($limit = null){

		// if limit is not set use limit from configuration.
		$limit = ($limit) ? 'limit '. $limit : 'limit '.$this->config['andreask_ium_email_limit'];
		$table_name = $this->table_prefix . $this->table_name;

		$this->log->add('admin', 54 , '127.0.0.1', 'Getting users...', time());

		// prepare the sql statement.
		$sql = 'SELECT p.user_id, p.username, p.user_email, p.user_lang, p.user_dateformat, p.user_regdate, p.user_posts, p.user_lastvisit, p.user_inactive_time, p.user_inactive_reason, r.remind_counter, r.previous_sent_date, r.reminder_sent_date, r.dont_send
			FROM ' . USERS_TABLE . ' p
			LEFT OUTER JOIN ' . $table_name . ' r
			ON (p.user_id = r.user_id) 
			WHERE p.user_id not in (SELECT ban_userid FROM ' . BANLIST_TABLE . ') 
			AND p.group_id NOT IN (1,4,5,6) 
			AND from_unixtime(r.reminder_sent_date) < DATE_SUB(NOW(), INTERVAL '.$this->config['andreask_ium_interval'] .' MINUTE) 
			ORDER BY p.user_regdate ASC '.$limit;

//			AND from_unixtime(p.user_regdate) < DATE_SUB(NOW(), INTERVAL '.$this->config['andreask_ium_interval'] .' DAY)

		// Run the query
		$result = $this->db->sql_query($sql);

		// $row should hold the data you selected
		$inactive_users = array();

		// Store results to rows
		while ($row = $this->db->sql_fetchrow($result)) {
			$inactive_users[] = $row;
		};

		// Be sure to free the result after a SELECT query
		$this->db->sql_freeresult($result);

		// Store user sho we can use them.
		$this->set_users($inactive_users);
		$this->log->add('admin', 54 , '127.0.0.1', 'got '. sizeof($this->inactive_users).' users.', time());

	}

	/**
	 * Setter for $inactive_users
	 * @param $inactive_users
	 */

    private function set_users($inactive_users)
    {
    	$this->inactive_users = $inactive_users;
		$this->log->add('admin', 54 , '127.0.0.1', 'Users are stored...', time());
	}

	/**
	 * Check if inactive_users is populated
	 * @return bool returns false if empty.
	 */

    public function has_users()
    {
		$this->log->add('admin', 54 , '127.0.0.1', 'Checking if users exist', time());
		return (bool) sizeof($this->inactive_users);
    }

	/**
	 * @param $user, used to updates/populate ext's table ium_reminder
	 */

    private function update_ium_reminder($user)
	{

		// Does the user exists in ium_reminder?
		// If he does update the row?
		if ($this->user_exist($user['user_id']))
		{
			$this->log->add('admin', 54 , '127.0.0.1', 'User exist!' , time());

			$update_arr = array('reminder_sent_date' => time());
			$counter = ($user['remind_counter'] + 1);

			if ($user['remind_counter']  == 0 ){
				$this->log->add('admin', 54 , '127.0.0.1', 'First reminder' , time());
				$merge = array('remind_counter' => $counter);
				$update_arr = array_merge($update_arr, $merge);
			}
			elseif ($user['remind_counter']  == 1 ){
				$this->log->add('admin', 54 , '127.0.0.1', 'Second reminder' , time());
				$merge = array('previous_sent_date' => $user['reminder_sent_date'],
							   'remind_counter' => $counter );
				$update_arr = array_merge($update_arr, $merge);
			}
			elseif ($user['remind_counter'] == 2 )
			{
				$this->log->add('admin', 54 , '127.0.0.1', 'Last reminder...' , time());

				$merge = array('previous_sent_date' => $user['reminder_sent_date'],
							   'remind_counter'		=> $counter,
							   'dont_send'			=> 1);
				$update_arr = array_merge($update_arr, $merge);
			}

			$sql = 'UPDATE ' . $this->table_prefix.$this->table_name . ' SET ' . $this->db->sql_build_array('UPDATE', $update_arr) .
			' WHERE user_id = '.$user['user_id'];
			$this->log->add('admin', 54 , '127.0.0.1', "$sql" , time());

			$this->db->sql_query($sql);

		}
		// User does not exist in the table let's add him.
		else
		{
			$this->log->add('admin', 54 , '127.0.0.1', 'new user... inserting' , time());

			$insert_arr = array(
				'user_id'            => $user['user_id'],
				'username'           => $user['username'],
				'remind_counter'     => 1,
				'previous_sent_date' => 0,
				'reminder_sent_date' => time(),
			);

			$sql = 'INSERT INTO ' . $this->table_prefix . $this->table_name . $this->db->sql_build_array('INSERT', $insert_arr);
			$this->log->add('admin', 54 , '127.0.0.1', "$sql" , time());
			$this->db->sql_query($sql);
		}
	}

	/**
	 * @param $user_id, used to search in ium_reminder if exist
	 * @return bool
	 */

	private function user_exist($user_id){

		$this->log->add('admin', 54 , '127.0.0.1', 'Checking if user exist in custome table...' , time());

		$sql = 'SELECT COUNT(user_id) as user_count
        FROM ' . $this->table_prefix.$this->table_name . '
        WHERE user_id = ' . $user_id;

		$result = $this->db->sql_query($sql);
		$give_back = (bool) $this->db->sql_fetchfield('user_count');
		$this->db->sql_freeresult($result);

		// Return true if user found:
		return $give_back;
	}

	private function get_from_ium_reminder($user_id)
	{
		$select_array = array(
			'user_id'    => $user_id,
		);

		// Create the SQL statement
		$sql = 'SELECT username, remind_counter, previous_sent_date, reminder_sent_date, dont_send 
        FROM ' . $this->table_prefix.$this->table_name . ' 
        WHERE ' . $this->db->sql_build_array('SELECT', $select_array);

		// Run the query
				$result = $this->db->sql_query($sql);

		// $row should hold the data you selected
				$row = $this->db->sql_fetchrow($result);

		// Be sure to free the result after a SELECT query
				$this->db->sql_freeresult($result);

		// Show we got the result we were looking for
				return $row;
	}
}
Could you please review the above code OR perhaps explain to me what could cause this symptom?
I found nothing in the logs which I tail all the time.
I also put a lot of log entries just to see if everything goes as it suppose to go but I was not able to find the culprit.

*Please excuse me for bad coding. I'm still learning and if you have any suggestions again I am open to hear them.

Thanks again for your time and effort!
Here is what I am working on right now...
Inactive User Manager for phpBB
Give it a try...
If you would like to buy me a bier ;) for my work I will drink it on a hot summer day and thank you!!!
andreask
Registered User
Posts: 623
Joined: Fri Feb 27, 2009 6:13 pm
Name: Andreas

Re: Crone task run only once.

Post by andreask »

I don't expect someone to fix my code.
What I need is someone to explain to me how I can see what is causing this?
Because I have no idea how to do it.
Board is on debug mode, but this does not seem to help me on the cron task.
Here is what I am working on right now...
Inactive User Manager for phpBB
Give it a try...
If you would like to buy me a bier ;) for my work I will drink it on a hot summer day and thank you!!!
User avatar
Elsensee
Former Team Member
Posts: 122
Joined: Sat May 07, 2011 11:04 am
Location: Hamburg, Germany
Name: Oliver Schramm
Contact:

Re: Crone task run only once.

Post by Elsensee »

You should change the last line in should_run() to return $this->config['send_reminder_last_gc'] < strtotime('10 minutes ago');
Probably the (bool) converts the saved config value into a boolean before comparing it. Don't ask why PHP doesn't convert the boolean into a integer value then... I don't know either.
However, this will fix the behaviour.
Development Team Member
I don't make bugs - I make features
andreask
Registered User
Posts: 623
Joined: Fri Feb 27, 2009 6:13 pm
Name: Andreas

Re: Crone task run only once.

Post by andreask »

Sir, you are officially my GOD my Lord and my SAVIOR! :D
Here is what I am working on right now...
Inactive User Manager for phpBB
Give it a try...
If you would like to buy me a bier ;) for my work I will drink it on a hot summer day and thank you!!!
Post Reply

Return to “Extension Writers Discussion”