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.

Sign up for GitHub

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

Fetch compound document #105

Merged
merged 18 commits into from
Nov 20, 2024
+3,062 −1,192
Merged

Fetch compound document #105

Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
12dce95
export controllers
leoraba Oct 16, 2024
dd829bc
add default centric entity
leoraba Oct 22, 2024
7e23d19
compound view to Get data operations
leoraba Oct 22, 2024
177f102
find parent relationships within a schema hierarchy
leoraba Oct 23, 2024
324e4fa
data model documentation
leoraba Oct 28, 2024
24bf9a5
rename column name to snake case standard
leoraba Oct 28, 2024
946280c
fix typos
leoraba Oct 30, 2024
72e81e1
pluralize schema names
leoraba Nov 4, 2024
56886b5
query param entityName incompatibility
leoraba Nov 14, 2024
04377e4
refactoring code
leoraba Nov 14, 2024
6a894a1
Changing view parameter
leoraba Nov 19, 2024
7516555
code refactor - consistent use of undefined and null values
leoraba Nov 19, 2024
3e2287f
code refactor - splitting services files
leoraba Nov 19, 2024
216eec9
validate request parameter defaultCentricEntity
leoraba Nov 19, 2024
392a836
throw error on compound view
leoraba Nov 20, 2024
1f76a4e
parse view request param
leoraba Nov 20, 2024
6a0400e
remove type assertions
leoraba Nov 20, 2024
88e76aa
express parse json
leoraba Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .env.schema
View file
Original file line number Diff line number Diff line change
@@ -11,3 +11,7 @@ LECTERN_URL=
LOG_LEVEL=
PORT=3030
UPLOAD_LIMIT=''
NESTED_RECORD_PREFIX=
NESTED_RECORD_SUFFIX=
PARENT_RECORD_PREFIX=
PARENT_RECORD_SUFFIX=
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to be careful with natural language property names... not everything is pluralized with an 's'.

I'll read through how you use these properties but at first read I want to just leave the property name as the schema name, unmodified.

34 changes: 19 additions & 15 deletions README.md
View file
Original file line number Diff line number Diff line change
@@ -72,21 +72,25 @@ Create a `.env` file based on `.env.schema` located on the root folder and set t

The Environment Variables used for this application are listed in the table bellow

| Name | Description | Default |
| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- |
| `AUDIT_ENABLED` | Ensures that any modifications to the submitted data are logged, providing a way to identify who made changes and when they were made. | true |
| `DB_HOST` | Database Hostname | |
| `DB_NAME` | Database Name | |
| `DB_PASSWORD` | Database Password | |
| `DB_PORT` | Database Port | |
| `DB_USER` | Database User | |
| `ID_CUSTOM_ALPHABET` | Custom Alphabet for local ID generation | '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ' |
| `ID_CUSTOM_SIZE` | Custom size of ID for local ID generation | 21 |
| `ID_USELOCAL` | Generate ID locally | true |
| `LECTERN_URL` | Schema Service (Lectern) URL | |
| `LOG_LEVEL` | Log Level | 'info' |
| `PORT` | Server Port. | 3030 |
| `UPLOAD_LIMIT` | Limit upload file size in string or number. <br>Supported units and abbreviations are as follows and are case-insensitive: <br> - b for bytes<br> - kb for kilobytes<br>- mb for megabytes<br>- gb for gigabytes<br>- tb for terabytes<br>- pb for petabytes<br>Any other text is considered as byte | '10mb' |
| Name | Description | Default |
| ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- |
| `AUDIT_ENABLED` | Ensures that any modifications to the submitted data are logged, providing a way to identify who made changes and when they were made. | true |
| `DB_HOST` | Database Hostname | |
| `DB_NAME` | Database Name | |
| `DB_PASSWORD` | Database Password | |
| `DB_PORT` | Database Port | |
| `DB_USER` | Database User | |
| `ID_CUSTOM_ALPHABET` | Custom Alphabet for local ID generation | '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ' |
| `ID_CUSTOM_SIZE` | Custom size of ID for local ID generation | 21 |
| `ID_USELOCAL` | Generate ID locally | true |
| `LECTERN_URL` | Schema Service (Lectern) URL | |
| `LOG_LEVEL` | Log Level | 'info' |
| `NESTED_RECORD_PREFIX` | Prefix to be added to the names of nested records within a hierarchical structure Level | |
| `NESTED_RECORD_SUFFIX` | Suffix to be added to the names of nested records within a hierarchical structure Level | 's' |
| `PARENT_RECORD_PREFIX` | Prefix to be added to the names of parent records within a hierarchical structure Level | |
| `PARENT_RECORD_PREFIX` | Suffix to be added to the names of parent records within a hierarchical structure Level | |
| `PORT` | Server Port. | 3030 |
| `UPLOAD_LIMIT` | Limit upload file size in string or number. <br>Supported units and abbreviations are as follows and are case-insensitive: <br> - b for bytes<br> - kb for kilobytes<br>- mb for megabytes<br>- gb for gigabytes<br>- tb for terabytes<br>- pb for petabytes<br>Any other text is considered as byte | '10mb' |

## Script commands (Workspace)

6 changes: 6 additions & 0 deletions apps/server/src/config/server.ts
View file
Original file line number Diff line number Diff line change
@@ -39,6 +39,12 @@ export const defaultAppConfig: AppConfig = {
audit: {
enabled: getBoolean(process.env.AUDIT_ENABLED, true),
},
recordHierarchy: {
nestedRecordPrefix: process.env.NESTED_RECORD_PREFIX || '',
nestedRecordSuffix: process.env.NESTED_RECORD_SUFFIX || 's',
parentRecordPrefix: process.env.PARENT_RECORD_PREFIX || '',
parentRecordSuffix: process.env.PARENT_RECORD_SUFFIX || '',
},
},
idService: {
useLocal: getBoolean(process.env.ID_USELOCAL, true),
61 changes: 59 additions & 2 deletions apps/server/swagger/data-api.yml
View file
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@
type: string
description: ID of the category
- name: entityName
description: Array of strings to filter by entity names
description: Array of strings to filter by entity names. Ignored if view parameter is `compound`
in: query
required: false
schema:
@@ -31,6 +31,13 @@
schema:
type: integer
description: Optional query parameter to specify the number of results per page. Default value is 20
- name: view
in: query
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding view query param on GET data endpoints

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm nervous about the performance of compound object retrieval in a list... there could be many joins that are slow to resolve. I suppose if this response is paginated then this is less of a concern.

Am I correct in assuming that the purpose of the defaultCentricEntity property you are introducing is to determine which entity to use as the root when returning data in this form?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This endpoint and others that fetch data as list or compound implements pagination. with default of 20 items per page. This will alleviate fetching large mount of data.

It's correct, the property defaultCentricEntity is used to determine thee root schema in compound view

required: false
schema:
type: string
enum: ['list', 'compound']
description: Optional query parameter to specify the data representation. Choose 'list' for an unordered list of records, or 'compound' for a nested, schema-centric structure. Default is 'list'.
Copy link
Contributor

Choose a reason for hiding this comment

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

This description is a bit unclear for me. Does compound view still return a list, but each entry is built into a compound document?

This feels like the options are actually "entity only" and "compound", since we get a list response in both cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!. I see the word list is ambiguous, I could suggest even something like flat which pairs well with compound and describes the data is non-nested and

responses:
200:
description: Submitted Data
@@ -49,6 +56,49 @@
503:
$ref: '#/components/responses/ServiceUnavailableError'

/data/category/{categoryId}/id/{systemId}:
get:
summary: Retrieve Submitted Data Record for a System ID
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new Endpoint to retrieve record by it's systemID

tags:
- Data
parameters:
- name: categoryId
in: path
required: true
schema:
type: string
description: ID of the category
- name: systemId
in: path
required: true
schema:
type: string
description: ID of the record
- name: view
in: query
required: false
schema:
type: string
enum: ['list', 'compound']
Copy link
Contributor

Choose a reason for hiding this comment

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

We are returning by id, this won't ever return a 'list'. Perhaps the values need to be renamed to compound and isolated? Or maybe simple? The naming here is pretty challenging.

We could also try joined and unjoined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

compound and flat maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I like that better, good choice.

description: Optional query parameter to specify the data representation. Choose 'list' for an unordered list of records, or 'compound' for a nested, schema-centric structure. Default is 'list'.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a repeat of the content comments from the previous endpoint. The same parameter is duplicated, maybe we want to write a reusable definition for this parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved view and most used parameters to a components file to be referenced

responses:
200:
description: Submitted Data
content:
application/json:
schema:
$ref: '#/components/schemas/SubmittedDataRecord'
400:
$ref: '#/components/responses/BadRequest'
401:
$ref: '#/components/responses/UnauthorizedError'
404:
$ref: '#/components/responses/NotFound'
500:
$ref: '#/components/responses/ServerError'
503:
$ref: '#/components/responses/ServiceUnavailableError'

/data/category/{categoryId}/organization/{organization}:
get:
summary: Retrieve Submitted Data for a specific Category and Organization
@@ -68,7 +118,7 @@
type: string
description: Organization name
- name: entityName
description: Array of strings to filter by entity names
description: Array of strings to filter by entity names. Ignored if view parameter is `compound`
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

in: query
required: false
schema:
@@ -88,6 +138,13 @@
schema:
type: integer
description: Optional query parameter to specify the number of results per page. Default value is 20
- name: view
in: query
required: false
schema:
type: string
enum: ['list', 'compound']
description: Optional query parameter to specify the data representation. Choose 'list' for an unordered list of records, or 'compound' for a nested, schema-centric structure. Default is 'list'.
Copy link
Contributor

Choose a reason for hiding this comment

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

Repeat paremeter details again. Should standardize and reuse?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved view parameter as a reference.

responses:
200:
description: Submitted Data
3 changes: 3 additions & 0 deletions apps/server/swagger/dictionary-api.yml
View file
Original file line number Diff line number Diff line change
@@ -23,6 +23,9 @@
type: string
description: A matching Dictionary Version defined on Dictionary Manager (Lectern)
required: true
defaultCentricEntity:
type: string
description: The default centric entity name
Comment on lines +23 to +25
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding new input parameter on Dictionary Registration endpoint. This is value is used to retrieve compound records

responses:
200:
description: Dictionary info
17 changes: 2 additions & 15 deletions apps/server/swagger/schemas.yml
View file
Original file line number Diff line number Diff line change
@@ -425,17 +425,6 @@ components:
type: string
description: ID of the Submission

DeleteDataActiveSubmission:
type: object
properties:
records:
type: array
items:
$ref: '#/components/schemas/SubmittedDataRecord'
submissionId:
type: string
description: ID of the Submission

GetSubmittedDataResult:
type: object
properties:
@@ -503,10 +492,8 @@ components:
type: object
properties:
data:
type: array
items:
type: object
description: Content of the record in JSON format
type: object
description: Content of the record in JSON format
entityName:
type: string
isValid:
3 changes: 2 additions & 1 deletion packages/data-model/docs/schema.dbml
View file
Original file line number Diff line number Diff line change
@@ -39,7 +39,8 @@ table dictionaries {

table dictionary_categories {
id serial [pk, not null, increment]
active_dictionary_id integer
active_dictionary_id integer [not null]
defaultCentricEntity varchar
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to make this decision when we are settign the dictionary? Maybe we should instead provide the root-entity as an http request parameter, so the data requester can choose this at request time. We probably want that ability anyways, and not every dictionary will have a clear default to use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think setting a "default" entity only once could reduce complexity to data requesters. However, I also like the idea to override the default entity at request time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be nullable, with no default provided?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes defaultCentricEntity can be null in database, and if provided it must match a schema name in the dictionary.

name varchar [not null, unique]
created_at timestamp [default: `now()`]
created_by varchar
2 changes: 2 additions & 0 deletions packages/data-model/migrations/0009_add_default_centric_entity.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE "dictionary_categories" ALTER COLUMN "active_dictionary_id" SET NOT NULL;--> statement-breakpoint
Copy link

Choose a reason for hiding this comment

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

Is --> statement-breakpoint intended to be committed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The .sql file is auto generated by Drizzle Kit. unfortunately this comment is added

ALTER TABLE "dictionary_categories" ADD COLUMN "defaultCentricEntity" varchar;
Loading