[ovirt-devel] Custom Properties code duplication

[Yair Zaslavsky]

----- Original Message -----
> From: "Lior Vernia" <lvernia at redhat.com>
> To: devel at ovirt.org
> Sent: Wednesday, April 30, 2014 3:52:16 PM
> Subject: [ovirt-devel] Custom Properties code duplication
> 
> Hello,
> 
> While adding network custom properties towards oVirt 3.5, I got to take
> a good look at the custom properties code in the backend and frontend.
> It seems to me like there's a lot of code duplication, and I would like
> to suggest the following refactoring:
> 
> 1. Remove dependencies on Pattern/Matcher and ApacheCommons methods from
> *CustomPropertiesUtils.java, to make them compilable with GWT, and move
> these utilities to the common package. The usage of the said methods is
> minimal and could easily be replaced with String methods, etc.


In general I am in favor, but how are you going to perform the regex validation of values?
for example , with vm custom properties, you have - sap_agent that can be either true or false.
So you need to validate both at the client and the engine, right? 

> 
> 2. Modify KeyValueModel to delegate to the common utilities instead of
> duplicating code, e.g. for validation.
> 
> 3. Move some validation, which is relevant to all custom properties
> (e.g. duplicate keys), from specific utils (e.g. VmPropertiesUtils) up
> to the shared CustomPropertiesUtils.
> 
> 4. Optionally change the implementation of custom properties members in
> entities (e.g. VM) from String to Map<String, String>, which would
> obviate the need for different conversion methods between String/Map -
> (de)serialization would only be required in DB interaction.

3,4 Agreed - good points.

> 
> The main argument against this would be that the frontend is to be moved
> over the REST, and might not be written in Java much longer anyway.
> 
> However, to my understanding, there's some time until these changes take
> effect. And even if the frontend is to be written in JavaScript, at
> least initially the existing frontend code will have to still be used
> somehow (e.g. auto-translated to JavaScript). That is to say, this
> refactoring might still be beneficial for the not-so-short term.

I agree with you here.




> 
> Before going through with this, I wanted to ask for your thoughts and to
> hear any specific objections to the proposed changes.
> 
> Yours, Lior.
> _______________________________________________
> Devel mailing list
> Devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>