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

feat: add support for get augmented #2469

Merged
merged 2 commits into from May 3, 2023
Merged

Conversation

schoren
Copy link
Collaborator

@schoren schoren commented May 2, 2023

This PR adds support for augmented entities. This is a feature needed, for example, in transactions. We have 2 "representations" of transactions: the one used in files, which looks like this:

type: Transaction
spec:
  id: transactionID
  name: some feature
  description: do stuff in sequence
  steps:
    - testID_1
    - testID_2

it contains just the information needed to create/update the transaction, and just a test ID to reference the step tests.

We also have another representation used in the API, that looks like this: (redacted)

{
  "id": "_2yuIhL4g",
  "name": "01- Transaction",
  "version": 1,
  "steps": [
    {
      "id": "XK1UI2L4R",
      "name": "01- Pokeshop - List",
      "description": "Get a Pokemon",
      // the rest of the test
    },
    {
      "id": "K74lShYVR",
      "name": "02 - Pokeshop - Add",
      "description": "Add a Pokemon",
      // the rest of the test
    }
  ],
  "createdAt": "2023-04-06T14:30:19.354747Z",
  "summary": {
    "runs": 2,
    "lastRun": {
      "time": "2023-04-06T14:31:00.770013Z",
      "fails": 3
    }
  }
}

With this PR, we allow resources to define "base" fields and "augmented" fields. In this example, the transaction base fields would be:

  • id
  • name
  • description
  • testIDs

And then the augmented fields would be:

  • tests
  • createdAt
  • summary

The "augmented" version is the sum of base and augmented fields.

The augmented version is requested by passing the X-Tracettest-Augmented: true header. It works on Get and List.

Since augmentation can be expensive in tems of DB querying, the ResourceManager allows resource handlers to specify methods to deal with augmented requests (GetAugmented and ListAugmented).

The resource struct can define the "augmented" fields just by making them optional. Like:

type Transaction struct{
  // this is a "base" fields, because it's not optional
  ID id.ID `mapstructure:"id"`

  // this field is augmented because it's optional (because omitempty is enabled
  Summary Summary `mapstructure:"summary,omitempty"`
}

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

@schoren schoren changed the title add support for get augmented feat: add support for get augmented May 2, 2023
@schoren schoren marked this pull request as ready for review May 2, 2023 17:08
@xoscar
Copy link
Collaborator

xoscar commented May 3, 2023

My only concern about this is the usage of a header instead of a query parameter, is there anything specifically why we would prefer one instead of the other?

@schoren
Copy link
Collaborator Author

schoren commented May 3, 2023

@xoscar My thinking is that this is a content modifier, so I think of it as content negotiation, like when requesting json vs yaml. I don't feel SUPER strong about using a header, but I think it keeps URLs clean and related to the data itself rather than the representation.

So, my thinking is: url query params: filters and other data modifiers; headers: content format negotiation.

Does that make sense?

@xoscar
Copy link
Collaborator

xoscar commented May 3, 2023

Makes sense, let's move forward with this

@schoren schoren merged commit 4c13748 into main May 3, 2023
28 checks passed
@schoren schoren deleted the resourcemanager-augmented branch May 3, 2023 17:11
schoren added a commit that referenced this pull request Jun 5, 2023
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.

None yet

3 participants