-
Notifications
You must be signed in to change notification settings - Fork 0
Coding Guidelines and Development Tips
###Formatting
USE TABS, do not use spaces, for indentation.
When continuing a line of code (long if statement, many arguments to a function or method call), use TWO tabs for the continuation line to distinguish it from inner blocks:
if (foo && bar
&& baz) {
doSomething()
doSomethingElse()
}
Please always put spaces around if conditions:
if (condition) {
}
NOT
if(condition){
}
The former is more readable.
###Javascript Inheritance and JSON calls
We're using a simple Javascript inheritance scheme for building widgets and model classes which you can find in base.js. Good examples of its use: treeview.js, taglist.js, query plot.js.
Also, we have a set of JSON methods in base.js (various methods ending in JSON)... you should always use these for JSON calls. They can retry if the net connection is down, and you can also use them to queue JSON calls so they will wait for a previous response to complete before sending the next one (if you want to ensure correct order of JSON calls).
###checkData() method is used for many JSON calls
Please call checkData(), defined in curious.js, to check the data returned from JSON calls --- to check for error messages (unless the method returns a map including a success and error message). This function interprets various string codes sent back by the server, and does specific things depending on the code, for instance "login" means return to the login page, because the session has timed out, or "refresh" means refresh the data for the page. checkData() calls various support methods, for instance — showAlert() needs to pop up an alert, refreshPage() should refresh the data for the page, doLogout() should logout, etc. --- see the source code for that method. location.reload(true) should reload the app from scratch as a last resort — we might want to change this to a different function such as reloadApp() which can do whatever is appropriate for the context.
###Factory methods
Model factory methods should be static methods in the model class called create(...) and delete(...). Updating an object should happen through instance methods called update(...). The static methods should be manually wrapped in Model.withTransaction {} blocks in case they are called directly from controllers without transaction boundaries. Remember that withTransaction blocks may return a value, so you can write:
return Model.withTransaction() {
... code ...
}
The Grails standard for this sort of thing would be to use services, but we are avoiding that because of the difficulty in calling services from model classes (see below).
Note: please avoid calling new Model(...) directly, or .delete() directly. We want to use factory methods always for construction and destruction of objects to make sure that we can intercept these events globally throughout the application.
Saving objects should use Utils.save() rather than directly calling save():
Utils.save(object)
Utils.save(object, true) // flush after save
The reason for this is Utils.save() will print validation errors to the error log if there is an error persisting the object, and .save() will often fail with an exception but provide no error log.
We are using a convention where fetchXXX() means "do a computation that may have side effects to get the value" and getXXX() means "return a value that is either cached or directly accessible".
###No optimistic locking
For more reliable operation we have carefully designed most of the classes so that it is acceptable for "last saved wins" to be the policy, rather than optimistic locking. Please be careful as you add new model classes that this assumption continues to be the case --- we are trying to avoid complex aggregate data in single large model classes, and instead are distributing data in tiny chunks (for instance, user tracking data is stored in Entry classes, which are each atomic units of data). So, in general, it is not necessary to use optimistic locking to deal with concurrent changes, and we should stick with this design principle (this will also aid us when we switch to a NoSQL solution or solutions, as, frequently, NoSQL databases are not transactional).
For this reason be sure to use:
static mapping = {
version false
}
in all your model classes to avoid optimistic locking issues.
##Database migrations
We are not using any of the database migration plugins, because in practice we found them to be simultaneously too limited yet more complicated to use than just writing some SQL commands. Please note that certain simple operations (such as adding a new instance variable) do NOT require an explicit migration as Grails will do it automatically. But, things such as changing database constraints (nullable, field size, etc.), removing an instance variable/column, etc., usually do require a migration.
We are using a very simple approach which is captured in the MigrationService class. All you need to do is add a tryMigration("Unique name of migration") { ... do migration ... } block to the doMigrations() method. These migrations will be run only once on any given installation; when a migration is done it will store that fact in the database, so it will not run again.
Note that a fresh installation will not run ANY migrations --- it will mark all existing migrations as "done" until the next startup of the server (see SKIP_INITIAL_MIGRATIONS_ID). Originally these IDs were numbers but we switched them to be strings for uniqueness across branches.
If a migration is expected to take a long time, you should write it so it can run in the background (use small transactions!) and put it in doBackgroundMigrations() instead.
###SecurityFilters and Authentication
Authentication logic is currently spread between SecurityFilters, which is a generic filter that processes incoming requests and SecurityService. Whenever you add a new controller, you MUST edit the SecurityFilters class to make sure that incoming requests are correctly checked for authentication. Also, you MUST make sure that all requests are protected from XSS attacks. See existing code for examples of how this is done (search for "CSRF" in .gsp and .js files).
We are following the convention that methods that return JSON (for Ajax calls) end with "Data". Many of the /home/ methods are currently in HomeController, but JSON/Ajax calls are being handled in DataController. More recently we've broken out some of the controller methods that would have gone into DataController into separated controllers --- however doing this requires updating the SecurityFilters as noted above. Putting methods into DataController or HomeController will be automatically checked for authentication in SecurityFilters. If you want to bypass auth for a method (i.e., the home page will load for everyone whether they're logged in or not), you can add exceptions to SecurityFilters.
There are two types of calls, and two contexts. Either the call is being made from the browser, or it is being made as an Ajax call.
BROWSER CALLS: If the call is coming from the browser, you must use redirects or entire pages to display the result in HTML.
AJAX CALLS: If the call is Ajax, you must send back error codes compatible with checkData() (i.e., strings, at present, though we may change this later --- if we do change it, it will be a change across the entire app).
Certain controllers MAY BE assumed to always be using Ajax calls even if the action names do not end with the "Data". If you do this you MUST modify SecurityFilters accordingly to send back the appropriate error codes if a session is required.
The two contexts are as follows:
NO SESSION REQUIRED: The call should go through and return whether or not a session is present. This applies, for instance, to the login page and other "about" style pages. Code to determine this is present in SecurityFilters. DO NOT PUT CODE TO CHECK OR EVADE THESE CHECKS ELSEWHERE IN JAVASCRIPT OR GROOVY CODE.
SESSION REQUIRED: The call requires a session. Again, SecurityFilters must detect this and issue either a redirect to the login page (if it is a browser call) or an error code (if it is an Ajax call).
A no-session-required browser page MUST NEVER CALL A SESSION-REQUIRED AJAX CALL. If this is occurring, the code MUST be rewritten accordingly. Do not place hacks into the code to get around these security requirements.
###Calling services from model classes
To avoid needless waste of memory, we aren't using Grails injection to inject services into model instances. Instead, since these are global singletons, we are using a static set() and get() method in the service classes. See BootStrap:
DatabaseService.set(databaseService)
which is used in model classes as:
DatabaseService.get()....
###The Model class
Some Grails methods can only be called syntactically by using a model class, as in "withTransaction". However, in some cases it isn't obvious which model class to use, so we have a dummy class called "Model" in the us.wearecurio.model package which you can use for these cases:
Model.withNewSession {
...
}
or
Model.withTransaction {
...
}
###DatabaseService class
Although eventually we will likely migrate to a NoSQL database, for now we are using MySQL and occasionally there are some things that can only be done easily or at all using SQL queries rather than HQL or GORM queries. We have a DatabaseService helper class for that purpose, that provides a bunch of easy-to-use functionality for calling SQL, processing result sets, etc. Please see the class and/or search the IDE for examples of use.
DatabaseService also includes a retry() abstraction which used to be used to retry code that fails due to an optimistic locking exception. However, since we have eliminated optimistic locking from most of our code, this method is no longer that necessary.
###Integration tests
All new code should have coverage with integration tests and controller tests, at least to do minimal "make sure it doesn't crash" code paths. Please see existing integration and controller tests for examples.
###Long-running Batch Processes
VERY IMPORTANT: You MUST write long-running batch processes so they use small, atomic transactions, otherwise you could freeze up the database during production for long periods!
Because of the bizarre nature of Grails capitalization conventions, if you start a class name with two uppercase letters the corresponding injected service object name is the same as the class name. I.e., OAuthAccountService is injected as a variable with the name "OAuthAccountService". Which makes no sense. But there it is. This makes code hard to read because now we have some variables with the same name as their class.
Hence --- new coding convention --- no service names with two uppercase letters to start. For example, OAuthAccountService should be called OauthAccountService.
We have various static caches we are using to speed things up — but these caches need to be made thread-safe, and they also need to be set up to clear themselves at the beginning of each test. Anything you'd like to run at the beginning of each setUp() method in tests which inherit from CuriousTestCase, etc., can add a closure which will be called at the start of each test, via Utils.registerTestReset(). See below for an example:
static Map<Long, List<Long>> tagIdCache = Collections.synchronizedMap(
new BoundedCache<Long, List<Long>>(100000))
static Map<Long, List<Long>> tagGroupIdCache = Collections.synchronizedMap(
new BoundedCache<Long, List<Long>>(100000))
static {
Utils.registerTestReset {
synchronized(tagIdCache) {
tagIdCache.clear()
tagGroupIdCache.clear()
}
}
}
We're using SiteMesh's applyLayout to facilitate nested templates. The layout structure uses template inheritance:
base.gsp - empty layout with basic Curious includes
-- logo.gsp - basic Curious logo and footer
---- menu.gsp - adds nav menu
------ plot.gsp - adds support for graphing
-------- graph.gsp - graph page layout (used by the graph and graphSignals views)
------ feed.gsp - feed layout
-- home.gsp - layout used by home page (and register, login, etc., pages)
These layouts inherit from each other in the above hierarchy, thus eliminating duplicated markup.
The base, logo, and home templates do not request login information from the server. Only menu, plot, graph, and feed, templates used on the app once you've logged in, will request login data.
This layout structure is based on a technique referenced in this article:
http://mrhaki.blogspot.com/2011/04/grails-goodness-applying-layouts-in.html
The basic idea is the base layout (base.gsp) is written like a standard Grails layout, but all the intermediate layouts use applyLayout to "inherit" layout from their parent layout. The actual view itself uses the standard meta tag for applying layouts. Please note that if you want to propagate page properties and/or layoutHead, layoutBody, etc., elements to parent layouts, you must include the property in all layouts in the inheritance hierarchy. Same goes for propagating injected variables, you must re-inject them to parent layouts if you want to reference them.
Create a new workspace by renaming your existing workspace and creating a new blank directory. Import your Grails project from the old workspace, copying dependencies. You may need to reconfigure the available Grails builds. Change the Groovy compiler version to 2.4 for the project, and go through the Eclipse setup again from the onboarding page.