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

Add option to don't store #type along json data #63

Open
B-Galati opened this issue Feb 27, 2019 · 17 comments
Open

Add option to don't store #type along json data #63

B-Galati opened this issue Feb 27, 2019 · 17 comments

Comments

@B-Galati
Copy link

Hello!

What about adding Deserialization type in the app codebase instead of storing it in the database?

Something like:

/**
  * @ORM\Column(type="json_document", class="App\Address", options={"jsonb": true})
  */
public $foo;

class could also be in options if it's easier to manage.
It's a bit like doctrine @Embedded.

My concern is that having classname stored in DB is hard to maintain as a migration strategy has to be established on each namespace change.

@B-Galati B-Galati changed the title Don't store #type with data Add option to don't store #type along json data Feb 27, 2019
@dunglas
Copy link
Owner

dunglas commented Feb 27, 2019

You mean for inner documents? We could even use PropertyInfo to get the type instead of adding a new option.

@dunglas
Copy link
Owner

dunglas commented Feb 27, 2019

But there is a limitation with this approach: currently JSON ODM allows to store very variable data (mixed). With this approach, it's not possible anymore.

@B-Galati
Copy link
Author

What would be possible to avoid having to store FQCN along json data in your opinion?

@Toflar
Copy link
Contributor

Toflar commented Mar 4, 2019

I like the idea, however, it won't work with nested entities of different types.
What might work is some kind of mapper. So instead of #type you store #mapKey (or whatever) and in the annotation you can specify something along the lines of

typeMapping={"foobar": "App\Foobar", "otherkey": "App\Other"}

It would be also compatible with the existing solution so it's totally opt-in.

@pbowyer
Copy link

pbowyer commented Apr 30, 2019

You are likely already aware, but a fork of this library says it has moved the metadata out of the field: https://github.com/goodwix/doctrine-json-odm

Unfortunately the commits are not well-organised, so there is not a specific commit where they implemented this.

@B-Galati
Copy link
Author

B-Galati commented May 1, 2019

@pbowyer thanks, it looks cool!

@Toflar
Copy link
Contributor

Toflar commented May 2, 2019

What do you think about by typeMapping solution? If that would solve your problem (and there are a few thumb ups so probably it would for others), why not make a PR? 😄

@B-Galati
Copy link
Author

B-Galati commented May 2, 2019

@Toflar It looks like a good solution to me 👍

Regarding the PR, I won't be able to do it honestly.

FYI, we choose another solution: custom doctrine type for each object. It is based on this article https://matthiasnoback.nl/2018/03/ormless-a-memento-like-pattern-for-object-persistence/.

It's a lot less generic but it's doing the job pretty well for us as we don't need a lot of custom type.

@pierrejolly
Copy link

You are likely already aware, but a fork of this library says it has moved the metadata out of the field: https://github.com/goodwix/doctrine-json-odm

Indeed, but like @dunglas said, with this approach you can't store variable data, @Toflar idea seem more flexible IMO.

@TamasSzigeti
Copy link
Contributor

Based on this,
@dunglas Would you accept a PR introducing an optional type map to not rely on class names? Not sure if it was possible to config in the annotation, I was thinking of a global bundle config. Optional, BC.

@alanpoulain
Copy link
Contributor

I think it would be great to have the possibility to not store the #type along with the great type map done in PR #118.
It makes sense when the stored data are always of the same type: in this case you don't want to store this useless metadata, you only want to (de)serialize your data with the same class.

@TamasSzigeti
Copy link
Contributor

@alanpoulain I might misunderstand your use case, but wouldn't a custom doctrine type would suit better if it's always the same class?

@alanpoulain
Copy link
Contributor

Sure but you need to write custom code to (de)serialize your data. Having it done by this library out of the box would be nice (that's what https://github.com/goodwix/doctrine-json-odm does).

@cyraid
Copy link

cyraid commented Mar 2, 2024

@alanpoulain I might misunderstand your use case, but wouldn't a custom doctrine type would suit better if it's always the same class?

It's much quicker to annotate a few times on a POPO that you probably already created, than create a custom Doctrine Type, and then possibly map it, etc. Also, not everyone is familiar with being able to create a Doctrine type, and have to worry about (de)serializing it properly, etc. I'm guessing quite a few of them would come to this repo to get a quick 'composer require <>' and expect to work with POPO's.

@dunglas
Copy link
Owner

dunglas commented Mar 2, 2024

PR welcome then!

@cyraid
Copy link

cyraid commented Mar 2, 2024

PR welcome then!

Wouldn't it be quicker and easier for you to do it as you know the codebase already and are already in the mindset? :) Not like I haven't done it before, but I'd have to fork the project, analyze it, coordinate, find out your coding style, etc. I'm not entirely opposed to doing it if I really want it, but is there any reason you can't?

@dunglas
Copy link
Owner

dunglas commented Mar 2, 2024

but is there any reason you can't

Time and other priorities 😅

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

No branches or pull requests

8 participants