algo-web / podata Goto Github PK

So our current documentation is about as clear as mud (and scattered throughout the code). We have abstract classes, simple classes, and interfaces.

I was surprised that @kirill533 was able to make use of the library given how hard it is to follow. As such I think we need to put some time into writhing the documentation. My thoughts on how that should come together are:

• It should be a collaborative effort between me and second developer. As I wrote the vast bulk of the API knowledge I think is implied may not be clear.

• it should be split into between the simple up and running documentation (using the Simple* class set) and advanced that explains how to make use of the abstracts and interfaces for functionality. (Holding the Simple* classes up as examples.)

• permissions should be well covered.

• a bold note to indicate that authentication is not the job of this library.

so I'm looking for volunteers to do the writing while I make myself avaliable to explain how somthing works or why it is done that way.

Oh and @CyberiaResurrection I'm looking in your direction.

When building JSON response an error occurs in quoteJScriptString of JsonWriter:

Catchable fatal error: Object of class POData\ObjectModel\ODataCategory could not be converted to string in C:\www\GPC\vendor\algo-web\podata\src\POData\Writers\Json\JsonWriter.php on line 187

It can't leave them as they are - at very least, Apache and IIS will bounce (ie, 400) the request for security reasons.

Frinstance,

odata.svc/Books(UniqueID='PAP/63-XXXX')

will get a 400 response from Apache.

Is this library ready for usage?

Their is a call to BaseService->getService() in /src/POData/Providers/Stream/StreamProviderWrapper.php on line on line 439. but the function is never defined. I can not find anything in the git log that indicates it has ever been defined.

After looking at the calls of it i believe the function function should return the providers for future use. taking in an interface name and returns something that implements it.

documentation indicates services should have implemented IServiceProvider. and the function SHOULD have looked something like this.

    public function getService($serviceType)
    {
        switch($serviceType){
        case 'IMetadataProvider':
            return $this->Metadata;
        case 'IQueryProvider':
            return $this->QueryProvider;
        case 'IStreamProvider':
            return  $this-?StreamProvider
        default:
            return null;
        }
    }

But i think we should look into other uses, to see if this case was supposed to be replaced with something else in the future.

Inherited from ancestral is a setup that does the following:

    public function getResourceProperties(ResourceSetWrapper $setWrapper, ResourceType $resourceType)
    {
        if ($resourceType->getResourceTypeKind() != ResourceTypeKind::ENTITY()) {
            //Complex resource type
            return $resourceType->getAllProperties();
        }
        //TODO: move this to doctrine annotations
        $cacheKey = $setWrapper->getName() . '_' . $resourceType->getFullName();
        if (!array_key_exists($cacheKey, $this->propertyCache)) {
            //Fill the cache
            $this->propertyCache[$cacheKey] = [];
            foreach ($resourceType->getAllProperties() as $resourceProperty) {
                //Check whether this is a visible navigation property
                //TODO: is this broken?? see #87
                if ($resourceProperty->getTypeKind() == ResourceTypeKind::ENTITY()
                    && $resourceType instanceof ResourceEntityType
                    && null !== $this->getResourceSetWrapperForNavigationProperty(
                        $setWrapper,
                        $resourceType,
                        $resourceProperty
                    )
                ) {
                    $this->propertyCache[$cacheKey][$resourceProperty->getName()] = $resourceProperty;
                } else {
                    //primitive, bag or complex property
                    $this->propertyCache[$cacheKey][$resourceProperty->getName()] = $resourceProperty;
                }
            }
        }

        return $this->propertyCache[$cacheKey];
    }

For moment, I've ripped it out since that if statement is trivial. Flagging it as an issue to make going back to it easier, if someone ever figures out what the heck it was doing.

Hi Guys, came back to your library after my code started to break with the latest version of myclabs/php-enum package.

I though, it will be fixed in your version, but seems like it is the same issue here.

If you do the composer update to the latest version of myclabs/php-enum package, the tests should start giving errors. Most common error is like this:

68) UnitTests\POData\Providers\Metadata\ResourceTypeTest::testisResourceKindValidForPropertyKind with data set #16 (24, 2, false)
UnexpectedValueException: Value '24' is not part of the enum POData\Providers\Metadata\ResourcePropertyKind

The reason is that myclabs/php-enum package has now extra validation here: \MyCLabs\Enum\Enum::assertValidValueReturningKey

I guess it expects the class ResourcePropertyKind to have a constant with number 24 (KEY | PRIMITIVE). And actually, adding extra constants helps to fix the errors on my side.

My PHP version: PHP 7.3.19-1+ubuntu18.04.1+deb.sury.org+1

One solution would be to adjust composer.json to support only myclabs/php-enum with version below 1.8.0. But, please, make it compatible with myclabs/php-enum 1.8 and higher to make the library easier to combine with other libraries.

So I have been going through some of the commits that @kirill533 has made in his fork (based on #240) , some of them are quite elegant. I believe bringing those changes back to upstream would be be mutually beneficial as reducing the differences between his fork and us would reduce the maintance burden on him keeping up to date and would improve our code base.

That said @kirill533 has identifird that his needs may not match ours perfectly so not all commits will be able to come across. As such @CyberiaResurrection as our resident got expert. Can you look at a practical way to cherry pick his commits across into a branch that can then be opened as a pull request. I would like to maintain credit where it is due in the commit structure.

Let me know what you think the best way to-do that would be?

when a model has no primary key, the object model serializer enters a field which breaks the metadata.

Hi!

I dont quite understand how the new version of IndentedTextWriter::getResult() method is supposed to work.

What if the result variable will already include CRLF line endings. In this case, it will be replaced to CRCRLF?

    /**
     * @return string the current written text
     *                strReplace as json_encode does not always respect PHP_EOL
     */
    public function getResult()
    {
        return str_replace("\n", PHP_EOL, $this->result);
    }

p.s.

But actually, line separators sometimes needs to be adapted to meet the format requirements of some clients. For example, since I integrate with Microsoft products, I might prefer to return CRLF, even though my server runs on Linux.

I have recently integrated the last master version and it worked well after the recent refactoring. Thanks for that!

I have also noticed that some tests throw errors on windows version, probably because of usage of PHP_EOL. Thats why I started to look into the topic.

In the metadata info sent inline with the results in JSON format the referenced type is not qualified with metadata namespace.

Incorrect output (from version 0.3.0):

__metadata": { "uri":"http://localhost//AlgoWebPOData/odata.svc/Sessoes(ID=1)","type":"Sessao" }

Correct output (before version 0.3.0):

__metadata": { "uri":"http://localhost//AlgoWebPOData/odata.svc/Sessoes(ID=1)","type":"OData.Sessao" }

Is this still maintained? Will there be releases?

Please could I ask if this will allow me to expose a MySQL database as an OData service working for a PHP CMS e.g. Wordpress or Drupal?

If so, please could you let me know the installation and set-up instructions. I'm looking to integrate with Salesforce.

I have mapped the Product class that refers to the product table in my database, but my table is contained in a schema, eje: mydatabase.logistic.product.

how can I map in my Product class the reference to logistic.product and not only to product????

Thanks!!

This header appears to drive what writer is used when connecting Odata from Excel.

From looking over the available json writers, JsonLightODataWriter should be used, but can't match odata.metadata in https://github.com/Algo-Web/POData/blob/master/src/POData/Writers/Json/JsonLightODataWriter.php#L73

Backtracking from this, it seems this is removed by this line:
https://github.com/Algo-Web/POData/blob/master/src/POData/Writers/ResponseWriter.php#L74
, I'm not sure why. Just commenting this line doesn't fix the problem either though.

In src/POData/BaseService.php on lines 358-360

$method = $this->_serviceHost->getOperationContext()->incomingRequest()->getMethod();
$method = ($method != HTTPRequestMethod::POST());
if (!$request->isSingleResult() && $method) {

the checks that are being performed here in the $method variable should be performed inside the isSingleResult function. otherwise it will need to be duplicated at other locations.

Cough and die - library itself can't handle external forward and reverse mapping. That task belongs in the adaptor.

When $expand clause is used to deliver a set and provider does not send any results the output of an empty set is missing (as if the $expand was not used at all).

For this request: odata.svc/Tabelas('S_Anexos')?$expand=CamposChave

In atom output this is the output of an empty navigation to a set:
<link rel="http://schemas.microsoft.com/ado/2007/08/dataservices/related/CamposChave" type="application/atom+xml;type=feed" title="CamposChave" href="Tabelas(ID='S_Anexos')/CamposChave"/>

But it should be this:

<link rel="http://schemas.microsoft.com/ado/2007/08/dataservices/related/CamposChave" type="application/atom+xml;type=feed" title="CamposChave" href="Tabelas(ID='S_Anexos')/CamposChave">
<m:inline>
   <feed>
    <title type="text">CamposChave</title>
    <id>http://localhost//GPC/odata.svc/Tabelas(ID='S_Anexos')/CamposChave</id>
    <updated>2018-01-02T08:23:10+00:00</updated>
    <link rel="self" title="CamposChave" href="Tabelas(ID='S_Anexos')/CamposChave"/>
   </feed>
  </m:inline>
 </link>

In JSON output, the "__deferred" object is sent but an empty "results" array should be instead.
What is being sent:

{
    "d":{
        "__metadata":{
            "uri":"http://localhost//AlgoWebPOData/odata.svc/Tabelas(ID='S_Anexos')","type":"OData.Tabela"
        },"Campos":{
            "__deferred":{
                "uri":"Tabelas(ID='S_Anexos')/Campos"
            }
        },"CamposChave":{
            "__deferred":{
                "uri":"Tabelas(ID='S_Anexos')/CamposChave"
            }
        },"ID":"S_Anexos","Descricao":"S_Anexos","Copia":null
    }
}

What should be:

{
    "d":{
        "__metadata":{
            "uri":"http://localhost//GPC/odata.svc/Tabelas(ID='S_Anexos')","type":"OData.Tabela"
        },"Campos":{
            "__deferred":{
                "uri":"Tabelas(ID='S_Anexos')/Campos"
            }
        },"CamposChave":{
            "results":[

            ]
        },"ID":"S_Anexos","Descricao":"S_Anexos","Copia":null
    }
}

This happens for $expand from a single entry or $expand from a set.

the current one that i notice is the POData/src/POData/OperationContext/Web/Illuminate/ folder This is clearly taken from another package, and i feel like we should update composer json to get the package instead of copying the code out.

Think we might have tripped over why previous iterations of this library have had problems - those two classes strain even our Fearless Leader's ability to grok them, let alone mine.

TODO to fix:
1 - Figure out object structure implied by ODataFeed and its compatriots - recast it from a problem of forensic software archaeology to one of graph theory.
2 - Figure out what invariants are implied by that structure.
3 - Spin up IODataObject interface requiring IsOK and Serialise methods - C# has the right idea for this bit.
4 - Add that object to ODataFeed and its compatriots and implement contained methods.
5 - Spin up new object model serialisation gubbins.
6 - Verify new serialisation gubbins gives same result as old, via (functional testing?).
7 - NUKE OBJECTMODELSERIALIZER{,BASE} FROM ORBIT, IS ONLY WAY TO BE SURE.

The resourceSetName property in deserialised ODataEntry objects must point to a ResourceSet, otherwise the cynic deserialiser doesn't know what properties it should be expecting, etc.

Another issue that @kirill533 shed light on in issue #240 related to podata not being very memory friendly for large query sets. He attempted to solve the issue at
kirill533@99caf30

But had some issues related to expansion.

I would propose that if we change the interface at

And

It would then allow for the data to be loaded in a more dynamic nature to better suite. Smaller datasets and tests could just use the ArrayIterator Class to maintain simplicity for those a not sending thouands of records.

Current value for the ODataConstants::JSON_LIGHT_NEXT_STRING constant is: "odata.next".
Another value that appears in docs is: "odata.nextLink"

Example:
https://www.odata.org/getting-started/basic-tutorial/

{
    "@odata.context": "serviceRoot/$metadata#People",
    "@odata.nextLink": "serviceRoot/People?%24skiptoken=8",
    "value": [
        {
....

Confusing nature of the code.

the base directory contains examples, and docs, etc. I believe this not only makes the source base more confusing, but also makes the size of the composer packages larger. However, a lot of the base folder dose contain value, but not value to anyone seeking to simply use the package.

The Docs Directory contains value information for implementation and usage, both in pdf and html form. I would propose that the html version be migrated to use the wiki here on github, and we should find an alternative location for the pdf (because it could still be handy to have the single page document for printing, review on portable devices, etc)

The services directory has no direct value to the project or developers, but would have value for both testing and usability. specifically, our single point of truth for odata spec and existing web tutorials still provide these services (E.g
http://services.odata.org/V3/Northwind/Northwind.svc/ and http://www.odata.org/blog/odata-v3-demo-services/ ) I would as such propose that a new repo be created called "POData-Example-Services" and the contents of the services directory be migrated their.

Since version 0.3.0 $expand clause applied to a query that returns a set does not work. Works fine when $expand is used on a query for single record.

This works, "Orders" navigation is returned inline with record of customer "ALFKI":
http://localhost/NorthWind.svc/Customers(‘ALFKI’)?$expand=Orders

This does not work, since version 0.3.0, "Orders" navigation is not returned inline with any record of resulting set:
http://localhost/NorthWind.svc/Customers?$expand=Orders

Eg createForbiddenError - is this a forbiddenError on a create attempt, or what?

We kind of got carried away bolting new bits into POData. It would serve as an excellent familiarisation for someone to audit the change history and bring the changelog up to date.

Should thus be happening? It causes PHP 7.0.22 to jump up and down:

"return (strpos($fooClass->textIdentifier, '') === 0);"

This violates OData v3 - $skipToken should be a first-match pattern handler

After changes in version 0.3.0, JSON formed data sent in payload is no longer accepted. Failure occurs in executePost() and executePut() methods of URIProcessorNew.

Root cause seems to be missing implementation of deserialization of data into an instance of ODataEntry in readData() of RequestDescription.

I am getting an error when the method getResourceSetNameFromResource is called in MetadataManager because it doesn't exist. I see this was introduced in commit 6b7e8f2 but I don't see the implementation of this method anywhere, so I am not sure how is that possibly working in the tests.

I reverted the commit in my code and it works, but since there is something else going on that I'm not aware of I preferred to open an issue than opening a PR.

OData clients/servers deliver date differently. For example, https://services.odata.org/OData/OData.svc/Products
deivers field "updated" like this:
<updated>2020-05-07T12:41:26Z</updated> .

In case of Power BI, it works if the format of the date will be done the following way:

JsonWriter::writeValue

                $dateTime = new \DateTime($value, new \DateTimeZone('UTC'));
                $formattedDateTime = $dateTime->format('Y-m-d\TH:i:s');
                $this->writeCore($formattedDateTime, /* quotes */ true);

Current implementation in master is:

                $dateTime          = new \DateTime($value, new \DateTimeZone('UTC'));
                $formattedDateTime = $dateTime->format('U')*1000;
                $this->writeCore('/Date(' . $formattedDateTime . ')/', /* quotes */ true);

Could you please make it configurable?

Any plans regarding PHP 8.1 support?

I am required to make my libraries supporting 8.1, so will probably be doing another round of coding acound Podata library in my fork.

we really should be checking the names when they are being set to propertys to make sure they only contain underscores.
https://github.com/c-harris/POData/blob/master/src/POData/Providers/Metadata/ResourceProperty.php

There are large parts of the code base that were based on poor design concepts from time immemorial. i think this has dragged POData progress for many many years.

As the lead design team have reviewed we have decided a need exists to rewrite large parts of the existing code base to make better use of composer packages and to make the code base a lot simpler. major components to review should be:
UriProcessor
Writers
Providers
and
ObjectModel

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): object this would return an empty container object with appropriate properties to hold the new Entity
• • public function saveNewEntity(object $newEntity, array $bindProperties): ?object would 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): bool which 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): bool this 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.

When calling addResourceSet() in SimpleMetadataProvider, first parameter is the name of Entity Set being added. This parameter is now ignored (from version 0.3.0) and Entity Set name is always the result of Str::plural over Entity Type name.

This behavior is quite annoying when Entity names are not in English, using Illuminate\Support\Str (to my knowledge currently only supporting English) on non English words is wrong.

Of course this is not a bug, but to avoid changing the names of all my constructs (in Portuguese language...), is there any possibility to enable again the use of first parameter in addResourceSet()?

The wealth of information provided at #240 has identified that we have some major dependency issues that we could probably work around/avoid. firstly we have pulled in entire packages to access a tiny amount of it. secondly we have oddly specific dependencies. Finally we are using namespaces that we don't have dependencies for and praying they get pulled in at the second level. Audit and clean is required

Issuing a request with $expand of a resource set (1:n) like this:
http://localhost/NorthWind.svc/Customers(‘ALFKI’)?$expand=Orders

Ends with fatal error in line 378 of RequestExpander:

Fatal error: Uncaught Error: Call to undefined method Closure::getReference() in /www/GEA/vendor/algo-web/podata/src/POData/UriProcessor/RequestExpander.php:378

Taking out ...->getReference() works fine, apparently:
$orderByFunction = $internalOrderByInfo->getSorterFunction();

Currently JsonWriter.php produces a Edm.DateTime in format "yyyy-mm-ddThh:mm:ss" which does not comply with OData Json specs in http://www.odata.org/documentation/odata-version-2-0/json-format/

Edm.DateTime

"/Date(<ticks>["+" | "-" <offset>)/"<ticks> = number of milliseconds since midnight Jan 1, 1970<offset> = number of minutes to add or subtract

an extract from the documentation

10.2.2. Requesting Individual Properties
A service SHOULD support retrieving an individual property value.

To retrieve an individual property, a client issues a GET request to the property URL. The property URL is the entity request URL with “/” and the property name appended.

For complex typed properties, the path MAY be further extended with the name of the individual property of the complex type.

See OData:URL for details.

For example:

http://services.odata.org/OData/OData.svc/Products(1)/Name
10.2.2.1. Requesting a Property’s Raw Value using $value

A service SHOULD support retrieving the raw value of a primitive type property. To retrieve this value, a client sends a GET request to the property value URL. See the OData:URL document for details.

For example:

http://services.odata.org/OData/OData.svc/Products(1)/Name/$value
The raw value of an Edm.Binary property MUST be serialized as an unencoded byte stream.

The raw value of other properties SHOULD be represented using the text/plain media type. See OData:ABNF for details.

A $value request for a property that is NULL SHOULD result in a 404 Not Found. response.

The values are always null when attempting to execute.

The simple metadata provider was some time replaced by the metadataManager in the o-data-metadata package. but the Simplemetadata provider was left as a wrapper around that class. as such it probably needs to be reworked so it is a complete wrapper around the metadata manager and the rest of POData use the the objects in o-data-metadata.

Because this is likely to lead to an interface change i would like to give some time for other people using the project to have some input on how the provider should work and changes in the interface. specifically @cdcampos i believe you are probably using the provider directly so your input would be appreciated.

As at the next major release, we're planning on altering the license POData is licensed under - details to be finalised. If you're unhappy with this change, please notify us and we'll remove your code.

Do we need to check that values are in range before serialisation?