[Opensim-dev] Some new code for your review and possible acceptance

[Justin Clark-Casey]

Hi Alex.  Always great to see someone new playing with the code!  In general, our submission process has been to put patches on Mantis and then talk to developers about them in IRC - it's more work for us to have to search through remote git repositories and reduces the chance that these changes will come in.

I've also made contextual comments inline.

AlexRa wrote:
> Hello,
> 
> I've been playing around with the database access layer of the code, especially the AssetData plugins
> for all 3 supported databases (Sqlite, MySql, and MS SQL) and the underlying classes, and also made
> a few other changes:
> 
>    git at github.com:AlexRa/opensim-mods-Alex.git
> 
> I'd like to invite the developers of the OpenSim team to review the proposed changes
> and integrate whatever looks useful into the official repository.
> 
> This repository contains the following branches that can be of interest:
> 
> The *"master"* contains all my the changes. It is branched off
> r/127730 (and with the recent flow of commits I find it difficult to
> continue rebasing it to keep up). There is quite a lot of stuff there,
> some of it more useful than the other, so if anybody wants to look at
> it I'd appreciate that, but that may be too much to ask.
> 
> I've been trying to separate particular features and put them into separate
> branches. So far, the following branches are fairly independent from
> the rest of the stuff and should be easy to integrate into the primary
> repo if the team decides to do so:
> 
> * The "asset-loader"  branch contains a rewrite of the AssetLoader that
>   uses a timestamped asset to determine which of the asset files must
>   be loaded. This prevents the lengthy (and unnecessary) re-loading of
>   all predefined assets on each server start-up, but does load any
>   newly added assets. Status: NOT TESTED (but the logic is quite
>   simple, shouldn't have much problems with it(?))

Sounds good.  Of course, actually testing this would be even better since that's actually the hard part.

> 
> * The "Migrations" branch is a rewrite of the Migrations.cs to support
>   single-file migration histories. This way, all those endless
>   0xx_StoreName.sql are replaced with a single StoreName.migrations
>   file which is more readable and easier to maintain. The new
>   migration code supports stored procedures for both MySql and MS SQL.
>   It is also fully backward-compatible (does read the old-style
>   migration files as well).
> 
>   This also includes update of MySql.Data.dll to 6.2.3.0, to avoi a
>   known problem with defining stored procs/funcs in earlier versions.
> 
>   Status: TESTED and works in all modes.

I believe migrations were inspired by the Ruby on Rails way of doing things, so it would be nice to stick to this if possible (of course, your way may be completely compatible for all I know).  In the past, the migrations have been tidied up simply by bunching all the oldest ones up into a new 000 migration.

> 
> * The "NUnit-update" touches a lot of test-related code to make it compatible
>   with NUnit 2.5.5+. The main advantage is that test classes can be
>   made generic and parameterized. This (and the BasicDataServiceTest
>   class included in this branch) allows to build DBMS-independent data
>   tests. Some samples of such tests are available in my "master"
>   branch. The idea is to keep he connection strings for the test
>   databases in a single INI file (rather than in each test unit), and
>   to keep all data tests in OpenSim.Data.Tests, totally eliminating
>   Data.{MySql/MSSQL/SQLite}.Tests. Test sets for a specific DB are
>   produced just by adding another [TestFixture(...)] attribute before
>   the test class.

It would be very good to update nunit - just today I was looking to use the throws assertion before realizing that it was only implemented in nunit 2.5.  It would be especially nice to see this work as patches, preferably separating the nunit upgrades from any other new tests.

> 
> The other new stuff that is present in the "master" branch:
> 
> * Added Assets.CreatorID support for all dbs;
> 
> * A way to keep Asset.AccessTime updated (on MySql, a similar way
>   would work on MS SQL) entirely on the SQL server side, without the
>   performance costs of a separate UPDATE statement;
> 
> * refactored DB-access classes (more code moved to a base class, less stuff is DBMS-specific);
> 
> * configurable "keep alive" connection mode (for MySQL/MSSQL) - avoid
>   re-creating conn, keep stmts prepared (2x performance gain for MS SQL);
> 
> * performance tests for Assets;

These sound good.  But again, it's far easier for us, I would say, if these are submitted as discrete patches, probably best one at a time until each is accepted.  I know the codebase can change quickly but unfortunately this is sometimes a cost that has to be bourne.

-- 
Justin Clark-Casey (justincc)
http://justincc.org
http://twitter.com/justincc