Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keys difference on JSON vs. JSON-LD #2066

Closed
er1z opened this issue Jul 4, 2018 · 28 comments
Closed

Keys difference on JSON vs. JSON-LD #2066

er1z opened this issue Jul 4, 2018 · 28 comments
Labels

Comments

@er1z
Copy link
Contributor

er1z commented Jul 4, 2018

I have a data created by custom data provider. Using JSON-LD the data looks like this:

{
  "@context": "/api/contexts/Struct",
  "@id": "/api/structs",
  "@type": "hydra:Collection",
  "hydra:member": [
    {
      "@id": "/api/structs/50c10eb8-ff18-4b02-9ca7-96c4efd78610",
      "@type": "Struct",
      "field1": "50c10eb8-ff18-4b02-9ca7-96c4efd78610",
      "field2": 1530690809
    },
    {
      "@id": "/api/structs/0f9f8409-08a7-4f52-bfa6-28bd91965ef0",
      "@type": "Struct",
      "field1": "0f9f8409-08a7-4f52-bfa6-28bd91965ef0",
      "field2": 1530690809
    },
    {
      "@id": "/api/structs/cf6e09ca-667e-4155-a7bd-c93369bedece",
      "@type": "Struct",
      "field1": "cf6e09ca-667e-4155-a7bd-c93369bedece",
      "field2": 1530690809
    },
    {
      "@id": "/api/structs/821920b5-bf87-474f-8d19-0ede22390b85",
      "@type": "Struct",
      "field1": "821920b5-bf87-474f-8d19-0ede22390b85",
      "field2": 1530690809
    }
  ],
  "hydra:totalItems": 4
}

Whereas using pure JSON produces:

{
  "1": {
    "field1": "27557fc5-7f22-4c67-992e-da999f89e303",
    "field2": 1530690972
  },
  "2": {
    "field1": "488b08c4-3aee-41f1-89c3-72c258cae9f6",
    "field2": 1530690972
  },
  "3": {
    "field1": "61559539-2a52-491a-8518-e495fcc30c78",
    "field2": 1530690972
  },
  "4": {
    "field1": "6903a099-42cb-4bf1-ace2-11a2244ee029",
    "field2": 1530690972
  }
}

How keys should look like? Implementing partial paginator enforces using iterator and yielding keys. Two questions:

  1. Which form is correct? Both with keys or neither?
  2. How to disable keys creation within pure JSON object and keep custom data provider with pagination?
@dunglas
Copy link
Member

dunglas commented Jul 4, 2018

My advice is to always use JSON-LD. "raw" JSON has no benefits over JSON-LD, on the other hand, JSON-LD adds nice capabilities like pagination metadata, robust type system, filters documentation... (that your clients are free to not use, of course).

Regarding the keys it's because your associative array probably look like [1=> [], 2 => []] (there is no 0 key). And this case, json_encode consider that it's a map and convert it to a JSON object. If keys start at 0 and are consecutive (packed array), json_encode will create a JSON array instead. Another solution is to create an ArrayObject in your data provider instead of a typical array.

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

I know but I just need the pure JSON, ok? (especially for other clients that are not able to parse JSON-LD; say „thanks” to API Platform Client Generator that completely forgot about Angular2+ exists)

Back to the course, it's not an associative array but an object that implements \Iterator and the interface requires that key method is implemented. Returning nothing corrupts the overall results producing an array with single element that has a key with an empty string.

How to use custom data provider without enforcing to be associative? Is it a bug that should be fixed? I don't know APIP well so far, so I'm asking; may I do something in the wrong way.

@dunglas
Copy link
Member

dunglas commented Jul 4, 2018

especially for other clients that are not able to parse JSON-LD

All "pure" JSON parsers are able to parse JSON-LD. Including JSON.parse and json_decode. JSON-LD is valid JSON. It only reserves keys starting with a @ (that are also valid JSON).

API Platform Client Generator that completely forgot about Angular2+

API Platform is a free, open-source, community-driven project. If someone (including you) contribute Angular 2+ support, we'll be glad to merge it. Note that there is already a Typescript definition generator in this project.

I'm not sure to understand your last paragraph, can you paste the code. I'm pretty sure your issue is related to this: https://3v4l.org/7NDh3
It's a well known PHP behavior that cannot be fixed, you just need to be sure to use proper packed arrays.

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

Okay, I misused the word: I meant „process”. All the old processors need is an array of objects, not object with array so I want to use pure JSON.

I know it's OS, moreover even you have accepted my PR but right now I have to pay with my time to something else and dunno TS so well to create this clear.

To clarify, I do not mean the pure array. I'm working on Iterator in order to implement paginator interfaces. Look:

class sth implements \Iterator{

    // ...

    public function current()
    {
        return 'SOMETHING'; 
    }

    public function key()
    {
        return 1; // ...2,3,4,5...
    }
    
    // ...
    
}

Let's say it's a data collection provider. After calling iterator_to_array on this object we get:

$a = [
    1=>'SOMETHING',
    2=>'SOMETHING',
    //...
];

Which — obviously — produces the second array you mentioned in 3v4l. If I change the key method result to null, I get:

$a = [
   ''=>'SOMETHING'
];

Regardless how many records I return. My question: is it okay for adding to Core the PR with this behavior (pseudocode): IF is_null(iterator->key) THEN return iterator->current (without associating this with key)? Thus, the result could look like:

class sth implements \Iterator{

    // ...

    public function current()
    {
        return 'SOMETHING'; 
    }

    public function key()
    {
        return null;
    }
    
    // ...
    
}

Produced array:

$a = [
    'SOMETHING',
    'SOMETHING',
    //...
];

So that — pure JSON array will be properly serialized into array of objects.

I'm asking because I want to know if should I implement this on my custom data provider level or prepare a PR because it could be more global?

@dunglas
Copy link
Member

dunglas commented Jul 4, 2018

According to http://php.net/manual/en/iterator.key.php, your key method must return a 0-indexed position (to be encoded as an array). Returning null is only for failures and will throw a notice.

Should be:

    public function key()
    {
        return 0; // ...1,2,3,4,5...
    }

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

Ok, so what do you recommend in order to get rid of useless keys from iterator? Another interface for data collection provider to implement, let's say, NonIndexedCollection and then adjust serializer?

@teohhanhui
Copy link
Contributor

No, you just need to fix your Iterator implementation. 😄

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

Could you provide an example how to do that with custom data provider in order to return the data from iterator without indexing records using application/json type? Docs doesn't cover that although JSON-LD is indexed ok, so this is why I'm posting this issue.

@teohhanhui
Copy link
Contributor

If you don't want to change the keys returned by your Iterator, you could do

iterator_to_array($iterator, false);

@teohhanhui
Copy link
Contributor

Your issue is not related to JSON-LD. 😄

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

I know is unrelated to JSON-LD, it's related to pure JSON implementation. I'm asking to dismiss keys from iterator just like within JSON-LD it occurs. I, of course, could wrap the iterator's logic but the only solution is to put it on every data provider?

Within JSON-LD the key() result from iterator is ignored, I'm just asking for replicate the same behavior using pure JSON.

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

So I'm asking how to write this iterator in order to make this working? I'm completely stuck because docs don't contain any example, just some description. I know how to make a workaround but what's the right way (with working example, not a single line)?

@teohhanhui
Copy link
Contributor

teohhanhui commented Jul 4, 2018

I guess I must have misunderstood your question. You're returning an Iterator from CollectionDataProviderInterface::getCollection, right?

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

Yes, I'm returning an \Iterator.

@teohhanhui
Copy link
Contributor

Is it not possible for you to return 0-indexed keys from your Iterator?

@teohhanhui
Copy link
Contributor

In order to fix this properly, we might need to add a CollectionNormalizer that will be called for the json format. WDYT @dunglas?

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

@teohhanhui — it's possible but I need to dismiss them in resulting application/json.

@teohhanhui
Copy link
Contributor

teohhanhui commented Jul 4, 2018

@ertz If you return 0-indexed keys and with no gaps, it should work as expected. Have you tried it?

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

Yes, I've tried to return numbers from iterator's key but then I get:

{"0": "sth", "1": "sth"}

instead of

["sth", "sth"]

where sth is a record. This behavior doesn't occur when I use JSON-LD.

@teohhanhui
Copy link
Contributor

Thanks for confirming. I think we need to add a CollectionNormalizer then, as that kind of output is not expected.

@teohhanhui teohhanhui added the bug label Jul 4, 2018
@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

I'll try later to find fix for this but I don't know APIP so far so it may be weird. Thx for confirmation.

@soyuka
Copy link
Member

soyuka commented Jul 4, 2018

#1534 worth mentioning?

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

My apologies, when I start from 0, pure JSON has no indexing at all. But it doesn't explain different behavior comparing to JSON-LD when I start from 1.

Thanks for clarification.

@teohhanhui
Copy link
Contributor

teohhanhui commented Jul 4, 2018

@ertz It's because in the Hydra CollectionNormalizer we do not use the index:

foreach ($object as $obj) {
$data['hydra:member'][] = $this->normalizer->normalize($obj, $format, $context);
}

But for the json format, we don't have enough control over it because we don't implement our own CollectionNormalizer (not sure how feasible it is to do it). So it's actually handled here:

https://github.com/symfony/symfony/blob/68b56033c43d2c499d8dcf79f374c33049184b96/src/Symfony/Component/Serializer/Serializer.php#L145-L152

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

So why don't pass pure array using array_values before normalizing? I think the desired behavior is to keep indexes within two formats or neither.

@teohhanhui
Copy link
Contributor

If we try to convert everything to array, we'll cause you to lose the performance advantage you're trying to gain by using an Iterator. So that's not a solution.

@er1z
Copy link
Contributor Author

er1z commented Jul 4, 2018

How about something like „iterator proxy” capturing the key calls?

@trsteel88
Copy link
Contributor

This also happens if you use criteria to change the order of a collection:

    public function getItems(): Collection
    {
        $criteria = Criteria::create()->orderBy(['rank' => Criteria::ASC, 'id' => Criteria::ASC]);

        return $this->shots->matching($criteria);
    }

If the order changes, the result comes back as an array rather than an object. To get around this I had to create a serializer to do array_values() on the result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants