diff --git a/common/src/main/java/org/apache/sqoop/model/Form.java b/common/src/main/java/org/apache/sqoop/model/Form.java index 43215828..27a0831e 100644 --- a/common/src/main/java/org/apache/sqoop/model/Form.java +++ b/common/src/main/java/org/apache/sqoop/model/Form.java @@ -25,4 +25,11 @@ */ @Retention(RetentionPolicy.RUNTIME) public @interface Form { + + /** + * Optional name for the form object + * + * @return + */ + String name() default ""; } diff --git a/common/src/main/java/org/apache/sqoop/model/FormUtils.java b/common/src/main/java/org/apache/sqoop/model/FormUtils.java index 27db8af3..a829f1e4 100644 --- a/common/src/main/java/org/apache/sqoop/model/FormUtils.java +++ b/common/src/main/java/org/apache/sqoop/model/FormUtils.java @@ -17,6 +17,7 @@ */ package org.apache.sqoop.model; +import org.apache.commons.lang.StringUtils; import org.apache.sqoop.common.SqoopException; import org.apache.sqoop.utils.ClassUtils; import org.apache.sqoop.validation.Status; @@ -26,9 +27,12 @@ import java.lang.reflect.Field; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; + /** * Util class for transforming data from correctly annotated configuration @@ -56,6 +60,9 @@ public static List toForms(Class klass) { @SuppressWarnings("unchecked") public static List toForms(Class klass, Object configuration) { + + Set formNames = new HashSet(); + ConfigurationClass global = (ConfigurationClass)klass.getAnnotation(ConfigurationClass.class); @@ -68,25 +75,25 @@ public static List toForms(Class klass, Object configuration) { List forms = new LinkedList(); // Iterate over all declared fields - for (Field field : klass.getDeclaredFields()) { - field.setAccessible(true); - - String formName = field.getName(); + for (Field formField : klass.getDeclaredFields()) { + formField.setAccessible(true); // Each field that should be part of user input should have Input // annotation. - Form formAnnotation = field.getAnnotation(Form.class); + Form formAnnotation = formField.getAnnotation(Form.class); - if(formAnnotation != null) { - Class type = field.getType(); + if (formAnnotation != null) { + String formName = getFormName(formField, formAnnotation, formNames); + + Class type = formField.getType(); Object value = null; if(configuration != null) { try { - value = field.get(configuration); + value = formField.get(configuration); } catch (IllegalAccessException e) { throw new SqoopException(ModelError.MODEL_005, - "Can't retrieve value from " + field.getName(), e); + "Can't retrieve value from " + formField.getName(), e); } } @@ -98,7 +105,7 @@ public static List toForms(Class klass, Object configuration) { } @SuppressWarnings("unchecked") - private static MForm toForm(String formName, Class klass, Object object) { + private static MForm toForm(String formName, Class klass, Object object) { FormClass global = (FormClass)klass.getAnnotation(FormClass.class); @@ -125,7 +132,7 @@ private static MForm toForm(String formName, Class klass, Object object) { if(inputAnnotation != null) { boolean sensitive = inputAnnotation.sensitive(); short maxLen = inputAnnotation.size(); - Class type = field.getType(); + Class type = field.getType(); MInput input; @@ -136,19 +143,20 @@ private static MForm toForm(String formName, Class klass, Object object) { } // Instantiate corresponding MInput structure - if(type == String.class) { + if (type == String.class) { input = new MStringInput(inputName, sensitive, maxLen); } else if (type.isAssignableFrom(Map.class)) { input = new MMapInput(inputName, sensitive); - } else if(type == Integer.class) { + } else if (type == Integer.class) { input = new MIntegerInput(inputName, sensitive); - } else if(type == Boolean.class) { + } else if (type == Boolean.class) { input = new MBooleanInput(inputName, sensitive); - } else if(type.isEnum()) { - input = new MEnumInput(inputName, sensitive, ClassUtils.getEnumStrings(type)); + } else if (type.isEnum()) { + input = new MEnumInput(inputName, sensitive, + ClassUtils.getEnumStrings(type)); } else { - throw new SqoopException(ModelError.MODEL_004, - "Unsupported type " + type.getName() + " for input " + fieldName); + throw new SqoopException(ModelError.MODEL_004, "Unsupported type " + + type.getName() + " for input " + fieldName); } // Move value if it's present in original configuration object @@ -174,40 +182,57 @@ private static MForm toForm(String formName, Class klass, Object object) { return new MForm(formName, inputs); } + private static Field getFieldFromName(Class klass, String name) { + Field formField; + try { + formField = klass.getDeclaredField(name); + } catch (NoSuchFieldException e) { + // reverse lookup form field from custom form name + if (name != null) { + for (Field field : klass.getDeclaredFields()) { + Form formAnnotation = field.getAnnotation(Form.class); + if (formAnnotation == null) { + continue; + } + if (!StringUtils.isEmpty(formAnnotation.name()) && name.equals(formAnnotation.name())) { + return field; + } + } + } + throw new SqoopException(ModelError.MODEL_006, "Missing field " + name + " on form class " + + klass.getCanonicalName(), e); + } + return formField; + } + /** * Move form values from form list into corresponding configuration object. * - * @param forms Input form list - * @param configuration Output configuration object + * @param forms + * Input form list + * @param configuration + * Output configuration object */ public static void fromForms(List forms, Object configuration) { - Class klass = configuration.getClass(); - - for(MForm form : forms) { - Field formField; - try { - formField = klass.getDeclaredField(form.getName()); - } catch (NoSuchFieldException e) { - throw new SqoopException(ModelError.MODEL_006, - "Missing field " + form.getName() + " on form class " + klass.getCanonicalName(), e); - } + Class klass = configuration.getClass(); + for (MForm form : forms) { + Field formField = getFieldFromName(klass, form.getName()); // We need to access this field even if it would be declared as private formField.setAccessible(true); - - Class formClass = formField.getType(); + Class formClass = formField.getType(); Object newValue = ClassUtils.instantiate(formClass); - if(newValue == null) { + if (newValue == null) { throw new SqoopException(ModelError.MODEL_006, - "Can't instantiate new form " + formClass); + "Can't instantiate new form " + formClass); } - for(MInput input : form.getInputs()) { + for (MInput input : form.getInputs()) { String[] splitNames = input.getName().split("\\."); - if(splitNames.length != 2) { - throw new SqoopException(ModelError.MODEL_009, - "Invalid name: " + input.getName()); + if (splitNames.length != 2) { + throw new SqoopException(ModelError.MODEL_009, "Invalid name: " + + input.getName()); } String inputName = splitNames[1]; @@ -216,34 +241,36 @@ public static void fromForms(List forms, Object configuration) { try { inputField = formClass.getDeclaredField(inputName); } catch (NoSuchFieldException e) { - throw new SqoopException(ModelError.MODEL_006, - "Missing field " + input.getName(), e); + throw new SqoopException(ModelError.MODEL_006, "Missing field " + + input.getName(), e); } // We need to access this field even if it would be declared as private inputField.setAccessible(true); try { - if(input.isEmpty()) { + if (input.isEmpty()) { inputField.set(newValue, null); } else { if (input.getType() == MInputType.ENUM) { - inputField.set(newValue, Enum.valueOf((Class)inputField.getType(), (String) input.getValue())); + inputField.set(newValue, Enum.valueOf( + (Class) inputField.getType(), + (String) input.getValue())); } else { inputField.set(newValue, input.getValue()); } } } catch (IllegalAccessException e) { - throw new SqoopException(ModelError.MODEL_005, - "Issue with field " + inputField.getName(), e); + throw new SqoopException(ModelError.MODEL_005, "Issue with field " + + inputField.getName(), e); } } try { formField.set(configuration, newValue); } catch (IllegalAccessException e) { - throw new SqoopException(ModelError.MODEL_005, - "Issue with field " + formField.getName(), e); + throw new SqoopException(ModelError.MODEL_005, "Issue with field " + + formField.getName(), e); } } } @@ -251,16 +278,19 @@ public static void fromForms(List forms, Object configuration) { /** * Apply validations on the forms. * - * @param forms Forms that should be updated - * @param validation Validation that we should apply + * @param forms + * Forms that should be updated + * @param validation + * Validation that we should apply */ public static void applyValidation(List forms, Validation validation) { - Map messages = validation.getMessages(); + Map messages = validation + .getMessages(); - for(MForm form : forms) { + for (MForm form : forms) { applyValidation(form, messages); - for(MInput input : form.getInputs()) { + for (MInput input : form.getInputs()) { applyValidation(input, messages); } } @@ -269,13 +299,16 @@ public static void applyValidation(List forms, Validation validation) { /** * Apply validation on given validated element. * - * @param element Element on what we're applying the validations - * @param messages Map of all validation messages + * @param element + * Element on what we're applying the validations + * @param messages + * Map of all validation messages */ - public static void applyValidation(MValidatedElement element, Map messages) { + public static void applyValidation(MValidatedElement element, + Map messages) { Validation.FormInput name = new Validation.FormInput(element.getName()); - if(messages.containsKey(name)) { + if (messages.containsKey(name)) { Validation.Message message = messages.get(name); element.setValidationMessage(message.getStatus(), message.getMessage()); } else { @@ -293,14 +326,14 @@ public static void applyValidation(MValidatedElement element, Map formNames = new HashSet(); + ConfigurationClass global = (ConfigurationClass) klass + .getAnnotation(ConfigurationClass.class); // Each configuration object must have this class annotation - if(global == null) { + if (global == null) { throw new SqoopException(ModelError.MODEL_003, - "Missing annotation Configuration on class " + klass.getName()); + "Missing annotation Configuration on class " + klass.getName()); } JSONObject jsonOutput = new JSONObject(); @@ -308,26 +341,26 @@ public static String toJson(Object configuration) { // Iterate over all declared fields for (Field formField : klass.getDeclaredFields()) { formField.setAccessible(true); - String formName = formField.getName(); // We're processing only form validations Form formAnnotation = formField.getAnnotation(Form.class); - if(formAnnotation == null) { + if (formAnnotation == null) { continue; } + String formName = getFormName(formField, formAnnotation, formNames); Object formValue; try { formValue = formField.get(configuration); } catch (IllegalAccessException e) { - throw new SqoopException(ModelError.MODEL_005, - "Issue with field " + formName, e); + throw new SqoopException(ModelError.MODEL_005, "Issue with field " + + formName, e); } JSONObject jsonForm = new JSONObject(); // Now process each input on the form - for(Field inputField : formField.getType().getDeclaredFields()) { + for (Field inputField : formField.getType().getDeclaredFields()) { inputField.setAccessible(true); String inputName = inputField.getName(); @@ -335,46 +368,45 @@ public static String toJson(Object configuration) { try { value = inputField.get(formValue); } catch (IllegalAccessException e) { - throw new SqoopException(ModelError.MODEL_005, - "Issue with field " + formName + "." + inputName, e); + throw new SqoopException(ModelError.MODEL_005, "Issue with field " + + formName + "." + inputName, e); } Input inputAnnotation = inputField.getAnnotation(Input.class); // Do not serialize all values - if(inputAnnotation != null && value != null) { - Class type = inputField.getType(); + if (inputAnnotation != null && value != null) { + Class type = inputField.getType(); // We need to support NULL, so we do not support primitive types - if(type.isPrimitive()) { + if (type.isPrimitive()) { throw new SqoopException(ModelError.MODEL_007, - "Detected primitive type " + type + " for field " + formName + "." + inputName); + "Detected primitive type " + type + " for field " + formName + + "." + inputName); } - if(type == String.class) { + if (type == String.class) { jsonForm.put(inputName, value); } else if (type.isAssignableFrom(Map.class)) { JSONObject map = new JSONObject(); - for(Object key : ((Map)value).keySet()) { - map.put(key, ((Map)value).get(key)); + for (Object key : ((Map) value).keySet()) { + map.put(key, ((Map) value).get(key)); } jsonForm.put(inputName, map); - } else if(type == Integer.class) { + } else if (type == Integer.class) { jsonForm.put(inputName, value); - } else if(type.isEnum()) { + } else if (type.isEnum()) { jsonForm.put(inputName, value.toString()); - } else if(type == Boolean.class) { + } else if (type == Boolean.class) { jsonForm.put(inputName, value); - }else { - throw new SqoopException(ModelError.MODEL_004, - "Unsupported type " + type.getName() + " for input " + formName + "." + inputName); + } else { + throw new SqoopException(ModelError.MODEL_004, "Unsupported type " + + type.getName() + " for input " + formName + "." + inputName); } } } - jsonOutput.put(formName, jsonForm); } - return jsonOutput.toJSONString(); } @@ -389,16 +421,17 @@ public static void fillValues(String json, Object configuration) { Class klass = configuration.getClass(); JSONObject jsonForms = (JSONObject) JSONValue.parse(json); + Set formNames = new HashSet(); for(Field formField : klass.getDeclaredFields()) { formField.setAccessible(true); - String formName = formField.getName(); // We're processing only form validations Form formAnnotation = formField.getAnnotation(Form.class); if(formAnnotation == null) { continue; } + String formName = getFormName(formField, formAnnotation, formNames); try { formField.set(configuration, formField.getType().newInstance()); @@ -407,7 +440,7 @@ public static void fillValues(String json, Object configuration) { "Issue with field " + formName, e); } - JSONObject jsonInputs = (JSONObject) jsonForms.get(formField.getName()); + JSONObject jsonInputs = (JSONObject) jsonForms.get(formName); if(jsonInputs == null) { continue; } @@ -466,4 +499,40 @@ public static void fillValues(String json, Object configuration) { } } -} + private static String getFormName(Field member, Form annotation, Set existingFormNames) { + if (StringUtils.isEmpty(annotation.name())) { + return member.getName(); + } else { + checkForValidFormName(existingFormNames, annotation.name()); + existingFormNames.add(annotation.name()); + return annotation.name(); + } + } + + private static void checkForValidFormName(Set existingFormNames, + String customFormName) { + // uniqueness across fields check + if (existingFormNames.contains(customFormName)) { + throw new SqoopException(ModelError.MODEL_012, + "Issue with field form name " + customFormName); + } + + if (!Character.isJavaIdentifierStart(customFormName.toCharArray()[0])) { + throw new SqoopException(ModelError.MODEL_013, + "Issue with field form name " + customFormName); + } + for (Character c : customFormName.toCharArray()) { + if (Character.isJavaIdentifierPart(c)) + continue; + throw new SqoopException(ModelError.MODEL_013, + "Issue with field form name " + customFormName); + } + + if (customFormName.length() > 30) { + throw new SqoopException(ModelError.MODEL_014, + "Issue with field form name " + customFormName); + + } + } + +} \ No newline at end of file diff --git a/common/src/main/java/org/apache/sqoop/model/ModelError.java b/common/src/main/java/org/apache/sqoop/model/ModelError.java index 1f466fe8..86b626e3 100644 --- a/common/src/main/java/org/apache/sqoop/model/ModelError.java +++ b/common/src/main/java/org/apache/sqoop/model/ModelError.java @@ -46,7 +46,12 @@ public enum ModelError implements ErrorCode { MODEL_011("Input do not exist"), - ; + MODEL_012("Form name attribute should be unique across a configuration object"), + + MODEL_013("Form name attribute should not contain unsupported characters"), + + MODEL_014("Form name attribute cannot be more than 30 characters long") +; private final String message; diff --git a/common/src/test/java/org/apache/sqoop/model/TestFormUtils.java b/common/src/test/java/org/apache/sqoop/model/TestFormUtils.java index 08dfa7ba..6747c813 100644 --- a/common/src/test/java/org/apache/sqoop/model/TestFormUtils.java +++ b/common/src/test/java/org/apache/sqoop/model/TestFormUtils.java @@ -33,24 +33,24 @@ public class TestFormUtils extends TestCase { public void testToForms() { - Config config = new Config(); - config.aForm.a1 = "value"; + Configuration Configuration = new Configuration(); + Configuration.aForm.a1 = "value"; - List formsByInstance = FormUtils.toForms(config); + List formsByInstance = FormUtils.toForms(Configuration); assertEquals(getForms(), formsByInstance); assertEquals("value", formsByInstance.get(0).getInputs().get(0).getValue()); - List formsByClass = FormUtils.toForms(Config.class); + List formsByClass = FormUtils.toForms(Configuration.class); assertEquals(getForms(), formsByClass); - List formsByBoth = FormUtils.toForms(Config.class, config); + List formsByBoth = FormUtils.toForms(Configuration.class, Configuration); assertEquals(getForms(), formsByBoth); assertEquals("value", formsByBoth.get(0).getInputs().get(0).getValue()); } public void testToFormsMissingAnnotation() { try { - FormUtils.toForms(ConfigWithout.class); + FormUtils.toForms(ConfigurationWithoutAnnotation.class); } catch(SqoopException ex) { assertEquals(ModelError.MODEL_003, ex.getErrorCode()); return; @@ -59,11 +59,43 @@ public void testToFormsMissingAnnotation() { fail("Correct exception wasn't thrown"); } + public void testNonUniqueFormNameAttributes() { + try { + FormUtils.toForms(ConfigurationWithNonUniqueFormNameAttribute.class); + } catch (SqoopException ex) { + assertEquals(ModelError.MODEL_012, ex.getErrorCode()); + return; + } + + fail("Correct exception wasn't thrown"); + } + + public void testInvalidFormNameAttribute() { + try { + FormUtils.toForms(ConfigurationWithInvalidFormNameAttribute.class); + } catch (SqoopException ex) { + assertEquals(ModelError.MODEL_013, ex.getErrorCode()); + return; + } + + fail("Correct exception wasn't thrown"); + } + + public void testInvalidFormNameAttributeLength() { + try { + FormUtils.toForms(ConfigurationWithInvalidFormNameAttributeLength.class); + } catch (SqoopException ex) { + assertEquals(ModelError.MODEL_014, ex.getErrorCode()); + return; + } + fail("Correct exception wasn't thrown"); + } + public void testFailureOnPrimitiveType() { - PrimitiveConfig config = new PrimitiveConfig(); + PrimitiveConfiguration Configuration = new PrimitiveConfiguration(); try { - FormUtils.toForms(config); + FormUtils.toForms(Configuration); fail("We were expecting exception for unsupported type."); } catch(SqoopException ex) { assertEquals(ModelError.MODEL_007, ex.getErrorCode()); @@ -72,13 +104,16 @@ public void testFailureOnPrimitiveType() { public void testFillValues() { List forms = getForms(); + assertEquals("test_AForm", forms.get(0).getName()); + assertEquals("test_BForm", forms.get(1).getName()); + assertEquals("cForm", forms.get(2).getName()); ((MStringInput)forms.get(0).getInputs().get(0)).setValue("value"); - Config config = new Config(); + Configuration Configuration = new Configuration(); - FormUtils.fromForms(forms, config); - assertEquals("value", config.aForm.a1); + FormUtils.fromForms(forms, Configuration); + assertEquals("value", Configuration.aForm.a1); } public void testFillValuesObjectReuse() { @@ -86,15 +121,15 @@ public void testFillValuesObjectReuse() { ((MStringInput)forms.get(0).getInputs().get(0)).setValue("value"); - Config config = new Config(); - config.aForm.a2 = "x"; - config.bForm.b1 = "y"; + Configuration Configuration = new Configuration(); + Configuration.aForm.a2 = "x"; + Configuration.bForm.b1 = "y"; - FormUtils.fromForms(forms, config); - assertEquals("value", config.aForm.a1); - assertNull(config.aForm.a2); - assertNull(config.bForm.b2); - assertNull(config.bForm.b2); + FormUtils.fromForms(forms, Configuration); + assertEquals("value", Configuration.aForm.a1); + assertNull(Configuration.aForm.a2); + assertNull(Configuration.bForm.b2); + assertNull(Configuration.bForm.b2); } public void testApplyValidation() { @@ -115,16 +150,16 @@ public void testApplyValidation() { } public void testJson() { - Config config = new Config(); - config.aForm.a1 = "A"; - config.bForm.b2 = "B"; - config.cForm.intValue = 4; - config.cForm.map.put("C", "D"); - config.cForm.enumeration = Enumeration.X; + Configuration Configuration = new Configuration(); + Configuration.aForm.a1 = "A"; + Configuration.bForm.b2 = "B"; + Configuration.cForm.intValue = 4; + Configuration.cForm.map.put("C", "D"); + Configuration.cForm.enumeration = Enumeration.X; - String json = FormUtils.toJson(config); + String json = FormUtils.toJson(Configuration); - Config targetConfig = new Config(); + Configuration targetConfig = new Configuration(); // Old values from should be always removed targetConfig.aForm.a2 = "X"; @@ -152,17 +187,17 @@ protected Validation getValidation() { = new HashMap(); messages.put( - new Validation.FormInput("aForm", "a1"), + new Validation.FormInput("test_AForm", "a1"), new Validation.Message(Status.ACCEPTABLE, "e1")); messages.put( - new Validation.FormInput("aForm", "a2"), + new Validation.FormInput("test_AForm", "a2"), new Validation.Message(Status.UNACCEPTABLE, "e2")); return new Validation(Status.UNACCEPTABLE, messages); } /** - * Form structure that corresponds to Config class declared below + * Form structure that corresponds to Configuration class declared below * @return Form structure */ protected List getForms() { @@ -172,15 +207,15 @@ protected List getForms() { // Form A inputs = new LinkedList>(); - inputs.add(new MStringInput("aForm.a1", false, (short)30)); - inputs.add(new MStringInput("aForm.a2", true, (short)-1)); - ret.add(new MForm("aForm", inputs)); + inputs.add(new MStringInput("test_AForm.a1", false, (short)30)); + inputs.add(new MStringInput("test_AForm.a2", true, (short)-1)); + ret.add(new MForm("test_AForm", inputs)); // Form B inputs = new LinkedList>(); - inputs.add(new MStringInput("bForm.b1", false, (short)2)); - inputs.add(new MStringInput("bForm.b2", false, (short)3)); - ret.add(new MForm("bForm", inputs)); + inputs.add(new MStringInput("test_BForm.b1", false, (short)2)); + inputs.add(new MStringInput("test_BForm.b2", false, (short)3)); + ret.add(new MForm("test_BForm", inputs)); // Form C inputs = new LinkedList>(); @@ -193,21 +228,57 @@ protected List getForms() { } @ConfigurationClass - public static class Config { + public static class ConfigurationWithNonUniqueFormNameAttribute { + public ConfigurationWithNonUniqueFormNameAttribute() { + aForm = new InvalidForm(); + bForm = new InvalidForm(); + } - public Config() { + @Form(name = "sameName") + InvalidForm aForm; + @Form(name = "sameName") + InvalidForm bForm; + } + + @ConfigurationClass + public static class ConfigurationWithInvalidFormNameAttribute { + public ConfigurationWithInvalidFormNameAttribute() { + invalidForm = new InvalidForm(); + } + + @Form(name = "#_form") + InvalidForm invalidForm; + } + + @ConfigurationClass + public static class ConfigurationWithInvalidFormNameAttributeLength { + public ConfigurationWithInvalidFormNameAttributeLength() { + invalidLengthForm = new InvalidForm(); + } + + @Form(name = "longest_form_more_than_30_characers") + InvalidForm invalidLengthForm; + } + + @ConfigurationClass + public static class Configuration { + + public Configuration() { aForm = new AForm(); bForm = new BForm(); cForm = new CForm(); } - @Form AForm aForm; - @Form BForm bForm; - @Form CForm cForm; + @Form(name = "test_AForm") + AForm aForm; + @Form(name = "test_BForm") + BForm bForm; + @Form(name = "") + CForm cForm; } @ConfigurationClass - public static class PrimitiveConfig { + public static class PrimitiveConfiguration { @Form DForm dForm; } @@ -234,12 +305,16 @@ public CForm() { } } + @FormClass + public static class InvalidForm { + + } @FormClass public static class DForm { @Input int value; } - public static class ConfigWithout { + public static class ConfigurationWithoutAnnotation { } enum Enumeration {