Skip to content

RFC for Import/Export API#3758

Draft
cmelchior wants to merge 1 commit intomainfrom
cm/import-export-rfc
Draft

RFC for Import/Export API#3758
cmelchior wants to merge 1 commit intomainfrom
cm/import-export-rfc

Conversation

@cmelchior
Copy link
Copy Markdown
Contributor

Spinoff from #1470

This RFC tries to describe two possible approaches to the problem, but perhaps I am missing something even better.

What do you think @realm/java @TR4Android

@taltstidl
Copy link
Copy Markdown

@cmelchior Thanks for the detailed proposal, I would almost say that option B is better as it's more concise if the import is done directly one the Realm database object. A few suggestions though:

  • Just name the methods importData or similar. I think the keyword import conveys what those methods are meant to do: import data from another format into the database. (I would advise against the method name import itself as it's a reserved keyword in most languages.)
  • Don't try to auto-detect the type in the import API. Most, if not all, developers know the type they're importing, adding detection just increases the likelihood that a developer uses the less efficient version of the API and introduces a lot of code that needs to be maintained.
  • Make the imported type an enum, that way it's pretty clear what types are supported for importing and there's no need to check for invalid types as would be the case for String.

Also, still an open question for me is how to guarantee the order of execution of the annotation processors involved. We need to somehow make sure that the proxy classes get generated first, so the import/export processor can add its code to it.

@cmelchior
Copy link
Copy Markdown
Contributor Author

Just name the methods importData or similar. I think the keyword import conveys what those methods are meant to do: import data from another format into the database. (I would advise against the method name import itself as it's a reserved keyword in most languages.)

importData is an option as well. My primary concern was the distinction between update or not. Right now most of our apis are called e.g insertOrUpdate/createOrUpdate, importOrUpdate just sounds wrong?

Make the imported type an enum, that way it's pretty clear what types are supported for importing and there's no need to check for invalid types as would be the case for String.

I also like a enum a lot more, but it makes the API less flexible since you are then constrained to the types we define. I'm not sure it is a problem in practise though as XML/JSON/CSV would probably support 99% of export formats.

@Zhuinden
Copy link
Copy Markdown
Contributor

Zhuinden commented Nov 8, 2016

XML/JSON/CSV would probably support 99% of export formats.

I could define flatbuffer or something ridiculous like flatbuffers


as for JSON support, we should also take into consideration the fact that you still can't directly map between RealmList<RealmInteger>, so you'd need custom serialization logic for array of integers to either convert to that, or join them into a single String field like |5|7|10| if you so desire.



I have this wild guess that the right direction would be something like the Retrofit2 Converter.Factory approach.

@cmelchior
Copy link
Copy Markdown
Contributor Author

I have this wild guess that the right direction would be something like the Retrofit2 Converter.Factory approach.

Retrofit has the luxury of making the users define the interface, so they can annotate their API with @json / @xml if I remember correctly. I really like the factory approach as well, but there will be cases where it is impossible to tell the difference, e.g If I'm importing a String is that a json or xml string.

@Zhuinden
Copy link
Copy Markdown
Contributor

Zhuinden commented Nov 8, 2016

@cmelchior

there will be cases where it is impossible to tell the difference, e.g If I'm importing a String is that a json or xml string.

uh... isn't that why the converter factory gets a list of annotations which can be used to determine if it's for @Root annotation (SimpleXML) or @JsonObject annotation (LoganSquare) for example?

@cmelchior
Copy link
Copy Markdown
Contributor Author

Yes, and that is a great idea, but users doesn't control the Realm API, so they cannot annotate the import method.

@Zhuinden
Copy link
Copy Markdown
Contributor

Zhuinden commented Nov 8, 2016

They can annotate RealmObjects though. Maybe that changes things? The key here is "automating" batch insertion, or so I would think. Although I do wonder how relationships would be handled, or custom serialization like for RealmList of RealmInts.

This is a difficult issue, I'll try to think about it; although my wild guess is that you'll come up with something even better than any idea I'd have. :D /sleeptime

Comment thread rfcs/import-export-api.md
@@ -0,0 +1,243 @@
# Import / Export API

This document describe a possible new Import/Export API that is more generalized
Copy link
Copy Markdown
Contributor

@dalinaum dalinaum Nov 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe -> describes?

Comment thread rfcs/import-export-api.md

So instead of just keep piling features onto our JSON support we would much
rather look at this from a broader perspective of getting data in and out of
Realm through a more generalized Import/Export API. Such a API should be flexible
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such a API -> Such an API?

Comment thread rfcs/import-export-api.md

## Use cases - Already supported

Realm already have `copyToRealm()/copyFromRealm()` methods that uses in-memory
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realm already have -> Realm already has?
that uses -> that use?

Comment thread rfcs/import-export-api.md
- Move towards a opt-in plugin-type architecture.

**Disadvantages**
- Methods will crash if no plugin is installed.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This disadvantage is really really minor ... I wouldn't say it is a disadvantage, it is just a bug needs to be fixed when it happens.

Comment thread rfcs/import-export-api.md
## Open questions:

* Can we enumerate possible input formats (XML, JSON, CSV), or does it need to
be flexible?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like make it flexible

Comment thread rfcs/import-export-api.md
be flexible?

* A more radical solution could also be to just remove our JSON support
completely and completely delegate it to 3rd party libraries?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The converter has to call APIs in io.realm.internal for speed concerns. I am not quite sure we have all the checks for every internal API (but ideally we should have all checks inside the internal APIs)

@eygraber
Copy link
Copy Markdown

eygraber commented Apr 5, 2018

I like @Zhuinden idea of a retrofit-esque solution. Define an interface in realm for importing data, create a library with implementations using popular 3rd party libraries (gson, Moshi, something for XML, etc...), and let the user pass an instance of that implementation to the import method.

@cmelchior cmelchior removed their assignment Sep 6, 2018
@bmunkholm bmunkholm marked this pull request as draft February 22, 2022 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants