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

VPID replacement in arrays and serialized data #829

Closed
borekb opened this Issue Mar 30, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@borekb
Copy link
Member

borekb commented Mar 30, 2016

In references, we currently handle single-value integers only. For example, post_author containing 123 is processed fine. However, some fields contain arrays of integers (e.g., 1, 2, 3 in widgets – #814) or serialized data (e.g., "a:3:{i:0;i:3;i:1;i:5;i:2;i:7;}" in sticky posts – #819).

This ticket was created to discuss how to tackle this. There are two possible approaches:

  1. Enhance our schema format to allow an expression like "this option is a reference to entity XYZ, and, actually, expect an array of references / serialized structure here". Something like sticky_posts: post (SERIALIZED) (pseudocode).
  2. Process every value as if it could be integer, an array of integers or serialized structure. In this approach, the schema would always contain just a simple reference declaration, e.g., sticky_posts: post but the function doing the actual ID -> VPID translation would account for all the possible value types. For example, if it received 1, it would replace it with a single VPID. If it received 1, 2, 3, it would replace it with <VPID1>, <VPID2>, <VPID3>, etc.

The first approach has the advantage that it is very explicit, the later would keep the schema format simple and would be slightly more flexible. We need to discuss which way to go.

@JanVoracek

This comment has been minimized.

Copy link
Member

JanVoracek commented Mar 31, 2016

We should definitely define exact locations in serialized data where a database ID will be replaced with VPID. We cannot just replace all integers with VPIDs as they could target to different types of entities or could be just simple integers.

@JanVoracek

This comment has been minimized.

Copy link
Member

JanVoracek commented Mar 31, 2016

Here is an example from WordPress (widget_pages):

a:2:{
  i:2;a:3:{
    s:5:"title";s:4:"Test";
    s:6:"sortby";s:10:"post_title";
    s:7:"exclude";s:3:"1,2";
  }
  s:12:"_multiwidget";i:1;
}

In this example we should replace the list of IDs at widget_pages[2]["exclude"] but the integer at widget_pages["_multiwidget"] should stay.

@borekb

This comment has been minimized.

Copy link
Member

borekb commented Mar 31, 2016

True, serialized data are more tricky. Your example is actually pretty interesting because it combines two things: serialized value in the first place and then the exclude field which contains a list of IDs. Serialized data seem like something that needs to be described in the schema.yml, however, single integers vs. arrays could still perhaps be handled uniformly. Or do you think it's necessary to write something like:

some_option: post[]

in the schema? I think it could be just

some_option: post

and the code would handle both single integers and arrays of them.

@JanVoracek

This comment has been minimized.

Copy link
Member

JanVoracek commented Mar 31, 2016

I think that some_option: post handling multiple posts is too magic. On the other hand, it could make the writing of schema files easier and it could be also easier to implement.

@borekb

This comment has been minimized.

Copy link
Member

borekb commented Mar 31, 2016

I feel the same, it has both upsides and downsides.

The use case I have in mind when pushing the unified handling approach: a developer who wants to write a piece of schema sees a single integer in some_field in the database and adds some_field: entity to the schema. However, what he missed is that sometimes, depending on the actual data, it might be an array. And VersionPress crashes because it cannot parse the value.

So I think we should have a graceful handling of this anyway, and when we do, adding [] or some other array indicator to the schema is technically optional.

@borekb borekb referenced this issue Mar 31, 2016

Closed

IDs in widgets #814

@JanVoracek JanVoracek self-assigned this Mar 31, 2016

JanVoracek added a commit that referenced this issue Apr 4, 2016

@JanVoracek JanVoracek referenced this issue Apr 5, 2016

Merged

VPIDs in serialized data #855

1 of 1 task complete

JanVoracek added a commit that referenced this issue Apr 6, 2016

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