It seems like a bunch of settings are getting put into $_SESSION. Is this so that each user can change these settings for the session? I think that could be useful for some settings (e.g. items per page), but I don't understand others like the plugin list. What's the plan here?
Don't know that there was a plan:-) Just seemed to me that settings ought to be available to all code in an application. Storing thenm all into $_SESSION once seems like the right thing to do. In fact in my own code I do it as part of Logon for that reason, and only update $_SESSION when there is a change. OpenBiblio, I think, updates $_SESSION each time a module is invoked which doesn't seem to be the best idea.
The plugin list is used by the menu, so probably could be exempted. I suspect I did it as I did simply because it was simpler to do so, and didn't think the overhead a big deal at the time. I usually lean toward simplicity and reduction of programmers time at the cost of a bit of processing. I prefer to have all that stuff available if I should need it rather than to have to go make a special package for each case.
I think what I'm missing is what makes $_SESSION better than Settings::get('foo'). Both of them are available to the whole application, but the second one tells you you're asking for a system setting rather than something specific to this session.
I agree that there should be a single, simple way to load the current public settings from JS. The model component is already there in model/Settings.php, no?
I suppose if we had had this discussion in July when I began, this would not have happened. :-) I think you are right, aside from reducing the 'load' on the db server, I see no advantage to use of $_SESSION. It's simply what I thought of first. I don't know that I have ever looked at Settings.php. Remember I got into this to work on a specific isue - lookup. I never intended (nor do I want to now) get involved in the entire program.
In my mind I think of db accesses for the dynamic stuff or things modified by some other user. $_SESSION stuff seems to me to be static durring a users's connected time (that's what a session is I think). I don't know that I have been careful to truly implement what I have in that fasion, but that was the intent. However, as I have said before, this is your code and we should be doing it your way.
Actually, there's no extra load on the server. If you look at model/Settings.php, you'll see that it loads all the settings at startup and does not talk to the DB at all for Settings::get().
What am I missing here? If you do not store the settings in $_SESSION, then each time a page is started by the browser, then you have to reload the settings. In my mind that is a no-no. So the way I wrote that stuff, it all goes to #_SETTINGS, and again whenever you modify the settings, $_SESSION is updated. Now in my home case with only one user on a occasional basis who cares. But if I were at the Seatle, S.F. or Boston Public Library with lots of librarians at lots of counters this is a major issue. At more realistic places I really do not know. But still it seems like good practice.
Still your code, do it your way.
I think you are missing something: the current way of using $_SESSION actually loads the database a bit more than just using Settings::get(). Unless I'm really misreading shared/common.php, no time is saved using the $_SESSION scheme.
PHP keeps almost no state in RAM between page loads - everything's loaded from the DB every time a new PHP script is run. This includes the session data, see shared/common.php:119 and classes/SessionHandler.php. Settings::load() (called at shared/common.php:105) loads the current settings from the DB once per PHP script run - no matter how many times you call Settings::get(). So whether you access the settings using Settings::get() or $_SESSION, there is no DB use at access, and there is always one DB "hit" for settings in shared/common.php. The only difference between your way and my way as far as MySQL is concerned is that with your way the settings are loaded twice: once from the php_sess table and once from the settings table.
There are two decent ways I can think of to avoid reading the settings table on every page load: A. Load settings into the session only once, and never update them as long as the session sticks around. B. Load settings into the session only when they are older than n seconds, and keep a timestamp in the session so we know when to reload. The current code does neither of these. If necessary, either is a trivial change to model/Settings.php - nothing else need be touched, if we're using Settings::get(). But neither of them should be done unless or until measurement shows that Settings::load() is a speed bottleneck.
OK, I have never have used a construct like Settings::get() in my own code. I know of them from C++, but have never felt comfortable with them. As a result I never looked into how or where they obtained the data; I assumed it was as required.
The php_sess table was a new one for me. I don't think I ever noticed it before. I just checked the content and if the field 'last_access_dt' can be trusted, the table doesn't get much use - current value is '2009-06-17 12:17:50'.
Also I was mixing up in my mind the work I have been doing for the last few years and OpenBiblio. In my code, session values, which include the current staff member attributes, are loaded into $_SESSION at login and updated only when and as the relevent db tables are modified in some way. So that is why I think of it as 'static' session data.
In the newer AJAX based code, the session data gets loaded when the page is initially created then again every time there is an AJAX transmission to the host. That happens a lot if you watch the FireBug console display. (But realistically, response time on a LAN is very good with my local server so I dont think there is any speed bottleneck. Still… )
Seems to me that moving the $_SESSION loading to login time would be a 'good' thing. To make it all work with legacy code we could modify the Settings model code to load from $_SESSION if present, else from the DB. That doesn't sound like much effort to me - no more than a few hours to get it all together.
I said that _if_necessary_ Settings could be loaded once at login time. $_SESSION can't. $_SESSION is reloaded for every script run because that's how PHP does it. We can decide where and how to store the session data, but PHP will load and save it when it wants.
As to the dates in php_sess, I would imagine that there is more than one row in the table, at least if you're logged in to OpenBiblio while you're doing your select. There should be one per session, and PHP doesn't always delete them immediately when you log out. There's a chance that old rows will be cleaned up after they've been stale for too long - the odds of doing clean-up and how long is "too long" are php.ini settings. It's possible that the cleanup code is broken, though. It doesn't get tested often, and PHP doesn't complain if it doesn't work.
Please don't modify Settings.php to load settings only once per login. Optimization costs programmer time, can introduce bugs, and usually reduces code clarity. The only time optimization is justified is if a real-world system is running slow and actual measurements point at the bottleneck. Then you fix the bottleneck. These men are right:
“We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil. Yet we should not pass up our opportunities in that critical 3%. A good programmer will not be lulled into complacency by such reasoning, he will be wise to look carefully at the critical code; but only after that code has been identified.” - Donald Knuth
“Bottlenecks occur in surprising places, so don't try to second guess and put in a speed hack until you have proven that's where the bottleneck is.” - Rob Pike
This counts even more for optimizations that might introduce user confusion: "I just changed the library hours, why do the old ones still show on this computer? I keep hitting 'refresh' and nothing changes!"
Log in to post a comment.
Sign up for the SourceForge newsletter:
You seem to have CSS turned off.
Please don't fill out this field.