Modify GET package endpoint to follow creates.io API specification + add DB #1
Conversation
| @@ -0,0 +1,33 @@ | |||
| import { Migration } from '@mikro-orm/migrations'; | |||
There was a problem hiding this comment.
I know it took some work, but I recommend to remove these SQLs.
We can use automatic schema generation for entities: npx mikro-orm schema:update --run
Having migration SQLs we have to maintain 2 things: migration SQLs + entities. It's easier to maintain only entities.
src/model/package.entity.ts
Outdated
| @Entity() | ||
| export class Package { | ||
| @PrimaryKey() | ||
| id!: number; |
src/packages/packages.controller.ts
Outdated
| return { | ||
| package: name, | ||
| version: version, | ||
| totalDownloads: downloads.length, |
There was a problem hiding this comment.
if we have downloads - let's remove this property (it's just downloads array length)
src/model/version.entity.ts
Outdated
| @Property() | ||
| major!: number; | ||
|
|
||
| @Property() | ||
| minor!: number; | ||
|
|
||
| @Property() | ||
| patch!: number; | ||
|
|
||
| @Property({ unique: true }) | ||
| sortableVersion!: string; |
There was a problem hiding this comment.
This approach will not work. Look on this:
There was a problem hiding this comment.
Ok I have another thought on this: let's remove all of sorting on top of DB level. If needed we will add later. We don't need it anyway now, since we don't support latest. In every case we return to the users all versions, also we don't expect this "versions" load to be very big now. So pls, remove major, minor, patch from DB. Leave only 1 field: version string (composite primary key with package_name)
src/packages/packages.controller.ts
Outdated
|
|
||
|
|
||
| @Get(':name/latest/download') | ||
| async downloadLatestPackage( |
There was a problem hiding this comment.
Let's remove this endpoint.
Our CLI tool needs information about version it downloads, because it must also update .toml file. So if client has version - can use our version specific download endpoint, not the "latest". I thought it can be useful, but no - we don't need it.
src/packages/packages.controller.ts
Outdated
| async getPackage(@Param('name') name: string, @Param('version') version: string) { | ||
| const [major, minor, patch] = version.split('.').map(Number); | ||
|
|
||
| const pkg = await this.em.findOne(Package, { name }, { populate: ['versions'] }); |
There was a problem hiding this comment.
we should be able to fetch package by name and version here already in 1 DB query
There was a problem hiding this comment.
We have package name and version. We can do a fetch for specific package and version, not fetch whole versions. Pls fix it to avoid getting all versions. If you change primary keys to composite as I suggested above it will be index.
src/upload/upload.controller.ts
Outdated
There was a problem hiding this comment.
Pls remove it for now from this merge request (you can keep a code).
We just don't do upload in milesotne 1 and milestone 2
src/packages/packages.service.ts
Outdated
There was a problem hiding this comment.
Pls remove this service from PR (keep the code for later).
We will create first packages manually, later we will add "upload" support :)
src/model/version.entity.ts
Outdated
| @Property() | ||
| major!: number; | ||
|
|
||
| @Property() | ||
| minor!: number; | ||
|
|
||
| @Property() | ||
| patch!: number; | ||
|
|
||
| @Property({ unique: true }) | ||
| sortableVersion!: string; |
There was a problem hiding this comment.
Ok I have another thought on this: let's remove all of sorting on top of DB level. If needed we will add later. We don't need it anyway now, since we don't support latest. In every case we return to the users all versions, also we don't expect this "versions" load to be very big now. So pls, remove major, minor, patch from DB. Leave only 1 field: version string (composite primary key with package_name)
src/model/version.entity.ts
Outdated
| @PrimaryKey() | ||
| id!: number; |
There was a problem hiding this comment.
Make primary key as composite key <package, version> - it will be simpler and better index.
Is probably will work if you put @PrimaryKey() on id! and version! properties.
src/model/version.entity.ts
Outdated
| @Property({ columnType: 'text' }) | ||
| packageBlob!: string; |
There was a problem hiding this comment.
Maybe:
@Property({ type: 'blob' }) // Maps to BLOB in the database
data!: Buffer;
then we will use blob data. Here we store .tar.gz. It should be also easy to download file
src/packages/packages.controller.ts
Outdated
| async getPackage(@Param('name') name: string, @Param('version') version: string) { | ||
| const [major, minor, patch] = version.split('.').map(Number); | ||
|
|
||
| const pkg = await this.em.findOne(Package, { name }, { populate: ['versions'] }); |
There was a problem hiding this comment.
We have package name and version. We can do a fetch for specific package and version, not fetch whole versions. Pls fix it to avoid getting all versions. If you change primary keys to composite as I suggested above it will be index.
src/packages/packages.controller.ts
Outdated
| download.version = ver; | ||
| await this.em.persistAndFlush(download); | ||
|
|
||
| const buffer = Buffer.from(ver.packageBlob, 'base64'); |
There was a problem hiding this comment.
probably you will not have to do this if you change version entity to use blob (it will use Buffer)
src/packages/packages.service.ts
Outdated
There was a problem hiding this comment.
Remove this file pls for now from PR. We don't support it now.
The database was initialized, endpoints for retrieving packages were added, and the POST endpoint was enhanced:
Request examples:
GET for all packages: localhost:3000/api/v1/upload
GET for a single package: localhost:3000/api/v1/packages/package_name/major.minor.patch
GET for package download: localhost:3000/api/v1/packages/package_name/major.minor.patch/download
POST for manual package upload: localhost:3000/api/v1/upload
I will continue working on this PR, as it's still necessary to implement the GET endpoint for retrieving the number of downloads: