Return-Path: 
 <bloodhound-commits-return-2868-apmail-incubator-bloodhound-commits-archive=incubator.apache.org@incubator.apache.org>
X-Original-To: apmail-incubator-bloodhound-commits-archive@minotaur.apache.org
Delivered-To: apmail-incubator-bloodhound-commits-archive@minotaur.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 7513AE841
	for <apmail-incubator-bloodhound-commits-archive@minotaur.apache.org>;
 Fri, 25 Jan 2013 22:56:53 +0000 (UTC)
Received: (qmail 29833 invoked by uid 500); 25 Jan 2013 22:56:53 -0000
Delivered-To: apmail-incubator-bloodhound-commits-archive@incubator.apache.org
Received: (qmail 29807 invoked by uid 500); 25 Jan 2013 22:56:53 -0000
Mailing-List: contact bloodhound-commits-help@incubator.apache.org;
 run by ezmlm
Precedence: bulk
List-Help: <mailto:bloodhound-commits-help@incubator.apache.org>
List-Unsubscribe: <mailto:bloodhound-commits-unsubscribe@incubator.apache.org>
List-Post: <mailto:bloodhound-commits@incubator.apache.org>
List-Id: <bloodhound-commits.incubator.apache.org>
Reply-To: bloodhound-dev@incubator.apache.org
Delivered-To: mailing list bloodhound-commits@incubator.apache.org
Received: (qmail 29800 invoked by uid 99); 25 Jan 2013 22:56:53 -0000
Received: from bloodhound-vm.apache.org (HELO bloodhound-vm) (140.211.11.32)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 25 Jan 2013 22:56:53 +0000
Received: from bloodhound-vm.apache.org (localhost [127.0.0.1])
	by bloodhound-vm (Postfix) with ESMTP id 10D5A80736
	for <bloodhound-commits@incubator.apache.org>;
 Fri, 25 Jan 2013 22:56:52 +0000 (UTC)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
From: "Apache Bloodhound" <bloodhound-dev@incubator.apache.org>
X-Trac-Version: 1.0
X-Trac-Wiki-URL: https://issues.apache.org/bloodhound/wiki/Proposals/BEP-0005
Cc: bloodhound-commits@incubator.apache.org
Auto-Submitted: auto-generated
X-Mailer: Trac 1.0, by Edgewall Software
X-Trac-Project: Apache Bloodhound
Date: Fri, 25 Jan 2013 22:56:52 -0000
Reply-To: bloodhound-dev@incubator.apache.org
X-URL: https://issues.apache.org/bloodhound/
Subject: [Apache Bloodhound] Proposals/BEP-0005 modified
Message-Id: <20130125225653.10D5A80736@bloodhound-vm>

Page "Proposals/BEP-0005" was changed by franco
Diff URL: <https://issues.apache.org/bloodhound/wiki/Proposals/BEP-0005?action=diff&version=2>
Revision 2
Changes:
-------8<------8<------8<------8<------8<------8<------8<------8<--------
Index: Proposals/BEP-0005
=========================================================================
--- Proposals/BEP-0005 (version: 1)
+++ Proposals/BEP-0005 (version: 2)
@@ -18,68 +18,135 @@
 
 == Abstract #abstract
 
-<Delete text in this section and add a short (~200 word) description of the technical issue being addressed. Take a look at sample abstract below>
+Plugin upgrades is a reusable mechanism to provide trac plugins an upgrade process similar to the core upgrade process of trac itself. [[BR]]
+The upgrade code of each new version of the plugin can be encapsulated in a separate module, the mechanism dynamically loads the modules and consistently uses them as indicated for each plugin version. The amount of coding needed in the plugin is reduced as much as possible, since repetitive code related to the upgrade process is handled in a reusable manner.
+[[BR]]
+Test cases are provided to validate the general reusable algorithm in both scenarios incremental and full plugin upgrade.
 
-This template provides a boilerplate or sample template for creating your
-own BEPs.  In conjunction with the [wiki:/Proposals general content guidelines] and the [wiki:/Proposals/Formats/WikiFormatting WikiFormatting BEP guidelines]  
-, this should make it easy for you to conform your own
-BEPs to the format outlined below. See [#howto How to Use This Template] for further instructions.
 
-**Note**: if you are reading this template via the web, you should first try to create a new wiki page by selecting `ProposalsRst` |page template guide|.  **DO NOT EDIT THIS WIKI PAGE IN ORDER TO CREATE A NEW BEP! **
-
-If you would prefer not to use WikiFormatting markup in your BEP, please see  [wiki:/Proposals/Formats/RestructuredText reStructuredText BEP guidelines].
 
 == Motivation ==
 
-<The motivation is critical for BEPs that want to change the copy of ''Trac'' patched using vendor branch . It should clearly explain why the existing ''Bloodhound'' solution is inadequate to address the problem that the ''BEP'' solves. ''BEP'' submissions without sufficient motivation may be rejected outright. >
+An important motivation is been that of simplifying as much as possible the plugins concrete work regarding the environment upgrade by reusing steps of the upgrade algorithm that would be otherwise strong candidates for code repetition between plugins.
+
+Till now, some class in the plugin must implement trac’s IEnvironmentSetupParticipant interface.  The operation '''environment_needs_upgrade''' implemented in this class would have a very similar implementation across plugins, it would be something like:
+
+•	Query for installed version
+•	Check if installed version is greater than current plugins version, raising error if it is
+•	Check if current plugin’s version is greater than installed version and if so, environment does need to be upgraded
+
+There is an opportunity here to introduce some Template Method (GoF), leaving the plugin the space to indicate its name and its current version number, the rest of the algorithm could be reused avoiding code repetition.
+
+The operation '''upgrade_environment''' of IEnvironmentSetupParticipant  is likely to be implemented providing new tables to the schema (generally if it is the first fresh install) inserting new/default data to the tables specified and/or performing some unstructured database update (e.g. changing field types, adding new columns, updating existent data values).
+
+In current implementations like '''multiproduct.api.!MultiProductSystem''', you can see the code making if statements evaluating if the current version is the first one, else if it is the second one to alternatively create schema tables or simply perform the upgrade block related to the new version.
+
+'''What will happen once there get to be several versions of the plugin?''' Each one of them could require new tables, new data, and/or modification to the existing schema or data.
+
+There will possibly be a need to constantly change the already functioning code. 
+
+The possible improvement here comes with the application of the Open Close design principle; the existing code wouldn’t need to change if each new block of changes regarding each new plugin version is kept separately. So the design and code gets Open for extensions and closed for modifications as the design principle states.
+
+When the upgrade process finishes, the plugin needs to update the system table indicating the current version has been installed. This step once more is a candidate for code repetition across plugins. 
+
+All along the process, the plugin might have the need to register logs, and that is yet another aspect to reuse.
+
+Every fragment of code that is mentioned to be exposed to copy+paste between different plugins, is a point for possible bugs to be injected. The whole upgrade algorithm of the plugin inside a single upgrade operation, when version numbers increase, is also error prone, it affects both maintainability and testability. 
+
 
 == Proposal #proposal
+=== Summary:
+''Let the plugin implement/indicate only what is variable during the environment setup process, leaving everything that might be abstracted and reused in the hands of the reusable mechanism''
 
-<The technical specification should describe any new features , detail its impact on the components architecture , mention what plugins will be included as a result , whether they are hosted by ​[http://trac-hacks.org trac-hacks.org] or not , and any other relevant technical subject . The specification should be detailed enough to allow competing, interoperable implementations for any of the current supported database platforms (e.g. ''SQLite'', ''Postgres'', ''MySQL'') and server technologies (e.g. ''Apache HTTPD server'', ''nginx'', ''mod_wsgi'', ''CGI'').. >
+=== Details:
+1.	Create a new module bhdashboard.env, to place reusable elements. Create a base class named '''!BaseEnvironmentSetupParticipant''' within it; This class implements trac’s IEnvironmentSetupParticipant and contains all reusable logic regarding the environment upgrade process.
+
+2.	Have some component in your plugin extend this base class, and provide concrete information that varies from a plugin to another. Instead of directly implementing '''IEnvironmentSetupParticipant''', the component will override the following methods:
+||=Method =||=Description =||
+||get_db_system_key  ||  To return the value of the key that will be used to store  the current plugin's version number in the 'system' table of trac. If not provided will get a default value formed by plugin_name + “_version” suffix. ||
+||get_plugin_name ||Returns the plugin's name to be used in the logging messages   during the current environment setup process.  ||
+||get_plugin_version||To return the current version of the plugin  ||
+||get_db_setup_contributors =||Returns a list of db setup script names, E.g. (‘plugin.upgrades.db0’, ‘plugin.upgrades.db1’). The mechanism here gives the plugin a chance to explicitly indicate the setup scripts that must be used for the setup process.  ||
+[[BR]]
+
+3.	Implement each plugin version’s db upgrade in a separate module accomplishing the following protocol.
+
+The module must provide:
+||=Function =||=Description =||
+||get_new_tables()	||To indicate new tables to add to the schema. The returned structure is expected to be similar to trac.db_default.schema variable||
+||get_new_data()	||To indicate initial data to insert into the database. Its return value must be like the one returned by trac.db_default.get_data() function
+||do_upgrade() 	||To perform any free db upgrade if needed
+[[BR]]
+
+
+
+4.	At least two test scenarios must be verified:
+* A complete install from scratch where no version of the plugin exists and a version N is installed
+* An incremental install where a first initial install is made and subsequent versions are installed later.
+
+
 
 == Rationale #rationale
 
-<The rationale fleshes out the specification by describing what motivated the design and why particular design decisions were made. It should describe alternate designs that were considered and related work, e.g. how the feature is supported in other issue trackers or ''Trac'' hacks . The rationale should provide evidence of consensus within the community and discuss important objections or concerns raised during discussion. Take a look at sample rationale below>
+1.	The design was motivated by the way trac implements its core environment setup: A default_db module contains default initial schema and data. Subsequent install versions are kept apart one module per version number. In accordance with the current trac’s version and the current installed version, those setup upgrades are loaded and ran from current installed version to current environment’s version. 
 
-''BEP'' submissions come in a wide variety of forms, not all adhering to the format guidelines set forth below. Use this template, in conjunction with the [wiki:/Proposals general content guidelines] and the [wiki:/Proposals/Formats/WikiFormatting WikiFormatting BEP guidelines], to ensure that your ''BEP'' submission is easy to read and understand.
+2.	A first decision to be made was '''''whether encapsulating upgrade algorithms in classes or directly in modules.''''' 
+* Classes would allow us to have one class per plugin version no matter they were placed in a single module each or not. [[BR]]
+* Finally the use of separate modules without classes prevailed taking into consideration '''''memory use optimization''''', if a version was not to be used; the module and every declared member on it wouldn’t be loaded at all.
 
-This template allows to create BEPs and is very similar to [http://www.python.org/dev/peps/pep-0012 PEP 12] . However it has been optimized by moving long explanations to the [wiki:/Proposals/Formats/WikiFormatting WikiFormatting BEP guidelines] . If you are interested take a look at the  [?action=diff&old_version=1 differences]. The goal is to redact new BEPs just by following in-line instructions between angle brackets (i.e. **<** **>**) . Even if this will allow to write BEPs faster , it is highly recommended to read the [wiki:/Proposals/Formats/WikiFormatting WikiFormatting BEP guidelines] at least once in your lifetime to be aware of good practices and expected style rules . 
+3.	A second design decision to be made was '''''how to dynamically load the setup upgrade modules?'''''  
+* This could be done by convection like trac does, the plugin should then have a directory named “upgrades” and modules within it should be named following the ‘db<version_number>’ convention; this way ‘db0.py’ corresponds to the initial install and ‘dbN.py’ corresponds to the N version of the plugin.
 
-== How to Use This Template #howto
+* The decision made was to let the plugin '''''explicitly''''' indicate the names of the setup script modules, '''''to gain in flexibility.''''' This way setup scripts wouldn’t be forced to be placed in a single directory. The mere existence of these modules within “upgrades” directory wouldn’t forcedly indicate they should be used for the install. Been so, the plugin could indicate that some db upgrade module e.g version n-1 is not to be used during its current version setup, maybe because the current version’s setup totally overrides the previous one.
 
-<BEPs may include further sections. This is an example.>
+4.	 A third decision to be made was '''''whether using !ModelBase._get_schema() to generate the schema data of the environment setup script or not.'''''
+* Once the benefits of keeping each version of db upgrades separate are clear, saying that bhdashboard.model.!ModelBase._meta '''''is not version aware''''' should be enough. !ModelBase._meta has always the last snapshot of  the database schema related to a persistent class. This means that making a fresh install of version N from scratch works, but making an upgrade from version n-3 to version n would result a bit difficult, since there is no track of differences between versions.
 
-Quick edits will consist in following the instructions inside angle brackets (i.e. **<** **>**) . That should be everything needed to write new BEPs. To be more informed about advanced considerations please read the [wiki:/Proposals general content guidelines] and the [wiki:/Proposals/Formats/WikiFormatting WikiFormatting BEP guidelines] . If there is no point in including one of the sections in this document then feel free to remove it.
+* Let’s in addition say that the structure of the real database schema could be richer than what !ModelBase._meta supports. Indicating indexes, size of the columns, data types, m:n relationships 1:n relationships and so on. !ModelBase._meta is not yet aware of some of these elements, and maybe it shouldn’t even be, since it doesn’t need many of them at all '''''(CRUD logic doesn’t really need field size, type or indexes).'''''
+ 
+* The only reason why !ModelBase might need some of these elements is to '''''let us generate the db schema''''' during the env upgrade process.  [[BR]] That  makes us wonder if '''''it shouldn’t just be the opposite.'''''  Or maybe due to the richness of what a setup script might imply(take a look a trac.upgrades directory), and the relative independence of !ModelBase with that upgrade logic, it would be appropriate for !ModelBase to '''''have only the last snapshot of exactly what it needs''''' while setup scripts make the install magic with as much details as required.
+
+* While no further decision is made over the responsibilities of !ModelBase '''''maybe it is safe to keep things apart.'''''
+
 
 == Backwards Compatibility #backwards-compatibility
 
-<All BEPs that introduce backwards incompatibilities must include a section describing these incompatibilities and their severity. The ''BEP'' must explain how to deal with these incompatibilities. ''BEP'' submissions without a sufficient backwards compatibility treatise may be rejected outright. >
+TODO: verify if there is something needed here
 
 == Reference Implementation #reference-implementation
 
-< The reference implementation **must** be completed before any ''BEP'' is given status **Final**, but it need not be completed before the ''BEP'' is accepted. It is better to finish the specification and rationale first and reach consensus on it before writing code. The final implementation **must** include test code and documentation appropriate for either the wiki pages in ''Bloodhound'' users guide or an specific wiki page in the [http://issues.apache.org/bloodhound ​issue tracker] . >
+You can find a reference implementation [https://bitbucket.org/jose_angel_franco/bloodhound/compare/bep_0005_plugin_upgrades..default here], within a fork of bhdashboard plugin, a branch named bep_0005_plugin_upgrades has been created to ease the revision process.
+The diff includes the module bhdashboard.env.py with the implementation of the !BaseEnvironmentSetupParticipant class and the test related modules implementing two test cases for the identified test scenarios.[[BR]]
+During the work for #140 this mechanism has been used, the code of bhdashboard.api.!DashboardSystem regarding environment upgrade looks something like this:
+{{{
+class DashboardSystem(Component, BaseEnvironmentSetupParticipant):
+...
+#BaseEnvironmentSetupParticipant methods for handling db updates
+def get_plugin_name(self):
+    return 'Bloodhound dashboard'
+
+def get_plugin_version(self):
+    return 0 #First time installer, should be changed as the version increases
+
+def get_db_setup_contributors(self):
+    return  "bhdashboard.upgrades.db0", #Nothing but a first install for now
+...
+}}}
+
+
+
 
 == Resources #resources
 
-<Provide links to useful resources related to the subject discussed. See sample text below>
+* Template Method design pattern by GoF [http://www.amazon.com/Design-Patterns-Object-Oriented-Professional-Computing/dp/0201634988 Elements of Reusable Object-Oriented Software]
 
-See WikiFormatting for more help on the markup used to write wiki pages.
+* Open / Closed design principle by Robert C. Martin [http://www.amazon.com/Software-Development-Principles-Patterns-Practices/dp/0135974445 Software-Development-Principles-Patterns-Practices]
 
 == References #references
-
-<List the references included in BEP body>
-
-  1. PEP 1, PEP Purpose and Guidelines, Warsaw, Hylton
-     http://www.python.org/dev/peps/pep-0001/
-  2. PEP 9, Sample Plaintext PEP Template, Warsaw
-     http://www.python.org/dev/peps/pep-0009
-  2. PEP 12, Sample reStructuredText ''PEP'' Template, Goodger, Warsaw
-     http://www.python.org/dev/peps/pep-0012/
-  3. http://www.opencontent.org/openpub/
+[[BR]]
+TODO: check if there should be a reference here to the ticket related to the use of bhdashboard.!ModelBase._meta as a source for environment upgrades 
 
 == Copyright #copyright
-
-<In this section all licensing issues should be meticulously exposed . Library and plugin dependencies are among the most important topics . On the other hand each BEP will be explicitly labelled with a copyright statement like shown below, so should not change that. Requests for a different copyright statement have to be posted to bloodhound-dev@incubator.apache.org . For more details consult [wiki:/Proposals#what-belongs-in-a-successful-bep BEP structure explained] .>
 
 Copyright © 2009-2012 The [http://www.apache.org Apache Software Foundation] [[BR]] 
 Licensed under the [http://www.apache.org/licenses/LICENSE-2.0 Apache License, Version 2.0].
-------8<------8<------8<------8<------8<------8<------8<------8<--------

--
Page URL: <https://issues.apache.org/bloodhound/wiki/Proposals/BEP-0005>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker

This is an automated message. Someone added your email address to be
notified of changes on 'Proposals/BEP-0005' page.
If it was not you, please report to .