Skip to content
This repository has been archived by the owner on Aug 10, 2024. It is now read-only.

Automatic path-to-object serialization/deserialization #20

Closed
sanity opened this issue Apr 4, 2017 · 19 comments
Closed

Automatic path-to-object serialization/deserialization #20

sanity opened this issue Apr 4, 2017 · 19 comments

Comments

@sanity
Copy link
Member

sanity commented Apr 4, 2017

Note: While it will require a decent understanding of Kotlin reflection, this is a very self-contained project and thus might be an ideal choice for someone interested in getting their feet wet as a Kweb contributor. If you want to tackle it please comment below to avoid duplicated effort

I've been working on routing, and have an approach that's both relatively simple and I think very powerful.

One thing that would add greatly to it is some code that could convert a URL path like /users/123/friends/94 into an object representation of the path, like Users(id=123, Friend(id = 94)), similar to how Gson converts JSON into object representations and back again.

For example, let's create an object representation of our webapps paths as follows:

sealed class Entity {
  data class User(val id : Int, val userEntity : UserEntity) : Entity()
  class Root() : Entity()
}

sealed class UserEntity {
  class Root() : UserEntity()
  data class Friend(val id : Int) : UserEntity()
  class Settings() : UserEntity()
}

Note the use of sealed classes, this will be nice when we use when expressions to render the paths.

Here are some paths and the object equivalents:

/users/32 -> Entity.User(32, UserEntity.Root())
/users/152/friends/22 -> Entity.User(152, UserEntity.Friend(22))
/ -> Entity.Root()

Additionally, while defaults should be sensible and consistent with REST conventions for URL format, it should be possible to use annotations to override the default translations from path to object (much as with Gson).

@jmdesprez
Copy link
Contributor

Hello,
I think that /users/152/friends/22 should become Entity.User(152, UserEntity.Friend(22)) :)

@sanity
Copy link
Member Author

sanity commented Apr 5, 2017

Good point @jmdesprez, fixed :)

@jmdesprez
Copy link
Contributor

Hello,
There another typo in the example:
data class User(val id : Int, userEntity : UserEntity) : Entity()
Should be
data class User(val id : Int, val userEntity : UserEntity) : Entity()

BTW I've start a small POC about this feature :)

For now, it's located in a dedicated project: https://github.com/jmdesprez/kweb-urlparser
I've added a TODO for each subject I'm not sure about and for questions

At some point of the algorithm, I have to fetch subclasses of the entity (Root, Friend and Settings for example). I've implemented this in two ways:

Let me know if this POC match what you have in mind :)

@sanity
Copy link
Member Author

sanity commented Apr 10, 2017

Ah, fixed the typo :)

BTW I've start a small POC about this feature :)

Awesome!!

    // TODO maybe we need a smarter way to get the plural form?
   // For now, simply add a 's' at the end

Check out this library, it might be helpful https://github.com/MehdiK/Humanizer.jvm

       // TODO better exception handling

Note: I'm using https://github.com/MicroUtils/kotlin-logging (which is an interface to slf4j).

I only have a time for a few quick comments now but will try to spend more time on it ASAP. I suggest you post to #kweb on Kotlin Slack, might get some good feedback there too.

@sanity
Copy link
Member Author

sanity commented Apr 10, 2017

A few more comments/questions :)

Minor

In tests:

val user: Optional<User> = parse("/users/152", contextProvider)

Why Optional<User> rather than User??

Conceptual, what conventions should we establish regarding "subentities"?

I realize it's probably from the examples I came up with, but I'm still not 100% sure about the UserEntity concept, or maybe not the concept as much as the name of the class and the convention it suggests. Hmmm. UserRelation perhaps? This article might provide some good inspiration.

This isn't really about your implementation, it's more about establishing some intelligent conventions - and RESTful URL naming isn't something I've given a vast amount of thought to yet. At least now we have two people to bounce ideas back-and-forth :)

/users/234 should show a user interface pertaining to User 234, showing information about them, and possibly allowing attributes of the user to be modified.

/users/234/friends/25 might show the same UI as /users/234 but with the addition of information about friend 25 - perhaps there is a list of collapsed friends on /users/234 but this friend is expanded on /users/234/friends/25?

What should /users/123/friends show, if anything?

What's the significance of /users/234 versus /users/234/?

Any thoughts on how we handle parameters, eg. /users/123?foo=bar&dog=cat?

What about the rest of the URL?

Another question is whether we should only worry about the path portion of the URL, or should this mechanism translate the entire URL to objects - possibly including the protocol (http/https), hostname, and port? eg. http://demo.kweb.io/users/123?

Sorry, I know I'm providing a lot more questions than answers- work so far is really great, I think we just need to think through a few usecases in order to establish some good conventions.

@jmdesprez
Copy link
Contributor

Sorry, I know I'm providing a lot more questions than answers- work so far is really great, I think we just need to think through a few usecases in order to establish some good conventions.

No problem, that's why I have push the project "as is" ;)

Why Optional rather than User??

I should have put a 'TODO' on this :) I've no particular reason other than liking Optional and Functional Programming :)
I think we have 3 possibilities here:

  • return an Optional
  • return null
  • throw a not found exception

I don't think one solution is better than the others, it's mainly a matter of preference. Perhaps the exception can carry an error message if needed.

About the REST API

My view of it is "data oriented", I see it like a box in a box in a box in .... That's why I like the idea of creating and entity from the URL :)

  • GET /users return a json array of users
  • GET /users/alice return a json object with the user 'alice' ('alice' must be the key)
  • GET /users/alice/friends return a json array of alice's friends
  • GET /users/alice/friends/bob return a user if Alice and Bob are friends or 404 if not

My way of visualizing this (maybe I'm wrong) is like this:

  • "/users" is a box of ... users ;)
  • inside this box I have another box named "alice". To reach it I first need to open the box of users so the URL is "/users/alice"
  • inside the "alice" box I have a box with friends of Alice
  • and so on :)

Moreover, let's assume for the example that thoses API make sens:

  • GET /friends return an array of friends
  • GET /friends/bob return the friend which have the id "bob" (we assume that a class Friend exists)
  • GET /friends/bob/users return a list of users which are friend with bob

In this example /users/alice/friends/bob and /friends/bob/users/alice are two paths which lead to the same box (or the same destination). In both case, the result is the Friendship link between Alice and Bob.

What's the significance of /users/234 versus /users/234/?

Both URL should be considered as equals I think.

How we handle parameters?

I think we can bind a parameter to a property. For example GET /search?query=kotlin&count=15 can be stored into:

  • data class Search(val query: String, val count: Int) if both parameter are mandatory
  • data class Search(val query: String, val count: Int=10) if only query is mandatory (and fetch 10 results by default)

So it's almost like a binding between a path parameter and a property but with one exception. Extra parameter should be discarded whereas an extra part in the URL should end with a 404.

For example, given the class data class Search(val query: String, val count: Int=10) the URL GET /search?query=kotlin&count=15&type=book should return a 2xx status with 15 results about kotlin (type=book is simply discarded).

What about the rest of the URL?

Including the scheme, hostname and port is a good idea. Like for the URL parameters, we can store them into the class property. Maybe we can work by convention here, if the class have a property named "domain" then we will store the domain into it. Same for "scheme" and "port". For example: data class Search(val domain: String, val query: String, val count: Int)

@sanity
Copy link
Member Author

sanity commented Apr 11, 2017

Why Optional rather than User??

I should have put a 'TODO' on this :) I've no particular reason other than liking Optional and Functional Programming :)

Ah, in Kotlin - String? solves the same problem as Optional<String> - it's a typesafe way to handle nullable types. It has all of the benefits of Optional but without the hinderance of relying on a wrapper object. It's one of the most useful features in Kotlin IMHO :) See here for more info.

About the REST API

I think what you're describing is appropriate for a REST API to be accessed programmatically, however in our case the URLs reference a user interface in a particular state, rather than just information to be presented (the UI can be interacted with, of course).

But maybe this is irrelevant here - sorry, I'm thinking out loud :)

/search?query=kotlin&count=15&type=book
data class Search(val query: String, val count: Int)

interesting, yes, that is very intuitive.

Maybe we can work by convention here, if the class have a property named "domain" then we will store the domain into it. Same for "scheme" and "port". For example: data class Search(val domain: String, val query: String, val count: Int)

Yes, although we could also have an open class with properties scheme, domain, and port, and have the user's data class subclass from that... would allow us to rely on type safety rather than just convention.

What do you think?

@sanity
Copy link
Member Author

sanity commented Apr 12, 2017

Just thinking about plurals, I think probably

/users/123 should map to the class Users(id=123), trying to get too clever for our own good by pluralizing it ourselves :)

@jmdesprez
Copy link
Contributor

in Kotlin - String? solves the same problem as Optional

Yep, I know this already ;) but I still prefer the Optional. Actually, not the Optional from JAVA (I don't like it) but I use http://www.javaslang.io/ and https://github.com/MarioAriasC/funKTionale. Both propose an Option with very handy features (map, flatMap, exists, filter, peek, and so on). So I use ? for simple cases but I switch to an Option when needed. Anyway, it's not the main subject of this discussion :-D

in our case the URLs reference a user interface in a particular state

Ok, I'm really sorry :-( I've completely miss this point :-( I've seen this project long time ago (before this conversation) and I've get a quick look then decide to put it in my "study list". I haven't realized the importance of the GUI.

/users/123 should map to the class Users(id=123)

+1 Keep it simple :)

So, I will have a closer look at Kweb asap to improve my understanding

@sanity
Copy link
Member Author

sanity commented Apr 12, 2017

Yes, sorry, I didn't mean to sidetrack us on a conversation about nullability, or to lecture you on something you were already familiar with :) I'll take a look at the libs you mention.

in our case the URLs reference a user interface in a particular state

Ok, I'm really sorry :-( I've completely miss this point :-( I've seen this project long time ago (before this conversation) and I've get a quick look then decide to put it in my "study list". I haven't realized the importance of the GUI.

Oh, well I was thinking about it and I'm actually not sure that it matters too much for the purposes of the piece you're working on. Also this stuff is mostly still in my personal working branch, so totally understandable that it might not have been clear.

The way Kweb works (or will work soon!) is that you "bind" different parts of the browser DOM to stateful data on the server. This state can be local to the user's "session", or it can be persistent state stored on disk or in a DB. If the state changes then the UI is automatically modified accordingly. This is consistent with the Single source of truth principle, combined with the observer pattern.

So, for example, I can "bind" the text within a <h1> element to a value in Shoebox, and if that value is changed by some other code, the contents of <h1> will be updated automatically.

The URL is treated as just another piece of state, which can be changed by the user (depending on what URL they visit), or by something else in the code. If the user visits a URL then Kweb builds the UI accordingly, but the programmer can also modify the URL by modifying the state, and then the UI will automatically update accordingly.

If the user visits a URL, Kweb will respond with the UI in the appropriate state, for example visiting /users/123 might show you information about user 123 and give you the ability to edit that user.

In any case, for these purposes the only goal is to have an automatic way to convert a URL into a Kotlin object and back again - so that we can encapsulate the actual structure of the URL in a typesafe way.

I'm very happy to discuss further if it would be helpful.

@sanity
Copy link
Member Author

sanity commented Apr 13, 2017

@jmdesprez I'd like to pull this into the main codebase fairly soon - and major outstanding improvements in the wings?

@jmdesprez
Copy link
Contributor

It was very interesting to have your vision of "Null safety vs. Option" and I think we can talk about this for hours :) But I was afraid to "spam" this conversation :)

If you are happy with the POC you can surely use it and pull it into the main codebase :)

But, there one point that may need to be check. I'm not absolutely sure that the cast is safe (I've put a TODO on it)

I've no idea for an improvement right now. If ideas start coming I will create a pull request in kwebio ;)

Thank you, it was a very interesting conversation (and it's a very interesting project too :) )

@sanity
Copy link
Member Author

sanity commented Apr 14, 2017

Ok @jmdesprez, initial integration done:

https://github.com/kwebio/core/blob/url-parser/src/main/kotlin/io/kweb/routing/UrlPathParser.kt
https://github.com/kwebio/core/blob/url-parser/src/test/kotlin/io/kweb/routing/UrlPathParserSpec.kt

The major thing I need to do is have it throw a UrlParseException with a descriptive error message if a URL is malformed, rather than just returning null. This will likely be returned to the user as a 404 page.

Currently unit tests will fail because of the lack of a UrlParseException.

Still getting a good understanding of your code so haven't figured out how to do it yet - any pointers appreciated :)

@sanity
Copy link
Member Author

sanity commented Apr 14, 2017

Ah, and of course I totally forgot about the other side of it, taking an object and converting it into a URL (ie. same process but in reverse).

@jmdesprez
Copy link
Contributor

jmdesprez commented Apr 14, 2017

Hello,

#23 should do the trick for the URLParseException. Because a null value now throw an Exception, I've also change the return type from T? to T

Edit: I forgot to mention that:

  • All tests are now green
  • I don't know if the message is descriptive enough.

@jmdesprez
Copy link
Contributor

Regarding the entity -> path mechanism, we may get into trouble if the entity have many constructors (because each constructor will be translated into a different URL).

I've do a small POC we can start with. To solve the problem above, I've expect it will use the primary constructor to build the URL. Sorry I don't have too much time, I've not create a POC project or tests (I've test it manually using a main and it seems to work great)

Here is the POC:

fun Any.toPath(): String {
    val entityPath = javaClass.simpleName.toLowerCase().takeUnless { it == "root" }?.let { "/$it" } ?: ""
    val members = javaClass.kotlin.members
    val params = javaClass.kotlin.primaryConstructor?.parameters?.joinToString(separator = "") { parameter ->
        val value = members.find { it.name == parameter.name }?.call(this)
        val convertedValue = when (parameter.type.javaType) {
            Int::class.java,
            Long::class.java,
            Boolean::class.java,
            String::class.java -> value
            else -> value?.toPath()
        }
        "/${convertedValue ?: ""}"
    } ?: throw IllegalArgumentException("No primary constructor found")
    return "$entityPath$params"
}

@sanity
Copy link
Member Author

sanity commented Apr 14, 2017

Awesome, I've merged your POC and added some tests for it, a few of them are failing (I didn't test any URLs ending in / since a training / is and probably should be dropped).

I have to drive to Dallas today for a meeting (7 hours 👎 ) but will try to get to this this evening if I'm not too fried, or failing that tomorrow. If you feel like taking a look in the meantime please feel free :)

Then I think with some JavaDoc it will be ready to be deployed.

@sanity
Copy link
Member Author

sanity commented Apr 15, 2017

Ok, got the unit tests working, made a few more changes, and its now pushed to master.

Still need to document - dokka isn't living up to its promise :/

@sanity
Copy link
Member Author

sanity commented May 13, 2017

Going to close this, although #25 needs to be addressed before this is usable in practice.

@sanity sanity closed this as completed May 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants