add ddo crud

[an7e]

PR for #48

Changes proposed in this PR:

• added schemas for ddo, nonce, indexer
• added Database main class to initialize collections and provide interaction with them
• added DdoDatabase, NonceDatabase and IndexerDatabase classes to provide CRUD methods for ddo, nonce, indexer

[paulo-ocean]

Hi @an7e
Overall, looks good, but should we not handle errors for create, retrieve and delete as well?
Why only on update?. Also, what do these functions return? Shouldn't we have some return values?
Another personal note, why do we need to pass the schemas on the constructor if we already know them?
Personally i would prefer a simpler approach,

Just my 2 cents

export class Database {
    ddo: DdoDatabase
    nonce: NonceDatabase
    indexer: IndexerDatabase

    constructor(private config: OceanNodeDBConfig, schemes: Schemes) {
        return (async (): Promise<Database> => {
            this.ddo = await new DdoDatabase(config, schemes.ddoSchemes)
            this.nonce = await new NonceDatabase(config, schemes.nonceSchema)
            this.indexer = await new IndexerDatabase(config, schemes.indexerSchema)
            return this;
        })() as unknown as Database;
    }
}

async create(id: string, nonce: number) {
        return await this.provider.collections(this.schema.name).documents().create({id, nonce})
    }

    async retrieve(id: string) {
        const result = await this.provider.collections(this.schema.name).documents().retrieve(id)
        return result.nonce
    }

    async update(id: string, nonce: number) {
        try {
            return await this.provider.collections(this.schema.name).documents().update(id, {nonce})
        } catch (error) {
            if (error instanceof TypesenseError && error.httpStatus == 404) {
                return await this.provider.collections(this.schema.name).documents().create({id, nonce})
            } else {
                throw error;
            }
        }
    }

    async delete(id: string) {
        return await this.provider.collections(this.schema.name).documents().delete(id)
    }

[jamiehewitt15]

@an7e Can you pull from main and resolve the conflicts? Also, it would help a lot if you could list the main changes in the description of PR when opening it

[an7e]

@an7e Can you pull from main and resolve the conflicts? Also, it would help a lot if you could list the main changes in the description of PR when opening it

sure, i'll do it

[an7e]

Hi @an7e Overall, looks good, but should we not handle errors for create, retrieve and delete as well? Why only on update?. Also, what do these functions return? Shouldn't we have some return values? Another personal note, why do we need to pass the schemas on the constructor if we already know them? Personally i would prefer a simpler approach,

Just my 2 cents

export class Database {
    ddo: DdoDatabase
    nonce: NonceDatabase
    indexer: IndexerDatabase

    constructor(private config: OceanNodeDBConfig, schemes: Schemes) {
        return (async (): Promise<Database> => {
            this.ddo = await new DdoDatabase(config, schemes.ddoSchemes)
            this.nonce = await new NonceDatabase(config, schemes.nonceSchema)
            this.indexer = await new IndexerDatabase(config, schemes.indexerSchema)
            return this;
        })() as unknown as Database;
    }
}

async create(id: string, nonce: number) {
        return await this.provider.collections(this.schema.name).documents().create({id, nonce})
    }

    async retrieve(id: string) {
        const result = await this.provider.collections(this.schema.name).documents().retrieve(id)
        return result.nonce
    }

    async update(id: string, nonce: number) {
        try {
            return await this.provider.collections(this.schema.name).documents().update(id, {nonce})
        } catch (error) {
            if (error instanceof TypesenseError && error.httpStatus == 404) {
                return await this.provider.collections(this.schema.name).documents().create({id, nonce})
            } else {
                throw error;
            }
        }
    }

    async delete(id: string) {
        return await this.provider.collections(this.schema.name).documents().delete(id)
    }

@paulo-ocean any methods return document object or throw TypesenseError, you can see this by looking at methods types, idk maybe your ide not highlighting types
it's a question of what behavior you want to achieve, should methods on error throw an error or just return false for example?
also in ddo collections there will be multiple collections and the behavior of methods will start to differ a lot, also I think validators will be added

[paulo-ocean]

Hi @an7e Overall, looks good, but should we not handle errors for create, retrieve and delete as well? Why only on update?. Also, what do these functions return? Shouldn't we have some return values? Another personal note, why do we need to pass the schemas on the constructor if we already know them? Personally i would prefer a simpler approach,
Just my 2 cents

export class Database {
    ddo: DdoDatabase
    nonce: NonceDatabase
    indexer: IndexerDatabase

    constructor(private config: OceanNodeDBConfig, schemes: Schemes) {
        return (async (): Promise<Database> => {
            this.ddo = await new DdoDatabase(config, schemes.ddoSchemes)
            this.nonce = await new NonceDatabase(config, schemes.nonceSchema)
            this.indexer = await new IndexerDatabase(config, schemes.indexerSchema)
            return this;
        })() as unknown as Database;
    }
}

async create(id: string, nonce: number) {
        return await this.provider.collections(this.schema.name).documents().create({id, nonce})
    }

    async retrieve(id: string) {
        const result = await this.provider.collections(this.schema.name).documents().retrieve(id)
        return result.nonce
    }

    async update(id: string, nonce: number) {
        try {
            return await this.provider.collections(this.schema.name).documents().update(id, {nonce})
        } catch (error) {
            if (error instanceof TypesenseError && error.httpStatus == 404) {
                return await this.provider.collections(this.schema.name).documents().create({id, nonce})
            } else {
                throw error;
            }
        }
    }

    async delete(id: string) {
        return await this.provider.collections(this.schema.name).documents().delete(id)
    }

@paulo-ocean any methods return document object or throw TypesenseError, you can see this by looking at methods types, idk maybe your ide not highlighting types it's a question of what behavior you want to achieve, should methods on error throw an error or just return false for example? also in ddo collections there will be multiple collections and the behavior of methods will start to differ a lot, also I think validators will be added

Well, i know that, my point is that your functions could explicitly have the return type, without having to look at the return types on the functions using the IDE :-). Maybe a boolean on create, and on delete, etc... In any case not a big deal.
I'm just curious why you check for errors on update and not on others. This works for me but will will force all the work on error detection (try/catch) and also the error logging, to any components calling these Database functions, so eventually a lot of boilerplate and repeated code in many places, that could be encapsulated inside these functions..

[paulo-ocean]

Not gonna insist more here, but i think you could maintain things simpler man :-) ..
If you choose to return null on error, i would say that is fine (personally i would prefer a boolean is some cases, but that is just me) .. but why have an extra check on the error type? and still eventually throw an error?

why not something simpler as:

async create(ddo: Record<string, any>) {
    try {
      return await 
      this.provider.collections(this.schemes[0].name).documents().create(ddo)
    } catch (error) {
    // Maybe LOG the error? Why do we have a logger if we we don't use it :-)
     return null
    }
  }

Anyway, not a deal breaker, just some opinions

[an7e]

Not gonna insist more here, but i think you could maintain things simpler man :-) .. If you choose to return null on error, i would say that is fine (personally i would prefer a boolean is some cases, but that is just me) .. but why have an extra check on the error type? and still eventually throw an error?

why not something simpler as:

async create(ddo: Record<string, any>) {
    try {
      return await 
      this.provider.collections(this.schemes[0].name).documents().create(ddo)
    } catch (error) {
    // Maybe LOG the error? Why do we have a logger if we we don't use it :-)
     return null
    }
  }

Anyway, not a deal breaker, just some opinions

@paulo-ocean The idea is that all Typesense errors are handled or return null, but if the error is not Typesense error but a system error, then it throws an error to top to let you know that something has gone wrong. If it doesn't make sense, it's no problem, I'll change it.

[paulo-ocean]

LGTM
As a suggestion , maybe you should try add 1 or 2 lines of comments once in a while, it doesn't hurt :-)
and logs :-) 👍

[alexcos20]

lgtm, let's merge, we can always solve issues later if we need to