search

os-js / OS.js

OS.js - JavaScript Web Desktop Platform
https://www.os-js.org/
Other
6.87k stars 816 forks source link

Needed Interface for OSjs Search Engine #812

Open mahsashadi opened 1 year ago

mahsashadi commented 1 year ago

Hi. How can we extend osjs-search-engine? Imagine we have a search engine like Elasticsearch, what kind of interface osjs-search-engine needs to sync with our search engine?

andersevenrud commented 1 year ago

I had a look, and the Search is exported, but it's not possible to override it:

https://github.com/os-js/osjs-client/blob/a777f29e2a3b23eef6c354c8ba106777225fe1e1/src/desktop.js#L128-L133

This should probably be exposed via the DesktopServiceProvider as done with everything else.

andersevenrud commented 1 year ago

Feel free to open a PR on this (or anyone stumbling across this).

Should be a fairly easy modification :)

andersevenrud commented 1 year ago

Just thought of a thing. This could probably benefit from an Adapter pattern so that you don't have to override the actual Search abstraction, but rather have a layer on top that can gather results from different sources.

// Changed core service
import VFSSearchAdapter from './adapters/search/vfs'

class Search {
  constructor(core, options) {
    // The options would come from the Service provider
    const providedAdapters = options.adapters || []
    const useAdapters = [VFSSearchAdapter, ...providedAdapters]
    this.adapters = useAdapters.map(A => new A(core))
  }

  async init() {
    await Promise.all(this.adapters.map(a => a.init()))
  }

  async search(pattern) {
    const results = await Promise.all(this.adapters.map(a => a.search(pattern)))
    // Maybe sorted ?! Maybe provide grouping in UI ?!
    // Start simple with a flat result ?!
    return results.flat(1)
  }
}
// Changed core service initialization
this.search = core.config('search.enabled')
  ? new Search(core, options.search || {})
  : null;
// A custom adapter in the distro
// Uses the standard interface as the rest of OS.js
export default class MySearchAdapter {
  constructor(core) {
    this.core = core

    // If settings are needed it can be provided via a custom config entry
    const config = core.config('search.mySearchAdapterSetttings', {})

    this.settings = deepmerge({
     someDefault: 'value',
    }, config)
  }

  destroy() {}

  async init() {
    // For custom stuff during boot
  }

  async search(pattern) {
    return []
  }
}
// Desktop service provider can now do this
import MySearchAdapter from './SearchAdapter'

osjs.register(DesktopServiceProvider, {
  args: {
    search: {
      adapters: [MySearchAdapter],
    }
  }
});
mahsashadi commented 1 year ago

Since we have previously developed and contributed our VFS Adapter, isn't it enough to only implement search function of ours, which returns an array of files metadata?

It seems here it loops over mountpoints and execute all search functions.

mahsashadi commented 1 year ago

Before going ahead, I create a PR for search function annotation. https://github.com/os-js/osjs-server/pull/70

andersevenrud commented 1 year ago

Since we have previously developed and contributed our VFS Adapter, isn't it enough to only implement search function of ours, which returns an array of files metadata?

Yes.

But adding this could allow for custom searches, not just VFS. It could return http links for example as paths and open tabs, etc :)

mahdi00021 commented 1 year ago

@andersevenrud 

// Changed core service
import VFSSearchAdapter from './adapters/search/vfs'

class Search {
  constructor(core, options) {
    // The options would come from the Service provider
    const providedAdapters = options.adapters || []
    const useAdapters = [VFSSearchAdapter, ...providedAdapters]
    this.adapters = useAdapters.map(A => new A(core))
  }

  async init() {
    await Promise.all(this.adapters.map(a => a.init()))
  }

  async search(pattern) {
    const results = await Promise.all(this.adapters.map(a => a.search(pattern)))
    // Maybe sorted ?! Maybe provide grouping in UI ?!
    // Start simple with a flat result ?!
    return results.flat(1)
  }
}

// Changed core service initialization
this.search = core.config('search.enabled')
  ? new Search(core, options.search || {})
  : null;

Hi , Now in which file and where should we put this piece of code? And what changes should we make?

andersevenrud commented 1 year ago

This would be the search.js (core service) and desktop.js (where search is inited) respectively.

And then move the existing search implementation to a new adapter (path in import in example).

On Mon, 5 Dec 2022, 19:33 mahdi norouzi, @.***> wrote:

// Changed core service import VFSSearchAdapter from './adapters/search/vfs'

class Search { constructor(core, options) { // The options would come from the Service provider const providedAdapters = options.adapters || [] const useAdapters = [VFSSearchAdapter, ...providedAdapters] this.adapters = useAdapters.map(A => new A(core)) }

async init() { await Promise.all(this.adapters.map(a => a.init())) }

async search(pattern) { const results = await Promise.all(this.adapters.map(a => a.search(pattern))) // Maybe sorted ?! Maybe provide grouping in UI ?! // Start simple with a flat result ?! return results.flat(1) } }

// Changed core service initialization this.search = core.config('search.enabled') ? new Search(core, options.search || {}) : null;

Hi , Now in which file and where should we put this piece of code? And what changes should we make?

— Reply to this email directly, view it on GitHub https://github.com/os-js/OS.js/issues/812#issuecomment-1337911285, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABHODAQMXS2AW67YO2UZUTWLYYPPANCNFSM6AAAAAARMWS7IM . You are receiving this because you commented.Message ID: <os-js/OS. @.***>

mahdi00021 commented 1 year ago

@andersevenrud

Should a part of the search be deleted or how can I replace it?

andersevenrud commented 1 year ago

I can't go into much more detail because I'm on mobile.

The brief overview of code changes I gave should be enough to get started. It's just a matter of moving some code and then implement an adapter pattern + pass on some options from provider.

Open a PR as it's easier to do reviews than me just writing out the code myself in issues.

On Mon, 5 Dec 2022, 20:57 mahdi norouzi, @.***> wrote:

@andersevenrud https://github.com/andersevenrud

Should a part of the search be deleted or how can I replace it?

— Reply to this email directly, view it on GitHub https://github.com/os-js/OS.js/issues/812#issuecomment-1338086321, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABHODHAJE3BVJMPVPKLZWLWLZCJLANCNFSM6AAAAAARMWS7IM . You are receiving this because you were mentioned.Message ID: <os-js/OS. @.***>

mahdi00021 commented 1 year ago

@andersevenrud hi this file search.js : Should I change like this?

I am newcomer to this project

export default class Search {
  /**
   * Create Search instance
   * @param {Core} core Core reference
   */
  constructor(core, options) {
     const providedAdapters = options.adapters || []
    const useAdapters = [VFSSearchAdapter, ...providedAdapters]
    this.adapters = useAdapters.map(A => new A(core))

    /**
     * Core instance reference
     * @type {Core}
     * @readonly
     */
    this.core = core;

    /**
     * Wired actions
     * @type {Object}
     */
    this.ui = null;

    /**
     * Last focused window
     * @type {Window}
     */
    this.focusLastWindow = null;

    /**
     * Search root DOM element
     * @type {Element}
     * @readonly
     */
    this.$element = document.createElement('div');

  }

 init() {
    const {icon} = this.core.make('osjs/theme');
    const _ = this.core.make('osjs/locale').translate;

    this.$element.className = 'osjs-search';
    this.core.$root.appendChild(this.$element);

    this.core.make('osjs/tray').create({
      title: _('LBL_SEARCH_TOOLTOP', 'F3'),
      icon: icon('system-search.png')
    }, () => this.show());

    this.ui = createUI(this.core, this.$element);
    this.ui.on('hide', () => this.hide());
    this.ui.on('open', iter => this.core.open(iter));
    this.ui.on('search', query => {
      this.search(query)
        .then(results => this.ui.emit('success', results))
        .catch(error => this.ui.emit('error', error));
    });
  await Promise.all(this.adapters.map(a => a.init()))

  }

search(pattern) {
 const results = await Promise.all(this.adapters.map(a => a.search(pattern)))
    // Maybe sorted ?! Maybe provide grouping in UI ?!
    // Start simple with a flat result ?!

    const vfs = this.core.make('osjs/vfs');
    const promises = this.core.make('osjs/fs')
      .mountpoints()
      .map(mount => `${mount.name}:/`)
      .map(path => {
        return vfs.search({path}, pattern)
          .catch(error => {
            logger.warn('Error while searching', error);
            return [];
          });
      });

     return results.flat(1)
  }

....
....

and file desktop.js in constructor:
Should I change like this?

    /**
     * Search instance
     * @type {Search|null}
     * @readonly
     */
    this.search = core.config('search.enabled') ? new Search(core) : null;
andersevenrud commented 1 year ago

Please open a PR so I can write reviews. It's a bit hard in issues.

On Thu, 8 Dec 2022, 20:35 mahdi norouzi, @.***> wrote:

@andersevenrud https://github.com/andersevenrud hi this file search.js : Should I change like this?

I am newcomer to this project

export default class Search { /**

  • Create Search instance

  • @param {Core} core Core reference */ constructor(core, options) { const providedAdapters = options.adapters || [] const useAdapters = [VFSSearchAdapter, ...providedAdapters] this.adapters = useAdapters.map(A => new A(core))

    /**

    • Core instance reference
    • @type {Core}
    • @readonly */ this.core = core;

    /**

    • Wired actions
    • @type {Object} */ this.ui = null;

    /**

    • Last focused window
    • @type {Window} */ this.focusLastWindow = null;

    /**

    • Search root DOM element
    • @type {Element}
    • @readonly */ this.$element = document.createElement('div');

    }

    init() { const {icon} = this.core.make('osjs/theme'); const _ = this.core.make('osjs/locale').translate;

    this.$element.className = 'osjs-search'; this.core.$root.appendChild(this.$element);

    this.core.make('osjs/tray').create({ title: _('LBL_SEARCH_TOOLTOP', 'F3'), icon: icon('system-search.png') }, () => this.show());

    this.ui = createUI(this.core, this.$element); this.ui.on('hide', () => this.hide()); this.ui.on('open', iter => this.core.open(iter)); this.ui.on('search', query => { this.search(query) .then(results => this.ui.emit('success', results)) .catch(error => this.ui.emit('error', error)); }); await Promise.all(this.adapters.map(a => a.init()))

    }

search(pattern) { const results = await Promise.all(this.adapters.map(a => a.search(pattern))) // Maybe sorted ?! Maybe provide grouping in UI ?! // Start simple with a flat result ?!

const vfs = this.core.make('osjs/vfs');
const promises = this.core.make('osjs/fs')
  .mountpoints()
  .map(mount => `${mount.name}:/`)
  .map(path => {
    return vfs.search({path}, pattern)
      .catch(error => {
        logger.warn('Error while searching', error);
        return [];
      });
  });

 return results.flat(1)

}

.... ....

and file desktop.js in constructor: Should I change like this?

/**
 * Search instance
 * @type {Search|null}
 * @readonly
 */
this.search = core.config('search.enabled') ? new Search(core) : null;

— Reply to this email directly, view it on GitHub https://github.com/os-js/OS.js/issues/812#issuecomment-1343258396, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABHODAR53ICPQHH7R5WRDLWMI2AFANCNFSM6AAAAAARMWS7IM . You are receiving this because you were mentioned.Message ID: <os-js/OS. @.***>

mahdi00021 commented 1 year ago

I wanted to know that I did the change right?

andersevenrud commented 1 year ago

It's not fully correct.

If you open a PR i can make annotatilns where you need to fix stuff. On Thu, 8 Dec 2022, 20:39 mahdi norouzi, @.***> wrote:

I wanted to know that I did the change right?

— Reply to this email directly, view it on GitHub https://github.com/os-js/OS.js/issues/812#issuecomment-1343261915, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABHODBMX7PA6O4UVWWTQPLWMI2OHANCNFSM6AAAAAARMWS7IM . You are receiving this because you were mentioned.Message ID: <os-js/OS. @.***>

mahdi00021 commented 1 year ago

@andersevenrud i opened pr for your reviews

mahdi00021 commented 1 year ago

@andersevenrud Many ambiguities have arisen for me in relation to changing the codes, for example, which sections should be deleted, which sections should remain, or which sections should be defined.

andersevenrud commented 1 year ago

Many ambiguities have arisen for me in relation to changing the codes, for example, which sections should be deleted, which sections should remain, or which sections should be defined.\

That was kind of the whole point of this. I wrote some ideas and suggestions on how it could be solved.

If I were to point out (or write here) exactly what sections should be deleted/changed, I could just have written it myself.

andersevenrud commented 1 year ago

I wanted this to be an easy exercise into getting familiar with how everything works, and I thought that I had explained myself clearly.

But maybe there is some kind of language barrier here or some knowledge gap which makes it very hard for me to give the information needed.

andersevenrud commented 1 year ago

Based on my comments, this is basically what's needed to make this work as I explained:

New adapter file src/adapters/search/vfs.js

export default class VFSSearchAdapter {
  constructor(core) {
    this.core = core
  }

  destroy() {}

  async init() {}

  async search(pattern) {
    const vfs = this.core.make('osjs/vfs');
    const promises = this.core.make('osjs/fs')
      .mountpoints()
      .map(mount => `${mount.name}:/`)
      .map(path => {
        return vfs.search({path}, pattern)
          .catch(error => {
            logger.warn('Error while searching', error);
            return [];
          });
      });

    return Promise.all(promises)
      .then(lists => [].concat(...lists));
  }
}

Changes in src/search.js

// New import to adapter that replaces search function
import VFSSearchAdapter from './adapters/search/vfs.js';

export default class Search {
  constructor(core) {
    // ...
    // New lines on bottom
    const providedAdapters = options.adapters || []
    const useAdapters = [VFSSearchAdapter, ...providedAdapters]
    this.adapters = useAdapters.map(A => new A(core))
  }

  async destroy() {
    // ...
    // New lines on bottom
    await this.adapters.map(a => a.destroy())
  }

  /**
   * Initializes Search Service
   */
  init() {
    // ...
    // New lines on bottom
    await Promise.all(this.adapters.map(a => a.init()))
  }

  async search(pattern) {
    // Replace with this
    const results = await Promise.all(this.adapters.map(a => a.search(pattern)))
    return results.flat(1)
  }
}

Changes in src/desktop.js

export default class Desktop extends EventEmitter {
  constructor(core, options = {}) {
    // Replace the old construtor with this
    this.search = core.config('search.enabled')
      ? new Search(core, options.search || {})
      : null;
  }
}
mahdi00021 commented 1 year ago

@andersevenrud very thanks i again reviews code

mahdi00021 commented 1 year ago

@andersevenrud

Excuse me, one question how MySearchAdapter be connected (I mean is override) to search through the DesktopServiceProvider ? what is bridge between these?