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

Add option interface for VFS readdir #163

Open
mahsashadi opened this issue Aug 24, 2021 · 13 comments
Open

Add option interface for VFS readdir #163

mahsashadi opened this issue Aug 24, 2021 · 13 comments
Labels
enhancement New feature or request

Comments

@mahsashadi
Copy link
Contributor

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 andersevenrud changed the title Gradual fetching of data by scrolling down in File Manger Application Add option interface for VFS readdir Aug 24, 2021
@andersevenrud andersevenrud added the enhancement New feature or request label Aug 24, 2021
@andersevenrud
Copy link
Member

NB: I changed the title of this issue

@mahsashadi
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

mahsashadi commented Aug 28, 2021

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
Copy link
Member

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

@mahsashadi
Copy link
Contributor Author

@andersevenrud
Copy link
Member

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

@mahsashadi
Copy link
Contributor Author

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
Copy link
Member

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

@mahsashadi
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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

@andersevenrud
Copy link
Member

I have left reviews in all PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants