[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

[Michael Wechner]

Guillaume Déflache schrieb:
> Hi!
>>
>> -
>>  /**
>> - *
>> + * 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.

right and I admit it was a mistake not to discuss this beforehand.

The reason I did it was because I have discovered that we have this

<realm-config class=""/>

attribute which allows to overwrite the realm implementation, but it 
wasn't documented at all and
AFAIK the only realm using this is the FOAF realm, so I thought I better 
take action now than wait

Btw, in order to upgrade one has to extend now RealmDefaultImpl.class 
instead Realm.class
>
>
>> [...]
>>      public static String DEFAULT_REPOSITORY_FACTORY_BEAN_ID = 
>> "DefaultRepositoryFactory";
>
> Not sure this should be public.

I think not, but it's used at several places and need to think how to 
get best rid of it ;-)

well, I have currently removed some of them, but there are more. Let me 
check how we can best do this ...
>
>
>
>> [...]
>>      /**
>>       * 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?

yes, whereas with something like export/import/copy methods we could 
probably get rid of it
>
>>
>>  
>>      /**
>>       *
>>       */
>> -    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?

not as far as I know, but yes it would be nice to have such an object
>
>
>> +    public String toString();
>
> This is redundant BTW, all objects have that.

ok, will remove it
>
>
>
>>  [...]
>>      /**
>>       * 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?

I need to check this
>
>
>>      /**
>>       * 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).

I think there should always be at least one language
>
>
>
> Again nice, all of these are already interfaces! :)
>
> I think we may now officially be able to mock realms! Thanks Michi!

I hope so :-)

>
>
>> [...]
>> 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!

It's contained within Realm.java, but agreed copying would have been nicer.

Cheers

Michi
>
> Cheers,
>    Guillaume