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

Schemas endpoint: ignore '_metadata' key, convert 'map' type to Object #256

Merged
merged 2 commits into from
Feb 7, 2016

Conversation

wasnotrice
Copy link
Contributor

Thanks to everyone working so hard on parse-server!

Legacy Parse platform databases may have additional fields that aren't supported in the current
database-to-api-response conversion. This PR accounts for some discrepancies that appear in schemas created on the Parse platform, and newer schemas created with parse-server. For example, here's a _Role schema created on legacy Parse:

{
    "_id": "_Role",
    "_metadata": {
        "class_permissions": {
            "get": {
                "*": true
            },
            "find": {
                "*": true
            },
            "update": {
                "*": true
            },
            "create": {
                "*": true
            },
            "delete": {
                "*": true
            },
            "addField": {
                "*": true
            },
            "readUserFields": [],
            "writeUserFields": []
        }
    },
    "name": "string",
    "roles": "relation<_Role>",
    "users": "relation<_User>"
}

and here's one created with parse-server:

{
  "_id": "_Role",
  "ACL": "object",
  "createdAt": "string",
  "name": "string",
  "id": "string",
  "updatedAt": "string",
  "objectId": "string",
  "roles": "relation<_Role>",
  "users": "relation<_User>"
}
  • the _metadata field doesn't appear in the api-response version
    of Schema. Its value (an object), crashes the conversion function,
    because it isn't a string, so it doesn't have a startsWith function.
  • the 'map' type (not shown in the _Role schema) appears in legacy database representations, and seems to be the equivalent of the current object type. It was added to Schema in Added map to schema object types, fixed expiresAt #87, but it isn't incorporated into the conversion.

There may be more exceptions that these, and it may make sense to make the conversion more robust to unexpected values, rather than just adding an exception for _metadata. But this seems like a sensible place to start.

No tests, because existing tests create objects using parse-server, and those objects don't have the problematic keys. I'd love to add some, but not sure what would be the best approach.

Legacy Parse platform databases have additional fields that
database-to-api-response conversion. This commit accounts for

- the '_metadata' field, which doesn't appear in the api-response version
  of Schema, and whose value (an object), crashes the conversion function
  (which expects only string values)
- the 'map' type, which appears in legacy database representations to
  describe Objects
@drew-gross
Copy link
Contributor

Good catch! Thank you especially for testing pre-release features. Can you add some tests for these changes? Then I will merge.

@wasnotrice
Copy link
Contributor Author

@drew-gross thanks for review! I spent some time looking at how to add tests for this fix. It's tricky because we are testing a case where the data in the database isn't as it would be when created through parse-server. I don't see any examples in the specs of seeding the database with raw data.

The testing pain makes me think maybe this fix is at the wrong level. It looks like Schema intends to intercept the _metadata field, so we should probably never see it from schemas.js. There don't seem to be any specs around handling of the _metadata field. I'll look there next.

@drew-gross
Copy link
Contributor

Hmmm, I think you are right. The schemas API isn't finished yet, so maybe in the schema creation portion of the API, we can add insertion of the _metadata key. That will also enable us to improve compatibility with parse.com so it's probably a good idea anyway.

The map keys are a bit different, since those are truly legacy and we don't want to bring them back to parse-server. It might require us expose a special testing function like createLegacySchemaForTests() or something like that.

Those are both big jobs, so I'll merge this PR without those tests. If you want to keep working on this, that would be awesome :) otherwise, I'll make sure I write tests for these before we release the schemas API.

@drew-gross drew-gross closed this Feb 6, 2016
@drew-gross drew-gross reopened this Feb 6, 2016
@drew-gross
Copy link
Contributor

Whoops, looks like there are now some merge conflicts. Could you fix those and then I will merge?

@facebook-github-bot
Copy link

@wasnotrice updated the pull request.

drew-gross added a commit that referenced this pull request Feb 7, 2016
Schemas endpoint: ignore '_metadata' key, convert 'map' type to Object
@drew-gross drew-gross merged commit 034145f into parse-community:master Feb 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants