From: David Gageot <david@gageot.net>
Date: Mon, 1 Oct 2012 14:18:19 +0000 (+0200)
Subject: SONAR-3529 Property field validation
X-Git-Tag: 3.3~155
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=92ad85d9a740553f4c971b8732b34121dafd5af7;p=sonarqube.git

SONAR-3529 Property field validation
---

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java
index caea3a388b7..3a4fcbea156 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java
@@ -81,7 +81,7 @@ public class PropertyDefinitionTest {
   }
 
   @Properties(@Property(key = "hello", name = "Hello", fields = {
-    @PropertyField(key = "first", name = "First"),
+    @PropertyField(key = "first", name = "First", description = "Description", options = {"A", "B"}),
     @PropertyField(key = "second", name = "Second", type = PropertyType.INTEGER)}))
   static class WithPropertySet {
   }
@@ -96,10 +96,13 @@ public class PropertyDefinitionTest {
     assertThat(def.getFields()).hasSize(2);
     assertThat(def.getFields()[0].getKey()).isEqualTo("first");
     assertThat(def.getFields()[0].getName()).isEqualTo("First");
+    assertThat(def.getFields()[0].getDescription()).isEqualTo("Description");
     assertThat(def.getFields()[0].getType()).isEqualTo(PropertyType.STRING);
+    assertThat(def.getFields()[0].getOptions()).containsOnly("A", "B");
     assertThat(def.getFields()[1].getKey()).isEqualTo("second");
     assertThat(def.getFields()[1].getName()).isEqualTo("Second");
     assertThat(def.getFields()[1].getType()).isEqualTo(PropertyType.INTEGER);
+    assertThat(def.getFields()[1].getOptions()).isEmpty();
   }
 
   @Properties(@Property(key = "hello", name = "Hello"))
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/settings_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/settings_controller.rb
index 0db50cd74b9..bbb052d4261 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/settings_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/settings_controller.rb
@@ -38,6 +38,8 @@ class SettingsController < ApplicationController
     access_denied if (@resource.nil? && !is_admin?)
 
     load_properties()
+
+    @updated_properties = {}
     save_properties(resource_id)
     save_property_sets(resource_id)
 
@@ -46,7 +48,6 @@ class SettingsController < ApplicationController
 
   private
 
-  # TODO: Validation
   def save_property_sets(resource_id)
     (params[:property_sets] || []).each do |key, value|
       set_keys = drop_trailing_blank_values(value)
@@ -57,15 +58,16 @@ class SettingsController < ApplicationController
       params[key].each do |field_key, field_values|
         field_values.each_with_index do |field_value, index|
           set_key = set_keys[index]
-          Property.set("#{key}.#{set_key}.#{field_key}", field_value, resource_id) if set_key
+          if set_key
+            field_property_key = "#{key}.#{set_key}.#{field_key}"
+            @updated_properties[field_property_key] = Property.set(field_property_key, field_value, resource_id)
+          end
         end
       end
     end
   end
 
   def save_properties(resource_id)
-    @updated_properties = {}
-
     (params[:settings] || []).each do |key, value|
       if value.kind_of? Array
         value = drop_trailing_blank_values(value)
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/helpers/settings_helper.rb b/sonar-server/src/main/webapp/WEB-INF/app/helpers/settings_helper.rb
index 3376c8807d2..6e3500af73d 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/helpers/settings_helper.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/helpers/settings_helper.rb
@@ -38,6 +38,14 @@ module SettingsHelper
     message("field.#{property.key}.#{field.key}.description", :default => field.description)
   end
 
+  def option_name(property, field, option)
+    if field
+      message("option.#{property.key}.#{field.key}.#{option}.name", :default => option)
+    else
+      message("option.#{property.key}.#{option}.name", :default => option)
+    end
+  end
+
   def property_help(property)
     message("property.#{property.key}.help", :default => '')
   end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/property.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/property.rb
index e4bec485aaf..3ebf1126a2c 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/property.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/property.rb
@@ -137,6 +137,18 @@ class Property < ActiveRecord::Base
       end
   end
 
+  def java_field_definition
+    @java_field_definition ||=
+      begin
+        if /(.*)\..*\.(.*)/.match(key)
+          property_definition = Java::OrgSonarServerUi::JRubyFacade.getInstance().getPropertyDefinitions().get(Regexp.last_match(1))
+          if property_definition
+            property_definition.fields.find { |field| field.key == Regexp.last_match(2) }
+          end
+        end
+      end
+  end
+
   def validation_error_message
     msg=''
     errors.each_full do |error|
@@ -156,9 +168,21 @@ class Property < ActiveRecord::Base
   end
 
   def validate
+    validate_property()
+    validate_field()
+  end
+
+  def validate_property
     if java_definition
       validation_result = java_definition.validate(text_value)
       errors.add_to_base(validation_result.getErrorKey()) unless validation_result.isValid()
     end
   end
+
+  def validate_field
+    if java_field_definition
+      validation_result = java_field_definition.validate(text_value)
+      errors.add_to_base(validation_result.getErrorKey()) unless validation_result.isValid()
+    end
+  end
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_error.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_error.html.erb
new file mode 100644
index 00000000000..c1f5cf0a517
--- /dev/null
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_error.html.erb
@@ -0,0 +1,7 @@
+<% if @updated_properties -%>
+  <% p = @updated_properties[key] -%>
+
+  <% if p && !p.valid? -%>
+    <div class="error"><%= p.validation_error_message -%></div>
+  <% end -%>
+<% end -%>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_properties.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_properties.html.erb
index 5d2e340b5c3..9626071b3ee 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_properties.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_properties.html.erb
@@ -40,10 +40,7 @@
               <%= render "settings/single_value", :property => property, :value => value -%>
             <% end -%>
 
-            <% p = @updated_properties[property.key] if @updated_properties -%>
-            <% if p && !p.valid? -%>
-              <div class="error"><%= p.validation_error_message -%></div>
-            <% end -%>
+            <%= render "settings/error", :key => property.key -%>
 
             <% default_prop_value = (@resource ? Property.value(property.key, nil, property.defaultValue) : property.defaultValue) -%>
             <% unless default_prop_value.blank? -%>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_set_instance.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_set_instance.html.erb
index 4cafb504ce0..bc3ad9282a8 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_set_instance.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_set_instance.html.erb
@@ -6,16 +6,19 @@
   </div>
 
   <% property.fields.each do |field| -%>
+    <% key = "#{property.key}.#{set_key}.#{field.key}" if set_key -%>
+    <% value = Property.value(key, resource_id) if set_key -%>
+
     <div class="field">
       <label><%= field_name(property, field) -%>: </label>
-
-      <% value = Property.value("#{property.key}.#{set_key}.#{field.key}", resource_id) if set_key -%>
-      <%= render "settings/type_#{field.type}", :property => field, :value => value, :name => "#{property.key}[#{field.key}][]", :id => "input_#{h field.key}" -%>
+      <%= render "settings/type_#{field.type}", :property => field, :field => field, :value => value, :name => "#{property.key}[#{field.key}][]", :id => "input_#{h field.key}" -%>
 
       <% desc=field_description(property, field) -%>
       <% unless desc.blank? %>
         <p class="note"><%= desc -%></p>
       <% end -%>
+
+      <%= render "settings/error", :key => key -%>
     </div>
   <% end -%>
   <br/>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_single_value.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_single_value.html.erb
index a08cc708b38..f9a0bcaa091 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_single_value.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_single_value.html.erb
@@ -1 +1 @@
-<%= render "settings/type_#{property_type(property, value)}", :property => property, :value => value, :name => input_name(property), :id => "input_#{h property.key}" -%>
+<%= render "settings/type_#{property_type(property, value)}", :property => property, :field => nil, :value => value, :name => input_name(property), :id => "input_#{h property.key}" -%>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_type_SINGLE_SELECT_LIST.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_type_SINGLE_SELECT_LIST.html.erb
index e2ffefee9ec..2737ec732fc 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_type_SINGLE_SELECT_LIST.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/settings/_type_SINGLE_SELECT_LIST.html.erb
@@ -1,6 +1,6 @@
 <select name="<%= name -%>" id="<%= id -%>">
   <option value=""><%= message('default') -%></option>
   <% property.options.each do |option| %>
-    <option value="<%= h option -%>" <%= 'selected' if value && value==option -%>><%= h option -%></option>
+    <option value="<%= h option -%>" <%= 'selected' if value && value==option -%>><%= option_name(property, field, option) -%></option>
   <% end %>
 </select>
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/stylesheets/style.css b/sonar-server/src/main/webapp/stylesheets/style.css
index 3eefcc481bc..9d214086666 100644
--- a/sonar-server/src/main/webapp/stylesheets/style.css
+++ b/sonar-server/src/main/webapp/stylesheets/style.css
@@ -2440,7 +2440,7 @@ textarea.width100 {
   padding-right: 5px;
 }
 
-.field .note {
+.field .note, .field .error  {
   margin-top: 3px;
-  margin-left: 90px;
+  margin-left: 88px;
 }