db-jdo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Zaun <Martin.Z...@Sun.COM>
Subject Re: status update: ri11 enhancer support for jdk 1.5
Date Thu, 29 Sep 2005 12:35:21 GMT

Hi Michael,
thanks for the patch, only a few cosmetic comments inline, below.
I'll send out a summary of our discussion on the jdk 1.5 class
registration problem and the chosen approach later.
Martin

Michael Bouschen wrote:
> Hi Martin,
> 
> attached you find a patch for the JDOModel implementation. It 
> initializes a class instance if the model instance runs with the 
> initialize=true flag. It also fixes the MissingResourceException. I 
> could successfully run the ri tests in a workspace including your 
> enhancer plus my JDOModel changes.
> I would port the JDOModel changes to core20 and check them in, if you 
> agree with the changes.
> 
> Regards Michael
> 
>>
>> Michael,
>> attached is a stable subset of my ri11 enhancer
>> changes that produce the problem with Test_Query.
>> Unzip in trunk and run 'maven build' from ri11.
>> Talk to you next week,
>> Martin
> 
> 
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Index: trunk/ri11/src/java/org/apache/jdo/impl/model/java/runtime/Bundle.properties
> ===================================================================
> --- trunk/ri11/src/java/org/apache/jdo/impl/model/java/runtime/Bundle.properties	(Revision
292403)
> +++ trunk/ri11/src/java/org/apache/jdo/impl/model/java/runtime/Bundle.properties	(Arbeitskopie)
> @@ -62,12 +62,3 @@
>  #NOI18N
>  ERR_CannotSetJDOModel=Cannot set JDOModel for JavaModel instance.
>  
> -#
> -# RuntimeJavaType
> -#
> -
> -# {0} - error location (class.method)
> -# {1} - implementation method name
> -# {2} - field name
> -#NOI18N
> -ERR_MultipleJavaField={0}: multiple JavaField ''{1}'' for class '{2}''.
> Index: trunk/ri11/src/java/org/apache/jdo/impl/model/java/reflection/ReflectionJavaModel.java
> ===================================================================
> --- trunk/ri11/src/java/org/apache/jdo/impl/model/java/reflection/ReflectionJavaModel.java
(Revision 292403)
> +++ trunk/ri11/src/java/org/apache/jdo/impl/model/java/reflection/ReflectionJavaModel.java
(Arbeitskopie)
> @@ -87,8 +87,8 @@
>                      // loaded, Class.forName will load the class which
>                      // calls RegisterClassListener.registerClass.
>                      // This will create a new JavaType entry in the cache.
> -                    javaType = getJavaType(Class.forName(name, initialize, 
> -                                                         classLoader));
> +                    javaType = getJavaTypeInternal(
> +                        Class.forName(name, initialize, classLoader));
>                  }
>                  catch (ClassNotFoundException ex) {
>                      // cannot find class => return null
> @@ -121,15 +121,20 @@
>       */
>      public JavaType getJavaType(Class clazz)
>      {
> -        String name = clazz.getName();
> -        synchronized (types) {
> -            JavaType javaType = (JavaType)types.get(name);
> -            if (javaType == null) {
> -                javaType = createJavaType(clazz);
> -                types.put(name, javaType);
> +        if (clazz == null)
> +            return null;
> +        
> +        if (initialize) {
> +            try {
> +                // make sure the class is initialized
> +                Class.forName(clazz.getName(), initialize, classLoader);

Since this is a public method, there's a chance for a bug that we're
called here with a class argument of a wrong classloader, I think.
If that happened, we'd load and initialize the class in the wrong
place.  So, I'd change this line, making an assumption explicit, to:
                 Class.forName(clazz.getName(), initialize, clazz.getClassLoader());
Or, if we allowed for assertions, we could keep the line but just add:
                 assert (classLoader == clazz.getClassLoader());

>              }
> -            return javaType;
> +            catch (ClassNotFoundException ex) {
> +                // ignore

I'd add:
                // ignore, since class has already been loaded

> +            }
>          }
> +
> +        return getJavaTypeInternal(clazz);
>      }
>  
>      /**
> @@ -170,6 +175,28 @@
>          return classLoader;
>      }
>  
> +    /**
> +     * The method returns the JavaType instance for the type name of the
> +     * specified class object. It first checks the cache and if there is no
> +     * entry for the type name in the cache then it creates a new JavaType
> +     * instance for the specified Class object.
> +     * @param clazz the Class instance representing the type
> +     * @return a JavaType instance for the name of the specified class
> +     * object or <code>null</code> if not present in this model instance.
> +     */
> +    protected JavaType getJavaTypeInternal(Class clazz)
> +    {
> +        String name = clazz.getName();
> +        synchronized (types) {
> +            JavaType javaType = (JavaType)types.get(name);
> +            if (javaType == null) {
> +                javaType = createJavaType(clazz);
> +                types.put(name, javaType);
> +            }
> +            return javaType;
> +        }
> +    }
> +
>      /** 
>       * Creates a new JavaType instance for the specified Class object.
>       * This method provides a hook such that ReflectionJavaModel subclasses
> Index: trunk/ri11/src/java/org/apache/jdo/impl/model/java/Bundle.properties
> ===================================================================
> --- trunk/ri11/src/java/org/apache/jdo/impl/model/java/Bundle.properties	(Revision 292403)
> +++ trunk/ri11/src/java/org/apache/jdo/impl/model/java/Bundle.properties	(Arbeitskopie)
> @@ -27,7 +27,7 @@
>  EXC_ClassLoadingError=Error during loading of class ''{0}'': {1}.
>  
>  #
> -# ReflectionJavaType
> +# BaseReflectionJavaType
>  #
>  
>  # {0} - error location (class.method)
> @@ -35,6 +35,16 @@
>  ERR_InvalidNullClassInstance={0}: specified Class instance is null.
>  
>  #
> +# ReflectionJavaType
> +#
> +
> +# {0} - error location (class.method)
> +# {1} - implementation method name
> +# {2} - field name
> +#NOI18N
> +ERR_MultipleJavaField={0}: multiple JavaField ''{1}'' for class ''{2}''.
> +
> +#
>  # BaseReflectionJavaField
>  #
>  # {0} - class name


Mime
View raw message