ReadonlyList and Readonly Factory DataAccess not generated properly

[GoogleCodeExporter]

What steps will reproduce the problem?
1. User Template CSharp\DataAccessLayer\DataAccessLayer.cst
2. Select ObjectFactoryParameterizedSQL for DataAccessImplementation
3. Generate some ReadOnlyList and ReadOnly objects

What is the expected output? What do you see instead?
I See this
  public ObjectFilter1List Fetch(ObjectFilter1Criteria criteria)
        {
            ObjectFilter1List item = (ObjectFilter1List)Activator.CreateInstance(typeof(ObjectFilter1List), true);

I should see this
 public ObjectFilter1InfoList Fetch(ObjectFilter1Criteria criteria)
        {
            ObjectFilter1InfoList item = (ObjectFilter1InfoList)Activator.CreateInstance(typeof(ObjectFilter1InfoList), true);

What version of the product are you using?
6.5.2 - CSLA 4.5

Please provide any additional information below.
Same for the ReadOnly object Factory Data Access 

Original issue reported on code.google.com by jteneg...@gmail.com on 31 Jan 2013 at 4:52

[GoogleCodeExporter]

Also,
 public ObjectFilter1Info Map(SafeDataReader reader)
        {
            var item = (ObjectFilter1Info)Activator.CreateInstance(typeof(ObjectFilter1Info), true);
            using (BypassPropertyChecks(item))
            {
                item.ObjectFilterUID = reader.GetGuid("Object_Filter_UID");
                item.TimeStamp = reader.GetDateTime("TimeStamp");
                item.UserStamp = reader.IsDBNull("UserStamp") ? (System.Guid?)null : reader.GetGuid("UserStamp");                
                item.Documentation = reader.GetString("Documentation");
            }

            MarkOld(item);
            MarkAsChild(item);

            return item;
        } 

These item.xxxxx generates Compile error
The property xxxxx  has no setter

Original comment by jteneg...@gmail.com on 31 Jan 2013 at 5:10

[GoogleCodeExporter]

Hello,

Could you please attach a sql script or send support@codesmithtools.com a 
sample schema that can reproduce this behavior. This would allow me to further 
look into this issue.

Original comment by bniemyjski on 1 Feb 2013 at 6:00

• Changed state: AwaitingInfo
• Added labels: Type-Defect, Framework-CSLA

[GoogleCodeExporter]

Here you go

Original comment by jteneg...@gmail.com on 5 Feb 2013 at 3:11

Attachments:

• ObjectFilterTable.sql

[GoogleCodeExporter]

I've modified my local templates to include 'Info' on the read only Object 
Factory return values, but I'm not sure about a safe way to load the field 
values in the Map() function, without exposing the FieldManager to the object 
factory.

So I have exactly the same problem, here.

Original comment by kbl...@puzzlebox.co.uk on 8 Feb 2013 at 4:25

[GoogleCodeExporter]

Of course - these can be set by adding an internal setter to the 
PropertiesReadOnly.cst, since the InternalsVisibleTo property allows these to 
be accessed by the ObjectFactory classes.

Since I now have a working set of read only lists using the object factory...  
Is there somewhere I can submit my changes so they could be looked at being 
incorporated into the standard templates?

Original comment by kbl...@puzzlebox.co.uk on 11 Feb 2013 at 12:46

[GoogleCodeExporter]

Yeah, just create a svn patch and create a new issue or attach here if this is 
the exact solution to this issue.

Original comment by bniemyjski on 11 Feb 2013 at 10:53

[GoogleCodeExporter]

Original comment by bniemyjski on 12 Feb 2013 at 12:01

• Changed state: Investigating

[GoogleCodeExporter]

Hi Blake,
If I may. This is kind of a PITA issue for us right now. Could you please, 
please look into this? I checked SVN (as mentionned above) and did not find 
anything related to this.
Perhaps you can share what kblake@puzzlebox.co.uk sent you?
Any help is greatly appreciated, really! Thanks.

Original comment by francois...@outlook.com on 18 Feb 2013 at 9:58

[GoogleCodeExporter]

Hello,

I'm looking into this as we speak. My apologies for not getting to it sooner.

Original comment by bniemyjski on 18 Feb 2013 at 11:27

[GoogleCodeExporter]

This issue was updated by revision r2952.

Updates to ReadonlyList and Readonly Factory DataAccess not generated properly

Original comment by bniemyjski on 19 Feb 2013 at 12:27

[GoogleCodeExporter]

Hey guys,

I committed some pretty big fixes for this scenario. Would you mind pulling the 
latest changes and giving me some feedback on this.

So there currently is one area we are still broken and I would like to get your 
feedback.

The BypassPropertyChecks requires BusinessBase which readonly objects are not. 
This causes an issue as the check is no longer valid. We need to look into the 
side effects of just removing this code.

Secondly The IPropertyInfo object is private and we don't have access to 
updating the property. How are you guys solving setting the properties via the 
map method? Reflection?

Original comment by bniemyjski on 19 Feb 2013 at 12:30

• Changed state: AwaitingInfo

[GoogleCodeExporter]

Any sample code for a valid solution is greatly appreciated.

Original comment by bniemyjski on 19 Feb 2013 at 12:30

[GoogleCodeExporter]

Hi Blake,

My apologies for not sending over my doctored templates, yet.  I had intended 
to clean them up (i.e. make them far less hacky) and submit as a proper svn 
patch, but I'm really struggling to find the time to do so.

In the meantime, to directly answer your questions - I blindly removed the 
BypassPropertyChecks which seemed to work for us, but I have not carried out 
any extensive testing or risk analysis of this, as you suggest, that's 
certainly a good idea.

Re. setting the properties, I made the setters internal for the readonly 
classes (I don't *think* they were before...) which makes them visible to the 
ObjectFactory via the InternalsVisibleTo assembly attribute contained within 
the FactoryNames class (as an aside, would this be better placed inside an 
AssemblyInfo class).

Original comment by kbl...@puzzlebox.co.uk on 19 Feb 2013 at 9:34

[GoogleCodeExporter]

I have attached a copy of the templates we're using, in-lieu of not being able 
to get a proper patch out to you quickly.

Please do feel free to use these for reference for your own changes, but do be 
aware that they also contain a number of other changes we've made which may be 
unique to us.

Full responsible disclosure... I wouldn't advise taking them as a complete set 
of templates (and *certainly* wouldn't recommend using for a production system 
instead of the templates in svn - there are scenarios that we're not 
implementing, which may result in these templates creating incomplete code that 
may not even compile).

But I do hope it helps :)

Original comment by kbl...@puzzlebox.co.uk on 19 Feb 2013 at 9:47

Attachments:

• Csla-Csharp.zip

[GoogleCodeExporter]

Blake

You missed one spot in the Entity Generator

partial void OnChildInserting(ObjectClass objectClass, ObjectType objectType, 
SqlConnection connection, ref bool cancel);

should be
partial void OnChildInserting(ObjectFilterInfo objectFilter, Language language, 
SqlConnection connection, ref bool cancel);;

AND

partial void OnChildUpdating(ObjectFilter objectFilter, Language language, 
SqlConnection connection, ref bool cancel);

Should be 
partial void OnChildUpdating(ObjectFilterInfo objectFilter, Language language, 
SqlConnection connection, ref bool cancel);

Though since this is readonly object, they should be be there at all...

As for the BypassPropertyChecks, i'd remove it and add an internal set for the 
properties set

  private static readonly PropertyInfo<System.DateTime> _timeStampProperty = RegisterProperty<System.DateTime>(p => p.TimeStamp, "Time Stamp");
        public System.DateTime TimeStamp
        {
            get { return GetProperty(_timeStampProperty); }
            internal set { LoadProperty(_timeStampProperty, value); }
        }

so the mapper would look like this
      public ObjectFilterInfo Map(SafeDataReader reader)
        {
            var item = (ObjectFilterInfo)Activator.CreateInstance(typeof(ObjectFilterInfo), true);

            item.ObjectFilterUID = reader.GetGuid("Object_Filter_UID");
            item.ObjectClassGID = reader.GetInt16("Object_Class_GID");
            item.ObjectTypeUID = reader.IsDBNull("Object_Type_UID") ? (System.Guid?)null : reader.GetGuid("Object_Type_UID");
            item.FilterText = reader.GetString("Filter_Text");
            item.Documentation = reader.GetString("Documentation");

            MarkOld(item);
            MarkAsChild(item);

            return item;
        }

Let me know if it is not clear enought

thanks

Original comment by jteneg...@gmail.com on 19 Feb 2013 at 1:42

[GoogleCodeExporter]

the line above should have been read this way

* Though since these are readonly objects, These methods should NOT be there at 
all...

Original comment by jteneg...@gmail.com on 19 Feb 2013 at 5:08

[GoogleCodeExporter]

Thanks, I'll take a look at the proposed changes.

Original comment by bniemyjski on 19 Feb 2013 at 7:38

[GoogleCodeExporter]

This is quickly turning into a chunk of work :s. I'll keep you guys posted.

Original comment by bniemyjski on 20 Feb 2013 at 10:47

[GoogleCodeExporter]

After much thought... I think the best way to solve this issue is via 
reflection. It's the only way I can think of that solves this without forcing 
users to put an internal visible to attribute on the class lib. Also it keeps 
users from modifying read only objects from within the same class library. 
Thoughts?

Original comment by bniemyjski on 26 Feb 2013 at 3:55

[GoogleCodeExporter]

This is one way, but I do not mind the internal visible solution as well. 
Obviously we do not want the object to be modified if it is read-only.

Original comment by jteneg...@gmail.com on 27 Feb 2013 at 2:04

[GoogleCodeExporter]

Likewise.

The InternalsVisibleTo property is already included in the code generated by 
the current templates (it's in FactoryNames, but applies to the whole 
assembly), so I don't have a problem with making use of it here as well... 
Reflection is fine, too - both should achieve the same goal.

Original comment by kbl...@puzzlebox.co.uk on 27 Feb 2013 at 9:20

[GoogleCodeExporter]

It would be nice going to a reflection based approach and getting rid of the 
attribute all together.  

If any of you would be willing to pair on this, let me know. I've marked it as 
accepted and will get to it as soon as I can. (We are finishing up some big 
releases internally and there is a push to get those out. And then I can work 
on getting the next big csla template release out. I do really hate making 
anyone wait, so an extra pair of hands would help speed things up/help motivate 
me hehe.)

Original comment by bniemyjski on 27 Feb 2013 at 8:29

• Changed state: Accepted
• Added labels: Priority-High