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

I am unable to specify a data type for my index, and that compromises the type safety #456

Closed
FlushBG opened this issue Aug 1, 2023 · 14 comments · Fixed by #470
Closed

Comments

@FlushBG
Copy link

FlushBG commented Aug 1, 2023

Describe the bug
I am using Orama search in a NestJS project. The use case is as follows:

  • When a module initializes (e.g. UserModule), in the onModuleInit() method a call to the database is made to retrieve all available users.
  • The data is indexed in an Orama index, to be made available for searching later and avoid unnecessary calls to the DB.
  • In case a new user is created or an existing one is updated, respective calls are made to the index to keep the data up to date.

The issue I'm facing is that I can't find a way to specify a type for the objects this index will be storing. Since the schema is only used to specify the searchable fields, I feel there should be a way to tell the index the full data schema that it will be holding. Currently, the search function returns the hits and the documents that were hit, but the return type is Document and it isn't generic. This causes some issues for other services (e.g. UserService), which are required to return an array of users - User[].

The current workaround is to either:

  • cast the document to unknown, and then to the type you need
    const user = document as unknown as User

  • manually map each property you need:

  private toUser(document: Document): User {
    return {
      id: doc['id'] as string,
      name: doc['name'] as string,
      email: doc['email'] as string,
      ...
    };
  }

To Reproduce
Steps to reproduce the behavior:

  1. Create a simple service that holds an Orama index:
@Injectable()
export class UserSearchService {
  private index: Orama;

  constructor(private dbService: DbService) {}

  async initializeIndex(): Promise<void> {
    this.index = await create({
      schema: {
        id: 'string',
        name: 'string',
        email: 'string',
      },
    });

    const users = await this.dbService.findAll<User>();
    await insertMultiple(this.index, users, 500);
  }

  async find(): Promise<User[]> {
    const result = await search(this.index, {...});

    const users = result.hits.map((hit) => hit.document);
    return users;
  }
  1. You will get a type error:
Type 'Document[]' is not assignable to type 'User[]'.
  Type 'Record<string, unknown>' is missing the following properties from type 'User': id, name, email
  1. Casting will also not work properly and will give the following error:
Conversion of type 'Document' to type 'User' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  1. The same errors occur the other way, when you are trying to update the index or insert something in it - for example, your service will receive an User object and you will not be able to insert it because it isn't of type Document.

Expected behavior
For the sake of type safety, I would expect to be able to specify the form of data my index will be holding, and avoid casting and / or mapping. A good approach would be to make all Orama methods generic, or at least the create method - so you can pass the type of your index when making it.

Example given:

const index = await create<User>({ // this will specify the full form of the data
     // and this will specify the searchable fields
      schema: {
        id: 'string',
        name: 'string',
        email: 'string',
        isActive: 'boolean',
        issuer: 'string',
        roles: 'string[]',
      },
    });

// this could either be generic or not, but the type specified at creation would guarantee the type safety of the returned document
const user = await search<User>(index);    

Additional context
The version of Orama I am facing this issue on is 1.1.1

@micheleriva
Copy link
Member

Hi @FlushBG, we're working on it. We'll open a PR to fix it ASAP.

@micheleriva
Copy link
Member

/bounty $150

@algora-pbc
Copy link

algora-pbc bot commented Aug 22, 2023

💎 $150 bounty created by micheleriva
🙋 If you start working on this, comment /attempt #456 to notify everyone
👉 To claim this bounty, submit a pull request that includes the text /claim #456 somewhere in its body
📝 Before proceeding, please make sure you can receive payouts in your country
💵 Payment arrives in your account 2-5 days after the bounty is rewarded
💯 You keep 100% of the bounty award
🙏 Thank you for contributing to oramasearch/orama!

Attempt Started (GMT+0) Solution
🟢 @evonloch Aug 22, 2023, 5:57:15 PM WIP
🟢 @H4ad Aug 24, 2023, 3:56:04 AM #470

@evonloch
Copy link

evonloch commented Aug 22, 2023

/attempt #456

Options

@H4ad
Copy link
Contributor

H4ad commented Aug 23, 2023

@micheleriva Some questions:

  • On insert, should we allow extra keys?
  • On insert, all the keys of the schema are required or optional?
  • How the return type of search should look like when we add extra keys?
  • It's okay to introduce "breaking-changes" on types like ISorter, they probably will need to introduce Orama type with the actual schema.

@micheleriva
Copy link
Member

micheleriva commented Aug 24, 2023

Hi @H4ad, let me answer your points:

On insert, should we allow extra keys?

Yes. Properties that are not defined in the schema are not searchable, but they can be inserted.

On insert, all the keys of the schema are required or optional?

They should be required. @allevo any thoughts? Should we make them optional?

How the return type of search should look like when we add extra keys?

Since we don't know what the extra properties look like, we can't determine their type. We can't also infer them as they could change from document to document.

It's okay to introduce "breaking-changes" on types like ISorter, they probably will need to introduce Orama type with the actual schema.

I'd let @allevo to answer this one

@micheleriva
Copy link
Member

@H4ad I think inferring types from the schema is good. But we could also create a generic type like:

import { search } from '@orama/orama'

type MyDoc = {
  id: 'string',
  title: 'string',
  foo?: 'number',
  bar?: 'boolean'
}

const results = search<MyDoc>(db, {
  term: ''
})

Internally, Orama could merge the MyDoc type with the inferred one from the schema. That way, we could easily manage most of the non-searchable properties.

@H4ad
Copy link
Contributor

H4ad commented Aug 24, 2023

@micheleriva For the insert, I leave all the properties optional but for most cases I allow the user to override the return object if it matches with the schema.

Some preview of the current state:

image

image

Now I'm fixing a bunch of other issues with types, I will try to create a PR on the weekend for this feature.

/attempt #456

Options

@algora-pbc
Copy link

algora-pbc bot commented Aug 24, 2023

Note: The user @evonloch is already attempting to complete issue #456 and claim the bounty. If you attempt to complete the same issue, there is a chance that @evonloch will complete the issue first, and be awarded the bounty. We recommend discussing with @evonloch and potentially collaborating on the same solution versus creating an alternate solution.

@H4ad
Copy link
Contributor

H4ad commented Aug 24, 2023

Hey @evonloch, what's your progress on this? I'm really interested in this issue.

@evonloch
Copy link

@H4ad I am still working in it.

@algora-pbc
Copy link

algora-pbc bot commented Aug 27, 2023

💡 @H4ad submitted a pull request that claims the bounty. You can visit your org dashboard to reward.
👉 @H4ad: To receive payouts, sign up on Algora, link your Github account and connect with Stripe on your dashboard.

@algora-pbc
Copy link

algora-pbc bot commented Sep 19, 2023

@H4ad: Your claim has been rewarded! 👉 Complete your Algora onboarding to collect the bounty.

@algora-pbc
Copy link

algora-pbc bot commented Sep 19, 2023

🎉🎈 @H4ad has been awarded $150! 🎈🎊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants