os-js / osjs-client

OS.js Client Module
https://manual.os-js.org/
Other
31 stars 31 forks source link

Add option interface for VFS readdir #163

Open mahsashadi opened 2 years ago

mahsashadi commented 2 years ago

Hi I plan to add gradual fetching of data by scrolling down in File Manger Application. Due to this issue, It seems default system adapter does not send options to the server correctly.

andersevenrud commented 2 years ago

NB: I changed the title of this issue

mahsashadi commented 2 years ago

I examined the code.

In addition to explicitly setting options to {} in this line, I think we are loosing the set options, in this line. Am I right?

andersevenrud commented 2 years ago

Ah, yes. I think I have an open issue about not properly encoding the options in GET requests. That needs to be looked at!

mahsashadi commented 2 years ago

In addition to replacing options : {} with options in this line, we can do the following changes in two mentioned files:

// osjs-client/src/utils/fetch.js 
// new encodeQueryData implementation

const encodeQueryData = (data, nesting = '') => {
  const pairs = Object.entries(data).map(([key, val]) => {
    if (typeof val === 'object') {
      return encodeQueryData(val, nesting + `${key}.`);
    } else {
      return encodeURIComponent(nesting + key) + '=' + encodeURIComponent(val);
    }
  });
  return pairs.join('&');
};
// osjs-server/src/vfs.js 
// new createOptions implementation
const createOptions = req => {
  const options = req.fields.options ? req.fields.options : decodeOptions(req.fields);
  // the rest code
}
const decodeOptions = (reqFileds) => {
  let options = {};
  Object.keys(reqFileds).map( item => {
      let keyPath = item.split(".");
      assignObject(options, keyPath,reqFileds[item]);
  })
    return options
}

const assignObject = (obj, keyPath, value) => {
  let lastKeyIndex = keyPath.length - 1;
  for (let i = 0; i < lastKeyIndex; ++i) {
    let key = keyPath[i];
    if (!(key in obj)) {
      obj[key] = {};
    }
    obj = obj[key];
  }
  obj[keyPath[lastKeyIndex]] = value;
};

If you agree with these implementations, I can make pull requests.

andersevenrud commented 2 years ago

Open up a PR and I will do a code review there :) I have some comments on the server integration there.

mahsashadi commented 2 years ago

https://github.com/os-js/osjs-client/pull/164 https://github.com/os-js/osjs-client/pull/165 https://github.com/os-js/osjs-server/pull/56

andersevenrud commented 2 years ago

Thanks! I was going to look at these today, but I've been at work all day and now realize it's almost midnight :sweat_smile: I'll check it out ASAP!

mahsashadi commented 2 years ago

As an example, below object is options in server-side vfs adapter, sending from file-manager. So by this encoding, the options you set here, are accessible by options.options

{
  path: 'home:/New directory',
  options: { showHiddenFiles: 'false' },
  session: {
    // the session
  }
}

Maybe the result should be somthing like this!

{
  path: 'home:/New directory',
  showHiddenFiles: 'false' ,
  session: {
    // the session
  }
}
andersevenrud commented 2 years ago

Sorry, but I don't understand what you mean by the last comment.

mahsashadi commented 2 years ago

I just want to ask you if I do the encoding of options in GET url in a correct way.

No problem, of course you will check its correctness when you test the PRs.

andersevenrud commented 2 years ago

No problem, of course you will check its correctness when you test the PRs.

I will add unit tests for everything so that we can confirm that it works in all scenarios :) If you know how to do that, feel free to add them yourself.

mahsashadi commented 2 years ago

Great, Thanks a lot. I hope you could find time to check these PRs out.

andersevenrud commented 2 years ago

I have left reviews in all PRs.