[Yanel-dev] Re: [Yanel-commits] rev 44104 - in public/yanel/trunk/src: core/java/org/wyona/yanel/core/map test/junit/org/wyona/yanel/core/util

[Guillaume Déflache]

Hi!

michi at wyona.com schrieb:
> Author: michi
> Date: 2009-08-10 16:11:08 +0200 (Mon, 10 Aug 2009)
> New Revision: 44104
> 
> Added:
>    public/yanel/trunk/src/core/java/org/wyona/yanel/core/map/RealmDefaultImpl.java
> Modified:
>    public/yanel/trunk/src/core/java/org/wyona/yanel/core/map/Realm.java
>    public/yanel/trunk/src/core/java/org/wyona/yanel/core/map/RealmManager.java
>    public/yanel/trunk/src/test/junit/org/wyona/yanel/core/util/PathUtilTest.java
> Log:
> realm interface introduced
> 
> Modified: public/yanel/trunk/src/core/java/org/wyona/yanel/core/map/Realm.java
> ===================================================================
> --- public/yanel/trunk/src/core/java/org/wyona/yanel/core/map/Realm.java	2009-08-10 13:41:39 UTC (rev 44103)
> +++ public/yanel/trunk/src/core/java/org/wyona/yanel/core/map/Realm.java	2009-08-10 14:11:08 UTC (rev 44104)
> @@ -39,522 +39,170 @@
>  import org.apache.avalon.framework.configuration.ConfigurationException;
>  import org.apache.avalon.framework.configuration.DefaultConfigurationBuilder;
>  
> -import org.apache.log4j.Logger;
> -
>  /**
> - *
> + * Realm interface
>   */
> -public class Realm {
> +public interface Realm {

Now we broke the API (a bit anyway), maybe we could try to remove some 
more dependencies as well. Here are some thoughts.


> [...]
>      public static String DEFAULT_REPOSITORY_FACTORY_BEAN_ID = "DefaultRepositoryFactory";

Not sure this should be public.


> [...]
> +    //public Realm(String name, String id, String mountPoint, File configFile) throws Exception;
> [...]
> -    protected void configure(Configuration config) throws Exception {
> -        Yanel yanel = Yanel.getInstance();
> -	File repoConfig = null;
> +    //protected void configure(Configuration config) throws Exception;

Nice move for testing, we do not depend on files anymore...


> [...]
>      /**
>       * Configuration file of realm.
>       */
> -    public File getConfigFile() {
> -        return configFile;
> -    }
> +    public File getConfigFile();

...well, almost: I wondered how heavily we depend on that. I think we 
mostly use that to get the path of the realm of the filesystem, don't we?


>  [...]
>      /**
>       *
>       */
> -    public void setProxy(String hostName, int port, int sslPort, String prefix) {
> -        proxySet = true;
> -        proxyHostName = hostName;
> -        proxyPort = port;
> -        proxySSLPort = sslPort;
> -        proxyPrefix = prefix;
> -    }
> +    public void setProxy(String hostName, int port, int sslPort, String prefix);
>  
>      /**
>       *
>       */
> -    public boolean isProxySet() {
> -        return proxySet;
> -    }
> +    public boolean isProxySet();
>  
>      /**
>       *
>       */
> -    public String getProxyHostName() {
> -        return proxyHostName;
> -    }
> +    public String getProxyHostName();
>  
>      /**
>       *
>       */
> -    public int getProxyPort() {
> -        return proxyPort;
> -    }
> +    public int getProxyPort();
>  
>      /**
>       *
>       */
> -    public int getProxySSLPort() {
> -        return proxySSLPort;
> -    }
> +    public int getProxySSLPort();
>  
>      /**
>       *
>       */
> -    public String getProxyPrefix() {
> -        return proxyPrefix;
> -    }
> +    public String getProxyPrefix();

I remember thinking it would be nice to have a proxy configuration 
object, because (IIRC) we also use that info for Neutron authentication 
(cf. http://bugzilla.wyona.com/cgi-bin/bugzilla/show_bug.cgi?id=4964 
whereas I did not add a comment there but I have some notes or at least 
code about it somewhere...), and defining the configuration there ties 
us to Yanel via the realm interface. Maybe there is some standard API 
out there?


> +    public String toString();

This is redundant BTW, all objects have that.


>  
>      /**
>       * Get data repository of realm
>       */
> -    public Repository getRepository() throws Exception {
> -        return repository;
> -    }
> +    public Repository getRepository() throws Exception;
>  
> -    public void setRepository(Repository repository) throws Exception {
> -        this.repository = repository;
> -    }
> +    public void setRepository(Repository repository) throws Exception;
>  
>      /**
>       * Get RTI (Resource Type Identifier) repository of realm
>       */
> -    public Repository getRTIRepository() throws Exception {
> -        return rtiRepository;
> -    }
> +    public Repository getRTIRepository() throws Exception;
>  
> -    public void setRTIRepository(Repository repository) throws Exception {
> -        this.rtiRepository = repository;
> -    }
> +    public void setRTIRepository(Repository repository) throws Exception;
>  
>      /**
>       *
>       */
> -    public WebAuthenticator getWebAuthenticator() {
> -        return privateWebAuthenticator;
> -    }
> +    public WebAuthenticator getWebAuthenticator();
> +
>      /**
>       *
>       */
> -    public void setWebAuthenticator(WebAuthenticator wa) {
> -        privateWebAuthenticator = wa;
> -    }
> +    public void setWebAuthenticator(WebAuthenticator wa);
>  
> -    public IdentityManager getIdentityManager() {
> -        return privateIdentityManager;
> -    }
> +    public IdentityManager getIdentityManager();
>  
> -    public void setIdentityManager(IdentityManager identityManager) {
> -        this.privateIdentityManager = identityManager;
> -    }
> +    public void setIdentityManager(IdentityManager identityManager);
>  
>      /**
>       * Get policy manager
>       */
> -    public PolicyManager getPolicyManager() {
> -        return privatePolicyManager;
> -    }
> +    public PolicyManager getPolicyManager();
>  
> -    public void setPolicyManager(PolicyManager policyManager) {
> -        this.privatePolicyManager = policyManager;
> -    }
> +    public void setPolicyManager(PolicyManager policyManager);
>  
>      /**
>       * Get repository navigation
>       */
> -    public Sitetree getRepoNavigation() {
> -        return repoNavigation;
> -    }
> +    public Sitetree getRepoNavigation();

Nice, all of these are already interfaces! :)


>  [...]
>      /**
>       * Please note that the menu element is optional
>       */
> -    public String getMenuClass() {
> -        try {
> -            Configuration realmConfig = new DefaultConfigurationBuilder().buildFromFile(getConfigFile());
> -            Configuration menuClassConfig = realmConfig.getChild("menu", false);
> -            if (menuClassConfig != null) {
> -                return menuClassConfig.getAttribute("class");
> -            }
> -        } catch (Exception e) {
> -            log.error(e.getMessage(), e);
> -        }
> -        return null;
> -    }
> +    public String getMenuClass();

Just wondering why we do not return an instanciated menu class instead 
(with fallback to org.wyona.yanel.servlet.menu.impl.DefaultMenu) here?


>      /**
>       * Gets a list of all languages supported by this realm.
>       * @return list of languages. may be empty.
>       */
> -    public String[] getLanguages() {
> -        return languages;
> -    }
> +    public String[] getLanguages();

Maybe we should add if it can ever be null (I suppose not).


> [...]
> -    public TranslationManager getTranslationManager() {
> -        //log.debug("Translation Manager: " + translationManager.getClass().getName());
> -        return translationManager;
> -    }
> +    public TranslationManager getTranslationManager();
>  
> -    public void setTranslationManager(TranslationManager translationManager) {
> -        this.translationManager = translationManager;
> -    }
> +    public void setTranslationManager(TranslationManager translationManager);
>  
> -    public Repository getRepository(String id) throws Exception {
> -        Yanel yanel = Yanel.getInstance();
> -        RepositoryFactory extraRepoFactory = yanel.getRepositoryFactory("ExtraRepositoryFactory");
> -        if (extraRepoFactory.exists(id)) {
> -            return extraRepoFactory.newRepository(id);
> -        } 
> -        return null;
> -    }
> +    public Repository getRepository(String id) throws Exception;
>  
> -    public LanguageHandler getLanguageHandler() {
> -        return languageHandler;
> -    }
> +    public LanguageHandler getLanguageHandler();
>  
> -    public void setLanguageHandler(LanguageHandler languageHandler) {
> -        this.languageHandler = languageHandler;
> -    }
> +    public void setLanguageHandler(LanguageHandler languageHandler);

Again nice, all of these are already interfaces! :)

I think we may now officially be able to mock realms! Thanks Michi!


> [...]
> Added: public/yanel/trunk/src/core/java/org/wyona/yanel/core/map/RealmDefaultImpl.java
> ===================================================================
> --- public/yanel/trunk/src/core/java/org/wyona/yanel/core/map/RealmDefaultImpl.java	                        (rev 0)
> +++ public/yanel/trunk/src/core/java/org/wyona/yanel/core/map/RealmDefaultImpl.java	2009-08-10 14:11:08 UTC (rev 44104)
> @@ -0,0 +1,560 @@
 > [...]

Too bad you did not use `svn cp` here, as we then lost the history! :( I 
assume nothing significant changed anyway!

Cheers,
    Guillaume