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 support for setting user-defined context variables #620

Closed
glennrfisher opened this issue Apr 13, 2017 · 6 comments
Closed

Add support for setting user-defined context variables #620

glennrfisher opened this issue Apr 13, 2017 · 6 comments
Assignees

Comments

@glennrfisher
Copy link
Contributor

Although the SDK supports (and includes readme examples for) accessing user-defined context variables, it does not provide good support for setting user-defined context variables.

For example, the json property is a constant (let not var), preventing users from executing response.context?.json["foo"] = "bar".

(I'm not sure whether this is the right way to expose user-defined context variables, but the example is instructive.)

In addition to an email that I received, other users in the community have struggled with this.

After designing a good mechanism for setting user-defined context variables, the readme should be updated with new examples to demonstrate it.

@glennrfisher
Copy link
Contributor Author

Just had a discussion about this issue with folks on our team. In the next few weeks, we'll probably add an initializer to Context that accepts additional fields. For example:

/**
 Create a `Context` object from an existing context and user-defined context variables.

 - parameter context: An existing `Context` object.
 - parameter variables: User-defined variables that should be included in this `Context` object.
 */
public init(context: Context, variables: [String: Any]? = nil) throws {
    var json = context.json            // get previous context
    variables.forEach{ json[$0] = $1 } // add user-defined variables
    Context(JSON(dictionary: json))    // initialize object
}

This would keep the properties of the Context model immutable, but enable users to add user-defined context variables. Here's an example of how this might be used:

message(...) { response in
	let context = Context(context: response.context, variables: ["topping": "onions"])
	message(...) // send context with user-defined variables
}

@awlevin

@glennrfisher
Copy link
Contributor Author

@bfaist also suggested an alternative approach, where the json property is exposed as a var, allowing it to be modified.

That brings up a few questions:

  • The model should be consistent, so if json is a var, should the other properties be var too?
  • Should the entire json be exposed? Perhaps we want to do something similar to our Python models, where we store additionalProperties: [String: Any], which does not include any explicit JSON properties. This could be modified directly (if exposed as a var) or we could expose a function, e.g. mutating addProperty(name: String, value: Any).

@bfaist
Copy link
Contributor

bfaist commented Sep 20, 2017

I like the solution mentioned above of adding the initializer that accepts an existing Context and merges custom variables.

@glennrfisher
Copy link
Contributor Author

I opened #680 today with a number of updates to the Conversation service, including better support for getting/setting context variables.

Example

Here's an example of what it looks like. Curious to get your feedback, and whether you like/dislike it!

// setting a context variable
let request = MessageRequest(...)
conversation.message(workspaceID: "your-workspace-id", request: request) {
    response in
    var context = response.context // `var` makes the context mutable
    context?.additionalProperties["foo"] = .string("bar")
}

// getting a context variable
let request = MessageRequest(...)
conversation.message(workspaceID: "your-workspace-id", request: request) {
    response in
    if case let .string(bar) = additionalProperties["foo"]! {
        print(bar)
    }
}

Details

The Context model now defines a var additionalProperties: [String: JSON] property. This dictionary captures any context variables, and allows users to define new context variables.

The JSON type is defined as:

/// A JSON value (one of string, number, object, array, true, false, or null).
public enum JSON: Equatable, Codable {
    case null
    case boolean(Bool)
    case string(String)
    case int(Int)
    case double(Double)
    case array([JSON])
    case object([String: JSON])
}

Instead of using an Any type, this concrete JSON type ensures that any user-defined context variables are serializable into JSON. It also helps to ensure better type safety when parsing deserialized context variables. And unlike Any, the JSON type we defined can be encoded/decoded with the new JSONEncoder and JSONDecoder in Swift 4.

@glennrfisher
Copy link
Contributor Author

This was merged with #680. Waiting on a quick fix with Visual Recognition, then we'll publish an official release with these changes in a few days.

@glennrfisher
Copy link
Contributor Author

These changes are included with release v0.19.0 of the Swift SDK. Hope that helps!

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

No branches or pull requests

2 participants