DataObjects-NET / dataobjects-net

https://dataobjects.net
MIT License
61 stars 25 forks source link

Recycled attribute does not apply from Interface #316

Closed HampikGitHub closed 1 year ago

HampikGitHub commented 1 year ago

Name from TestEntity will not be deleted from database after domain build. DO 7.0 ` [HierarchyRoot] public class TestEntity : Entity, IInterface { public TestEntity(Session s, int id) : base(s, id) { }

    [Key] [Field(Nullable = false)] public int Id { get; set; }

    public string Name { get; set; }
}

public interface IInterface
{
    [Recycled]
    [Field(Nullable = false)] public string Name { get; set; }
}`
alex-kulakov commented 1 year ago

Hello,

Thank you for your feedback. This attribute works on early stages of building model when no inheritance or interface implementations haven't been processed yet. It was done so to not waste time and memory for processing types or properties that won't get to final DomainModel 100%. I'll check whether it is possible to fix in current design.

If you need a workaround solution then I offer you to mark actual implementation of property with RecycledAttribute.

alex-kulakov commented 1 year ago

I've dug dipper for this issue. I didn't saw that this should be fixed.

Logically, it is similar to regular interface-to-class relation. A class is required to have all members of the interface it implements, but an interface doesn't have to have all members of implementors. Also, if you remove member from an interface it does not affect implementor (except for explicit member implementations). So, to remove a member from both, interface and implementor, you should actively remove it from both.

Persistent fields of a persistent interface are required to be presented in its implementors - entities. Language handles existence of properties in implementors and if implementation of property isn't marked with FieldAttribute this attribute will propagate from interface property to make sure field exists (case 1 down below), but nothing prevents you from marking persistent properties by yourself (case 2 down below). Lets see how your suggestion will affect each of way to declare persistent interface and its implementor.

1. When only properties within interface are marked with FieldAttribute.

Here's an example:

    public interface IEmployee : IEntity
    {
      [Field, Key]
      long Id { get; }

      [Field(Nullable = false)]
      string FirstName { get; set; }

      [Field(Nullable = false)]
      string LastName { get; set; }

      [Field]
      DateTime HiredOn { get; set; }

      [Field]
      string CommentOnPerson { get; set; }
    }

    [HierarchyRoot]
    public sealed class Employee : Entity, IEmployee
    {
      public long Id { get; private set; }

      public string FirstName { get; set; }

      public string LastName { get; set; }

      public DateTime HiredOn { get; set; }

      public string CommentOnPerson { get; set; }
    }

In such cases propagation of FieldAttribute will occur, it is done by Domain building process which assumes that if an interface property has FieldAttribute and implementation property hasn't then property implementation should be persistent field too.

If we recycle one of persistent fields in the IEmployee interface like so

    public interface IEmployee : IEntity
    {
      [Field, Key]
      long Id { get; }

      [Field(Nullable = false)]
      string FirstName { get; set; }

      [Field(Nullable = false)]
      string LastName { get; set; }

      [Field]
      DateTime HiredOn { get; set; }

      [Field, Recycled]
      string CommentOnPerson { get; set; }
    }

it will affect entity as well, similar to what you want. This happens because RecycledAttribute will lead to ignoring FieldAttribute in the interface like it never existed and since there is no FieldAttribute to propagate the implementing property will not become persistent field in Employee entity.

We could've chosen to throw an exception about non-existing attribute in entity but, I assume, we chose to make users' live a bit easier (I guess because I wasn't a part of the team back then).

It is also possible that entity can implement several persistent interfaces which may overlap by field set. For example,

    public interface IPerson : IEntity
    {
      [Field, Key]
      long Id { get; }

      [Field(Nullable = false)]
      string FirstName { get; set; }

      [Field(Nullable = false)]
      string LastName { get; set; }
    }

    public interface IEmployee : IEntity
    {
      [Field, Key]
      long Id { get; }

      [Field(Nullable = false)]
      string FirstName { get; set; }

      [Field(Nullable = false)]
      string LastName { get; set; }

      [Field]
      DateTime HiredOn { get; set; }

      [Field]
      string CommentOnPerson { get; set; }
    }

    [HierarchyRoot]
    public sealed class Employee : Entity, IEmployee, IPerson
    {
      public long Id { get; private set; }

      public string FirstName { get; set; }

      public string LastName { get; set; }

      public DateTime HiredOn { get; set; }

      public string CommentOnPerson { get; set; }
    }

Say, one of the interfaces recycled overlapping field LastName to exclude it from API it provides and the other one didn't. Since one of interfaces kept LastName as persistent field Employee entity would have another source of FieldAttribute to propagate and remain LastName as persistent field, and it is good. As result, one interface shrunk its API as we wanted, another one remained the same as we wanted, and the entity didn't lose its persistent field and no data loss occurred. Great!

If all overlapping interfaces recycled the LastName persistent field, then no source to propagate would exist so the property of Employee would be non-persistent and it would eliminate data stored in database for LastName. This is also expected behavior.

2. When property is marked with FieldAttribute in both - interface and entity. The case you want to change and add RecycledAttribute propagation. Let's see how it works now. Imagine we have a persistent interface and two implementations as shown below.

    public interface IPerson : IEntity
    {
      [Field]
      long Id { get; }

      [Field]
      string FirstName { get; set; }

      [Field]
      string LastName { get; set; }

      [Field]
      string CommentOnPerson { get; set; }
    }

    [HierarchyRoot]
    public sealed class Customer : Entity, IPerson
    {
      [Field, Key]
      public long Id { get; private set; }

      [Field(Nullable = false)]
      public string FirstName { get; set; }

      [Field(Nullable = false)]
      public string LastName { get; set; }

      [Field]
      public DateTime? BirthDay { get; set; }

      [Field]
      public string CommentOnPerson { get; set; }
    }

    [HierarchyRoot]
    public sealed class Employee : Entity, IPerson
    {
      [Field, Key]
      public long Id { get; private set; }

      [Field(Nullable = false)]
      public string FirstName { get; set; }

      [Field(Nullable = false)]
      public string LastName { get; set; }

      [Field]
      public DateTime HiredOn { get; set; }

      [Field]
      public string CommentOnPerson { get; set; }
    }

Such declaration enables us to change persistent interface and entities separately, kind of, like it would be with regular interfaces. In the following example, CommentOnPerson can be excluded from interface but kept in Customer and Employee (no data loss occurs).

    public interface IPerson : IEntity
    {
      [Field]
      long Id { get; }

      [Field]
      string FirstName { get; set; }

      [Field]
      string LastName { get; set; }

      [Field, Recycled]
      string CommentOnPerson { get; set; }
    }

    [HierarchyRoot]
    public sealed class Customer : Entity, IPerson
    {
      [Field, Key]
      public long Id { get; private set; }

      [Field(Nullable = false)]
      public string FirstName { get; set; }

      [Field(Nullable = false)]
      public string LastName { get; set; }

      [Field]
      public DateTime? BirthDay { get; set; }

      [Field]
      public string CommentOnPerson { get; set; }
    }

    [HierarchyRoot]
    public sealed class Employee : Entity, IPerson
    {
      [Field, Key]
      public long Id { get; private set; }

      [Field(Nullable = false)]
      public string FirstName { get; set; }

      [Field(Nullable = false)]
      public string LastName { get; set; }

      [Field]
      public DateTime HiredOn { get; set; }

      [Field]
      public string CommentOnPerson { get; set; }
    }

Notice RecycleAttribute in IPerson interface. Such change will affect only queries that work with the interface like

var customersAndEmployees = session.Query.All<IPerson>().Where(ip => ip.CommentOnPerson!=null).ToList();

but not queries which work with entities directly. And no raw data loss happens, which is important. I believe there are cases when this approach can be useful. And similarly to regular interfaces, to eliminate CommentOnPerson completely you need to remove persistent field from the interface and from all implementations, in other words, recycle persistent fields of the interface and all its implementations.

If RecycledAttribute propagation existed, it would remove fields from Customer and Employee entities which would cause data loss. Additionally, It would make impossible to shrink only the interface but keep entities the same.

Next scenario I would like to mention is when there are two interfaces, which are overlapping by some persistent field, one interface declares one of overlapping persistent fields as recycled and another one doesn't:

    public interface IPerson : IEntity
    {
      [Field]
      long Id { get; }

      [Field(Nullable = false)]
      string FirstName { get; set; }

      [Field(Nullable = false), Recycled]
      string LastName { get; set; }
    }

    public interface IEmployee : IEntity
    {
      [Field]
      long Id { get; }

      [Field(Nullable = false)]
      string FirstName { get; set; }

      [Field(Nullable = false)]
      string LastName { get; set; }

      [Field]
      DateTime HiredOn { get; set; }

      [Field]
      string CommentOnPerson { get; set; }
    }

    [HierarchyRoot]
    public sealed class Employee : Entity, IEmployee, IPerson
    {
      [Field]
      public long Id { get; private set; }

      [Field(Nullable = false)]
      public string FirstName { get; set; }

      [Field(Nullable = false)]
      public string LastName { get; set; }

      [Field]
      public DateTime HiredOn { get; set; }

      [Field]
      public string CommentOnPerson { get; set; }
    }

Now it works on persistent interface level, and will cause only IPerson persistent interface API shrunk, but the rest of model will stay the same. RecycledAttribute propagation would create conflict when overlapping persistent field of Employee is recycled and not recycled at the same time, which should be prevented by, for example, exception. This conflict would require you to resolve it with recycle persistent field in IEmployee interface. What if you want IEmployee to be unchanged? You can't mark Employee.CommentOnPerson recycled because it brings more uncertainty to the situation. It would become very clunky. As oppose, current logic is clearer to me, and allows more designs.

Summarizing all that said above, I wouldn't say that proposed RecycledAttribute propagation from an interface to implementors is good idea. Maybe it would work in your particular case but the rest of cases would become less flexible or even eliminated, which is not good.