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

[#IC-351] Wrap SDK Cosmos as functional (CRUD + Patch) #243

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fabriziopapi
Copy link
Contributor

Allow cosmos SDK to be used as functional programming (fp-ts style). Cover the CRUD and PATCH (partial update) operations for not versioned model.

List of Changes

Add CRUD wrapper
Add PATCH wrapper
Add propertiesToArray function to flat a nested object

Motivation and Context

We need all the object to perofmr and update (or upsert) operation on cosmos collection. With the wrapper we can modify a single collection field without knowning the entire document. The operations will be callable directly from a pipe or a flow.

How Has This Been Tested?

unit tests

Screenshots (if appropriate):

Types of changes

  • Chore (nothing changes by a user perspective)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@fabriziopapi fabriziopapi requested a review from a team as a code owner March 8, 2022 17:18
@pagopa-github-bot
Copy link
Contributor

Warnings
⚠️ This PR changes a total of 645 LOCs, that is more than a reasonable size of 250. Consider splitting the pull request into smaller ones.
⚠️ Please include a Pivotal story at the beginning of the PR title (see below).

Example of PR titles that include pivotal stories:

  • single story: [#123456] my PR title
  • multiple stories: [#123456,#123457,#123458] my PR title

Generated by 🚫 dangerJS against 3660cde

Copy link
Contributor

@balanza balanza left a comment

Choose a reason for hiding this comment

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

I see a lot of rewriting/duplication here. What's the need that motivated such work? If you looked for composability, I propose two alternative ways:

a) Wrap an instance over a method
Basically, create an instance for every method invocation and return the method itself

const create = (/* model constructor params */) => {
  const instance = new CosmosdbModel(/* model constructor params */)
  return CosmosdbModel.prototype.create.bind(instance)
}

b) Refactor cosmosdb_model with a RoRo/Module pattern
Instead of creating instances of a class, return a plain object with what you would call "public methods"

const createCosmosdbModel = (/* model constructor params */) => {
  const privateMethodA = () => /**/
  const privateMethodB = () => /**/
  
  return {
    create(newDocument) { /* */ },
    upsert(newDocument) { /* */ },
   /* other methods */
  }
}

const { create } = CosmosdbModel(/* */) // create is now a composable method

@fabriziopapi
Copy link
Contributor Author

the RoRo/Module is really an intersting solution. I'll try to rework with it.

@balanza
Copy link
Contributor

balanza commented Mar 14, 2022

the RoRo/Module is really an intersting solution. I'll try to rework with it.

Anyway, solution a can be used right now by implementers, without having to change the library

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