andig / videodb

The videoDB media collection software
65 stars 42 forks source link

seemingly redundant code using $config[http_caching] #187

Closed copperhead57 closed 1 year ago

copperhead57 commented 1 year ago

hi @johanneskonst , @kec2, @andig i have found what looks like redundant coding in a few modules that relies on the variable $config['http_caching']which is no longer set anywhere. code appears in template.php, index.php, search.php and show.php

Can we remove references to this variable? Cheers @copperhead57

Sample of the code found in template.php.

    // caching enabled?
    if ($config['http_caching'])
    {
        require_once('./core/httpcache.php');
        httpCacheCaptureStart();
    }

    tpl_display_show($template, !$config['http_caching']);

    if ($config['http_caching'])
    {
        httpCacheOutput($template, httpCacheCaptureEnd());
    }
kec2 commented 1 year ago

Hi

I don't think so. It is set in config.inc.php

$config['http_caching'] = 1; // enable http caching

Hilsen

Klaus Christiansen


From: Paul copperhead @.> Sent: 08 January 2023 09:43 To: andig/videodb @.> Cc: kec2 @.>; Mention @.> Subject: [andig/videodb] seemingly redundant code using $config[http_caching] (Issue #187)

hi @johanneskonsthttps://github.com/johanneskonst , @kec2https://github.com/kec2, @andighttps://github.com/andig i have found what looks like redundant coding in a few modules that relies on the variable $config['http_caching'] which is no longer set anywhere. code appears in template.php, index.php, search.php and show.php

Can we remove references to this variable? Cheers @copperhead57https://github.com/copperhead57

Sample of the code found in template.php.

// caching enabled?
if ($config['http_caching'])
{
    require_once('./core/httpcache.php');
    httpCacheCaptureStart();
}

tpl_display_show($template, !$config['http_caching']);

if ($config['http_caching'])
{
    httpCacheOutput($template, httpCacheCaptureEnd());
}

— Reply to this email directly, view it on GitHubhttps://github.com/andig/videodb/issues/187, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAXXF5QAMNV32DR7C2MNR6LWRJ44ZANCNFSM6AAAAAATUOMFOI. You are receiving this because you were mentioned.Message ID: @.***>

copperhead57 commented 1 year ago

@kec2 Hi, its not in mine or in the current config.sample.php file. i've checked sample back to 2013 and $config[http_caching] does not get mentioned, maybe an artifact from pre video 4.0 but still in peoples config.ini.php file.

kec2 commented 1 year ago

Hi Paul

You are right. Maybe I have added it, because of a warring. I don't remember.

The code was hosted at sourceForge before github. https://sourceforge.net/projects/videodb/files/videodb/ videoDB - Browse /videodb at SourceForge.nethttps://sourceforge.net/projects/videodb/files/videodb/ VideoDB is a database to manage your personal video collection. It uses PHP and MySQL and features fetching Movie data from the Internet Movie… sourceforge.net 

I have done a little digging and http_caching was introduced in version 2.1.0 back in 2005 01.09.05

But it was not added to config.sample.php in september 2005. If it ever was I don't know. I did not go through all version. There is also a misspell in index.php: if ($config['httpcaching']), that exists to this day.

Also the HttpClient uses the cache.php to get/set cache. Not httpcache.php. It's a little messy and maybe not complete. And templates are cached even if http_caching is false execpt index.php, show.php,

Hilsen

Klaus Christiansen


From: Paul copperhead @.> Sent: 08 January 2023 11:42 To: andig/videodb @.> Cc: kec2 @.>; Mention @.> Subject: Re: [andig/videodb] seemingly redundant code using $config[http_caching] (Issue #187)

@kec2https://github.com/kec2 Hi, its not in mine or in the current config.sample.php file.

i've checked sample back to 2013 and $config[http_caching] does not get mentioned, maybe an artifact from pre video 4.0 but still in peoples config.ini.php file.

— Reply to this email directly, view it on GitHubhttps://github.com/andig/videodb/issues/187#issuecomment-1374797153, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAXXF5UOK6YKIAD6CKEPSQ3WRKK2NANCNFSM6AAAAAATUOMFOI. You are receiving this because you were mentioned.Message ID: @.***>

copperhead57 commented 1 year ago

@kec2 Hi Klaus hope i am on right track here on checking the code in templates on line 54 tpl_display_show($template, !$config['http_caching']); if http_caching is not defined it is treated as if htp_caching has been set to '0', and true will be passed and the flush command will be done with no caching. this is similar to code in search, index and show. Cheers Paul.

edited. I see 3 options 1) remove code 2) add $config['http_caching] to config.sample with default '0' and fix misspelling in index 3) no changes

my preference would be option 2 ?

johanneskonst commented 1 year ago

I'm slightly in favour of 1 because in my experience http caching brings extra complexity for only minor gain, and in my opinion a project like this doesn't (shouldn't?) need http output caching. But for the sake of not making too many drastic changes and trying to keep BC I'll vote 2 as well...

andig commented 1 year ago

From what I can remember I agree...