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

Object model surface area and nomenclature changes (v2) #5

Closed
christopheranderson opened this issue Jun 4, 2018 · 16 comments
Closed
Labels
breaking non-additive functional behavior changes feature Adds or changes functional behavior specification Used to track issues which capture specification (as opposed to work)
Projects
Milestone

Comments

@christopheranderson
Copy link
Contributor

christopheranderson commented Jun 4, 2018

Goals

For each of the objects we operate on (database, collection, document, etc.), we want to move their operations off of DocumentClient and onto objects which represent those operations. We also want to rename those objects to better capture their functionality in today's Cosmos terms (as opposed to legacy Document DB terms). We want the SDK to be easy to use & grok (understand).

Nomenclature updates

Current New Notes
DocumentDB Cosmos
DocumentClient CosmosClient
DatabaseAccount DatabaseAccount No Change
Database Database No Change
User ? Still in discussion
User.* ? Still in discussion
Collection Container
Document Item
Attachment Attachment No Change
Conflict ?
Feed ?
Offers ?
PartitionKeyRanges ?
Stored Procedures ?
Triggers ?
User Defined Functions ?

Surface Area updates

A few notable changes:

  • We'll be moving the operations for a given object to the parent of that object. (aka container will have operations for readItem on it).
  • We'll have a "fluent" style API which will replace the current URI based model of referencing objects. (aka await client.getDatabase("foo").getContainer("bar").readItem("baz"))
  • "builder" operations (basically get operations) will be sync and not talk to server
  • QCRRUD operations will be async (and will talk to the server)

We take some inspiration from REST models, but have a goal of being easy to understand/code versus being technically pure.

Examples

Query Items

const {result: items} = await client
    .getDatabase(databaseName)
    .getContainer(containerName)
    .queryItems("select * from C")
    .toArray();

Note: Might want to support queryItems (for interfaces) to allow users to get help on the interface of items.

Pass a container object around

Register collection with DI framework

ioc.register("todoCollection", client.getDatabase(databaseName).getContainer(containerName));

Consume container in route logic

public async getTodos(@Injectable("todoContainer") Container todoContainer) {
    const {result: todos} = await todoContainer.readItems();
    return todos;
}

Objects

Heirarchy

  • A given object can have many children

    • "A database has collections" (1:N)
  • A given object only has 1 parent

    • "A collection has a database" (1:1)
  • client

    • database account
    • offer
    • databases
      • database
      • users
        • user
          • permissions
            • permission
      • containers
        -container
        - items
        - item
        - attachments
        - attachment
        - triggers
        - trigger
        - user defined functions
        - user defined function
        - stored procedures
        - stored procedure

Overall pattern will be:

const parent // any given parent object
parent.children.query // query many
parent.children.read //read many
parent.child("foo").read //read one
parent.child("foo").replace // replace one
parent.child.parent

CosmosClient

  • #.getDatabaseAccount(options?: RequestOptions) => Promise<DatabaseAccount>
  • #.databases: Databases
  • #.offers: Offers

DatabaseAccount

???

Databases

  • #.getDatabase(id: string) => Database
  • #.query(query: string | SqlQuerySpec, options?: FeedOptions) => QueryIterator<DatabaseDefinition>
  • #.create(body: object, options?: RequestOptions) => Promise<Response<DatabaseDefinition>>
  • #.read(options?: FeedOptions) => QueryIteratory<DatabaseDefinition>
    • TBD: Worried about confusion with getDatabase

Database

  • #.id
  • #.containers: Containers
  • #.read(options?: RequestOptions) => Promise<Response<DatabaseDefinition>>
  • #.replace(options?: RequestOptions) => Promise<Response<DatabaseDefinition>>
  • #.delete(options?: RequestOptions) => Promise<Response<DatabaseDefinition>>

Containers

  • #.getContainer(id: string) => Container
  • #.query(query: string | SqlQuerySpec, options?: FeedOptions) => QueryIterator<ContainerDefinition>
  • #.create(body: object, options?: RequestOptions) => Promise<Response<ContainerDefinition>>
  • #.read(options?: FeedOptions) => QueryIterator<ContainerDefinition>

Container

  • #.id
  • #.database: Database
  • #.items: Items
  • #.read(name: string, options?: RequestOptions) => Promise<Response<ContainerDefinition>>
  • #.replace(name: string, body: ContainerDefinition, options?: RequestOptions) => Promise<Response<ContainerDefinition>>
  • #.delete(name: string, options?: RequestOptions) => Promise<Response<ContainerDefinition>>

Items

  • #.getItem(id: string, pk?: string) => Item
  • #.query(query: string | SqlQuerySpec, options?: FeedOptions) => QueryIterator<?>
  • #.read(options?: FeedOptions) => QueryIterator<?>
  • #.create<T>(body: T, options?: RequestOptions) => Promise<Response<T>>
  • #.upsert<T>(body: T, options?: RequestOptions) => Promise<Response<T>>

Item

  • #.id
  • #.primaryKey
  • #.container: Container
  • #.readItem<T>(id: string, options?: RequestOptions) => Promise<Response<T>>
  • #.replaceItem<T>(id: string, body: T, options?: RequestOptions) => Promise<Response<T>>
    - Do we need this id? How do we sniff out if we're using a partition resolver?
  • #.deleteItem<T>(id: string, options?: ResquestOptions) => Promise<Response<T>>

StoredProcedures

  • #.getStoredProcedure(id: string) => StoredProcedure
  • #.query(query: string | SqlQuerySpec, options?: FeedOptions) => QueryIterator<StoredProcedureDefinition>
  • #.read(options?: FeedOptions) => QueryIterator<StoredProcedureDefinition>
  • #.create(body: StoredProcedureDefinition, options?: RequestOptions) => Promise<Response<StoredProcedureDefinition>>
  • #.upsert(body: StoredProcedureDefinition, options?: RequestOptions) => Promise<Response<StoredProcedureDefinition>>

StoredProcedure

  • #.id
  • #.container: Container
  • #.read(options?: RequestOptions) => Promise<Response<StoredProcedureDefinition>>
  • #.replace(body: StoredProcedureDefinition, options?: RequestOptions) => Promise<Response<StoredProcedureDefinition>>
  • #.delete(options?: RequestOptions) => Promise<Response<StoredProcedureDefinition>>
  • #.execute<T>(params?: any[], options?: RequestOptions) => Promise<Response<T>>

Triggers

  • #.getTrigger(id: string) => Trigger
  • #.query(query: string | SqlQuerySpec, options?: FeedOptions) => QueryIterator<TriggerDefinition>
  • #.read(options?: FeedOptions) => QueryIterator<TriggerDefinition>
  • #.create(body: TriggerDefinition, options?: RequestOptions) => Promise<Response<TriggerDefinition>>
  • #.upsert(body: TriggerDefinition, options?: RequestOptions) => Promise<Response<TriggerDefinition>>

Trigger

  • #.id
  • #.container: Container
  • #.read(options?: RequestOptions) => Promise<Response<TriggerDefinition>>
  • #.replace(body: TriggerDefinition, options?: RequestOptions) => Promise<Response<TriggerDefinition>>
  • #.delete(options?: RequestOptions) => Promise<Response<TriggerDefinition>>

UserDefinedFunctions

  • #.getUserDefinedFunction(id: string) => UserDefinedFunction
  • #.query(query: string | SqlQuerySpec, options?: FeedOptions) => QueryIterator<UserDefinedFunctionDefinition>
  • #.read(options?: FeedOptions) => QueryIterator<UserDefinedFunctionDefinition>
  • #.create(body: UserDefinedFunctionDefinition, options?: RequestOptions) => Promise<Response<UserDefinedFunctionDefinition>>
  • #.upsert(body: UserDefinedFunctionDefinition, options?: RequestOptions) => Promise<Response<UserDefinedFunctionDefinition>>

UserDefinedFunction

  • #.id
  • #.container: Container
  • #.read(id: string, options?: RequestOptions) => Promise<Response<UserDefinedFunctionDefinition>>
  • #.replace(id: string, body: UserDefinedFunctionDefinition, options?: RequestOptions) => Promise<Response<UserDefinedFunctionDefinition>>
  • #.delete(id: string, options?: RequestOptions) => Promise<Response<UserDefinedFunctionDefinition>>

Offers

  • #.getOffer(id: string) => Offer
  • #.queryOffers(query: string | SqlQuerySpec, options?: FeedOptions) => QueryIterator<OfferDefinition>
  • #.readOffers(options?: FeedOptions) => QueryIterator<OfferDefinition>

Offer

  • #.id
  • #.readOffer(id: string, options?: RequestOptions) => Promise<Response<OfferDefinition>>
  • #.replaceOffer(body: OfferDefinition, options?: RequestOptions) => Promise<Response<OfferDefinition>>

User

  • #.id
  • ???

Open Questions

  • Most of the API is top down, but can we put bottom up calls where it makes sense?

    For instance, client.readDatabase("foo") instead being client.database("foo").read()

  • What are we doing with Users?

  • Are we going to continue to support partition resolver?

Change Log

  • 2018-06-13: Modified the API structure to isolate operations to their representative objects
@christopheranderson christopheranderson added feature Adds or changes functional behavior breaking non-additive functional behavior changes specification Used to track issues which capture specification (as opposed to work) labels Jun 4, 2018
@christopheranderson
Copy link
Contributor Author

Suggested change:

I think it makes sense to allow objects to be able to modify themselves and to create objects that can be self-referencing.

The objects would still be stateless (save for the coordinates from the factory i.e. ids and partition keys)

Examples:

Container

#.database(): Database // point to parent database
#.read(options?: RequestOptions) => Promise<Response<ContainerDefinition>>
#.replace(body: T, options?: RequestOptions) => Promise<Response<ContainerDefinition>>
#.delete(options?: RequestOptions) => Promise<Response<ContainerDefinition>>

Example:

// check if this container has a sibling(s) with a suffix
async function findSiblings(c: Container): Container[] {
    const {result: siblings} = await c.database.query(`SELECT * FROM d WHERE STARTSWITH(${c.id})-`).toArray();
    return siblings;
}

Item

#.container(): Container  // points to parent container
#.read<T>(options?: RequestOptions) => Promise<Response<T>>
#.replace<T>(body: T, options?: RequestOptions) => Promise<Response<T>>
#.delete<T>(options?: RequestOptions) => Promise<Response<T>>

somewhat contrived example:

function getTodoForFutureUpdates(c: Container, todoId: string, userId: string): Item {
    return c.getItem(userId, todoId);
}

Stored Procedure

#.container(): Container  // points to parent container
#.execute<T>(params: any[], options?: RequestOptions) => Promise<Response<T>>
#.read<T>(options?: RequestOptions) => Promise<Response<T>>
#.replace<T>(body: T, options?: RequestOptions) => Promise<Response<T>>
#.delete<T>(options?: RequestOptions) => Promise<Response<T>>

Current:

c.executeStoredProcedure("fizzbuzz");
c.executeStoredProcedure("fizzbuzz");

Suggested:

(Current continues to work)

const sp = c.getStoredProcedure("fizzbuzz");
sp.execute();
sp.execute();
sp.execute();

@burkeholland
Copy link
Member

Container vs Collection

As a developer, "container" feels awkward to me. I don't know if that's because I'm used to seeing "collection", but I am wracking my brain and I cannot think of any other instances where I have used "container" in a NoSQL context.

@christopheranderson
Copy link
Contributor Author

re: Container vs Collection - @kirankumarkolli & @kirillg, could you comment?

@ausfeldt
Copy link

ausfeldt commented Jun 6, 2018

Assuming that there is not another contending name, I prefer "collection" over "container". Container I have mostly seen in a GUI context. It does feel awkward.

@christopheranderson
Copy link
Contributor Author

@burkeholland RE: Container vs Collection

Because we're multi-model, we're starting to rename the base concepts to more generic names, so we're not overloading traditional NoSQL names (Collection/Document/etc.) when we're doing Graph and Cassandra operations on the same data set, in the future.

We could, however, offer a shim or typedef for Collection & Document for folks looking for traditional NoSQL APIs, that are just direct representations of our Container/Item concepts. Do you think that might address your concern?

@burkeholland
Copy link
Member

burkeholland commented Jun 14, 2018 via email

@christopheranderson
Copy link
Contributor Author

It's a good question. We'd have to document them both, is my assumption. Same doc or different doc. I'm not sure how/when we're changing our top level messaging/portal UI/etc.

If it's an interesting idea to you, I can work it out with @kirillg in more detail.

@burkeholland
Copy link
Member

burkeholland commented Jun 16, 2018 via email

@christopheranderson
Copy link
Contributor Author

It's fair feedback on the naming, though. We've had a few folks mention it. We have good reasons to change the name for other consumers (like Mongo/Gremlin/etc.) to avoid overloading the Document NoSQL stuff for each of them, but that plan does mean impacting the folks using Cosmos for the Document style interaction. I definitely want to see what can be done to prevent this causing confusion.

We're doing user testing with the new names & API style. That will hopefully give us some more feedback in a controlled setting.

@christopheranderson
Copy link
Contributor Author

Suggestion:

All the .get*(id) calls should just be called .get(id), though they will all defer in which types they return, still.

I think it's self explanatory that databases.get("foo") will return a database, and it saves on line length/etc.

@christopheranderson
Copy link
Contributor Author

Suggestion: (Needs more investigation)

Could support a Proxy on the plural objects that have .get calls that will proxy calls to non-reserved indexes to .get.

So you can do:
client.databases["foo"].read()

instead of

client.databases.get("foo").read();

It really pays off once you add two layers:

client.databases.foo.containers.bar.items.get("baz","qux")
vs
client.databases.get("foo").containers.get("bar").items.get("baz", "qux")

.get would still work because Proxy would just proxy to it

Strawman implementation:

type ProxiedDatabases = Databases | [database: string]: Database;

const databases = new Databases(this);
this.databases = new Proxy(databases, {
    get: (obj, prop) => obj.get(prop);
Pros:
  • Faster to access things
  • Avoids overhead of ".get" call
  • Matches C# API
Cons:
  • Will add overhead to every single call since it has to go through the proxy.
  • It's not a well known pattern in JavaScript (Proxy is very new). It might lead to weird things that are hard to explain.

@christopheranderson
Copy link
Contributor Author

Suggestion:

.get accepts an id, but it should also be able to accept an object that has an id property on it

@christopheranderson
Copy link
Contributor Author

Suggestion:

Things like permissions require a link to a resource. It takes a resource url string, but it should also be able to take an object that has a url property on it.

@southpolesteve
Copy link
Contributor

👍 to both of @christopheranderson suggestion. I particularly found that overloading .get would have helped when refactoring the tests.

@christopheranderson
Copy link
Contributor Author

This was mostly done in #26. Some minor tweaks happened in response to feedback covered in #39.

@christopheranderson
Copy link
Contributor Author

This feedback has been addressed or moved to independent issues, so I'm closing.

Kanban automation moved this from In Progress to Done Jul 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking non-additive functional behavior changes feature Adds or changes functional behavior specification Used to track issues which capture specification (as opposed to work)
Projects
No open projects
Kanban
  
Done
Development

No branches or pull requests

4 participants