6 Replies Latest reply on Dec 21, 2007 12:23 PM by starksm64

    ProfileServiceUnitTestCase value/type misuse

    starksm64

      There are still some bad uses of deployment specific types in the ProfileServiceUnitTestCase tests. We are using mcf metadata types like ManagedConnectionFactoryPropertyMetaData, XAConnectionPropertyMetaData. These need to be converted into CollectionMetaType("java.lang.String", SimpleMetaType.STRING) types.

      Note that we are also leaking the attachment types as a classpath dependency because the ManagedObject references the deployment attachment. This should not be part of the serialization contract.

      We need to get the console work going forward to really drive the management implementation to completion in my view.

        • 1. Re: ProfileServiceUnitTestCase value/type misuse
          starksm64

          There was the question of primative types vs SimpleMetaType and whether the ManagedProperty should be accepting primatives and autoboxing these into SimpleValues. I think we should look into doing this.

          The question also came up of enforcing a check against the ManagedProperty.getMetaType().isValue(Object) for the values passed into ManagedProperty.setValue(Object). Could be done with aop, but I don't think we can assume jboss aop is available/used in the admin client. It could also be done via a wrapping proxy that imposed this behavior, as well as the autoboxing.

          • 2. Re: ProfileServiceUnitTestCase value/type misuse
            aloubyansky

            I made

            @XmlElement(name="xa-datasource-property")
             private List<XAConnectionPropertyMetaData> xaDataSourceProperties = new ArrayList<XAConnectionPropertyMetaData>();


            a Map in the management interface.

            Then there is
            @XmlElement(name="config-property")
             private List<ManagedConnectionFactoryPropertyMetaData> managedConnectionFactoryProperties = new ArrayList<ManagedConnectionFactoryPropertyMetaData>();


            ManagedConnectionFactoryPropertyMetaData has three properties: name, type and value. So, it should be a list of entries. I tried List<Map<String,String>> but current unwrap() impl doesn't seem to support it. Map<String,Map<String,String>> does work. So, that's what I decided to go with for now. Any better ideas?

            • 3. Re: ProfileServiceUnitTestCase value/type misuse
              alesj

               

              "scott.stark@jboss.org" wrote:
              There was the question of primative types vs SimpleMetaType and whether the ManagedProperty should be accepting primatives and autoboxing these into SimpleValues. I think we should look into doing this.

              The question also came up of enforcing a check against the ManagedProperty.getMetaType().isValue(Object) for the values passed into ManagedProperty.setValue(Object). Could be done with aop, but I don't think we can assume jboss aop is available/used in the admin client. It could also be done via a wrapping proxy that imposed this behavior, as well as the autoboxing.


              Looking at this piece of horrible code - which I added :-( - we really must clear up the confusion about this values/types.
               // TODO - decide what is the type of value when we set it
               // since currently get always returns MetaValue, but we see some cases
               // where plain Serializable is enough
               Serializable serializable;
               MetaValue metaValue;
               Object value = prop.getValue();
               if (value instanceof MetaValue)
               {
               metaValue = (MetaValue)value;
               MetaType metaType = metaValue.getMetaType();
               if (metaType.isSimple())
               serializable = ((SimpleValue)metaValue).getValue();
               else if (metaType.isGeneric())
               serializable = ((GenericValue)metaValue).getValue();
               else
               serializable = null;
               }
               else
               {
               serializable = (Serializable)value;
               metaValue = MetaValueFactory.getInstance().create(value);
               }
              
               if(serializable != null)
               ctxProp.setValue(serializable);
               // todo - should this also dispatch to runtime component?
               Object componentName = getComponentName(ctxProp);
               if (componentName != null)
               dispatcher.set(componentName, ctxProp.getName(), metaValue);
              


              I would expect a ManagedProperty to take MetaValue (so that it can also take non-serializable 'composite' values) and a runtime dispatcher to take plain object value.



              • 4. Re: ProfileServiceUnitTestCase value/type misuse
                alesj

                 

                "scott.stark@jboss.org" wrote:

                You can't pass in non-meta values. The api does not do conversion
                automatically. In general the admin client will not have access to the
                underlying ManagedProperty type, and so it should only use the meta
                value wrappers.


                Looks like we agree. :-)

                • 5. Re: ProfileServiceUnitTestCase value/type misuse
                  alesj

                   

                  "scott.stark@jboss.org" wrote:

                  Because the ManagedProperty can be used on the server with any value.
                  Its only the remote client view that should be restricted to meta values.

                  "alesj" wrote:

                  How come then the get/setValue API doesn't already enforce metavalue usage?


                  We should then have some nice OO distinction between client side ManagedProperty and server side.
                  To be strict regarding plain object value usage vs. generic serializable MetaValue for client view.

                  • 6. Re: ProfileServiceUnitTestCase value/type misuse
                    starksm64

                    Its too late for that. Introducing a client variation of the ManagedProperty with a setValue(MetaValue) is a new api. It can be added for clarification, but the existing ManagedProperty api would have to continue to work.