-
Notifications
You must be signed in to change notification settings - Fork 13
Description
Ok so write support is a complicated animal and it is currently handled by
CynicDeserialiser. Leaving it as is would be kind of like owning a sick goldfish. I think the entire thing needs to be thrown away and rewritten.
Firstly we need to consider how we are going to interface with the Query Provider. At the moment it's not big ball of mud that handles all data related matters. I think this was a mistake for three reasons:
- the two components are used independent of each other. Reading and writing are two different types of request.
- the complexity of the interface creates a significant maintainability burden for implimentors.
- Write support should be optional for the service implimentors. If I might quote the documentation
An OData service MAY support Create, Update, and Delete operations for some or all of the entities that it exposes
10.3. Data Modification
As sound I would propose:
- The existing IQueryProvider be renamed IReadQueryProvider with write related methods removed
- A IWriteQueryProvider be created.
As this would constitute a breaking change on approach to version 0.4.0 it would be ideal time to consider the definition of the new IWriteQueryProvider. Which my initial thoughts are
- transaction support should be optional.
public function supportsTransactions(): bool? When transactions are not supported we should create reverse set of operations and should a query fail we should attempt to roll back and changes. - 10.3.2. Create an Entity should consist of two methods
-
public function getEmptyContainer(ResourceEntityType $entityType): objectthis would return an empty container object with appropriate properties to hold the new Entity
-
public function saveNewEntity(object $newEntity, array $bindProperties): ?objectwould be responsible for creating the new entity in the data source. The $bindingProperties array would be a key value pair of associated objects to hook up at creation (<string propertyName,<object $entities>>). On sucess it would return the new object (including new default and key fields) on failure it would return null. If the write query provider does not support transactions the key should be logged in a rollback array for deletion.
- 10.3.3. Update an Entity should be a single method
public function updateEntity(object $entity, array $bindProperties): ?object. it should save the updated entity to the database and return the updated entity on sucess or null on failure. In Podata we would access the IReadQueryProvider to retrieve the entity apply the updated to property of the entity (depending on update or replace from client) and send the entity for saving. If transactions are not supported the old values should be saved in a rollback array. - 10.3.4. Delete an Entity should be a single method
public function deleteEntity(ResourceSet $resourceSet, KeyDescriptor $keyDescriptor): bool. It should simply lookup the object for deletion and delete it. Returning true on sucess and false on failure. If transactions are not supported we should first hit up the IReadQueryProvider to get a copy of the record to save in a rollback array - 10.3.5.1. Create a New Link Between Two Existing Entities in a One to Many Navigation Property should be a single method. Both would be responsible for hooking the models together returning true on sucess and false on failure. If transactions are not supported an entry should be created in the rollback array to delete the assocation. If a way exists to have both thag would be great but the candidates are:
-
public function associateOne(object $primary, string $propertyOnPrimary, object $secondary): boolwhich would fetch both objects from IReadQueryProvider and pass them to be associated. This has the advantage of simplicity but the disadvantage of having TWO hits to IReadQueryProvider (and probably the database). But the advantage of simplicity.
-
public function associateOne(ResourceSet $primaryResourceSet, KeyDescriptor $primaryKeyDescriptor,ResourceSet $secondaryResourceSet, KeyDescriptor $secondaryKeyDescriptor, string $propertyOnPrimary): boolthis would leave it to the provider to fetch any required data before associating. It has the advantage of not requiring any hits to the ReadQuery (or database). But has two main disadvantaged. It's complexity to implement and it would be a break from the other methods which do not require read operations.
- 10.3.5.2. Remove a Relationship Between Two Entities again this method could be implemented as a single method with the same signature and disadvantages as creating. The proposed signature is
public function disassociateOne(...): bool. If transactions are not supported entry should be created in the rollback array to reassociate. - 10.3.5.3. Change the Relation in a One to One Navigation Property can be handled by the options above (either updating the source entity or deleting and adding the relation)
- 10.3.6 Managing Media Entities should be handled by the IStreamProvider or possibly a counterpart at IStreamEditProvider
- 10.3.8. Managing Values and Properties Directly should be handled separately because when transactions are supported they can be handled without a ReadOperstion (being replace operations) the proposed signature would be
public function updateProperty(ResourceSet $resourceSet, KeyDescriptor $keyDescriptor, string $propertyName, ?IType $propertyValue): bool. It would save the entity to the database returning true on sucess and false on failure. If transactions are not supported a read operation would be performed before to save the original value to the rollback array.
In the interest of having a best of both worlds on update and delete. We could do along side the interface
abstract class SimpleReadQueryProviderBase implements IWriteQueryProvider
With a constructor constructor
function __construct(IReadQueryProvider $readProvider)
It could then wrap the more complex calls (involving ResourceSets and key descripters) around to the simpler alternative.
I will continue to update this original as this issue is discussed to be a clear reflection of a course of action.