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

Rename the built-in BatchFileAdapter class back to FileAdapter #235

Closed
hsluoyz opened this issue Feb 6, 2021 · 5 comments · Fixed by #236
Closed

Rename the built-in BatchFileAdapter class back to FileAdapter #235

hsluoyz opened this issue Feb 6, 2021 · 5 comments · Fixed by #236
Assignees
Labels
bug Something isn't working

Comments

@hsluoyz
Copy link
Member

hsluoyz commented Feb 6, 2021

See my comments here: #234 (comment)

I think first we need correctify the BatchFileAdapter code: rename BatchFileAdapter back to FileAdapter. From now on, FileAdapter will always be called FileAdapter, no matter how many interfaces it has implemented.

export class BatchFileAdapter extends FileAdapter implements BatchAdapter {
/**
* FileAdapter is the constructor for FileAdapter.
* @param {string} filePath filePath the path of the policy file.
*/
constructor(filePath: string) {
super(filePath);
}
// addPolicies adds policy rules to the storage.
// This is part of the Auto-Save feature.
public async addPolicies(sec: string, ptype: string, rules: string[][]): Promise<void> {
throw new Error('not implemented');
}
// removePolicies removes policy rules from the storage.
// This is part of the Auto-Save feature.
public async removePolicies(sec: string, ptype: string, rules: string[][]): Promise<void> {
throw new Error('not implemented');
}
}

@hsluoyz hsluoyz self-assigned this Feb 6, 2021
@hsluoyz hsluoyz added the bug Something isn't working label Feb 6, 2021
@Zxilly
Copy link
Contributor

Zxilly commented Feb 6, 2021

So we should move the batch functions and update function to FileAdapter?

@hsluoyz
Copy link
Member Author

hsluoyz commented Feb 6, 2021

@Zxilly see Go code.

@nodece
Copy link
Member

nodece commented Feb 6, 2021

@hsluoyz @Zxilly I don't recommend that rename the BatchFileAdapter to FileAdapter, some users may be using this, so we can copy BatchFileAdapter code to FileAdapter, and add @deprecated to BatchFileAdapter.

@hsluoyz
Copy link
Member Author

hsluoyz commented Feb 6, 2021

@nodece, @Zxilly, good idea, and it can be better:

We keep BatchFileAdapter but it's only an alias to FileAdapter, maybe by inheriting FileAdapter without any new functions. So we don't need to copy code. And we will add @deprecated to BatchFileAdapter. So we will not break backward compatibility.

@Zxilly
Copy link
Contributor

Zxilly commented Feb 6, 2021

@nodece @hsluoyz have implemented, plz review
915eff3

@hsluoyz hsluoyz removed this from Node-Casbin Easy Tasks in Casbin Easy Tasks for Beginners/Student Applicants Feb 6, 2021
hsluoyz pushed a commit that referenced this issue Feb 6, 2021
* feat: add updatePolicy()

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* feat: add unittest for updatePolicy()

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* feat: add updateForUpdatePolicy()

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* feat: add updatePolicy()

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* refactor: rename interface

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* docs: fix comment

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* fix: fix adapter error

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* feat: add updatableFileAdapter

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* fix: remove incorrect claim and fix comment

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* fix: fix comment

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* perf: refactor updatePolicy

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* fix: fix index

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* style: prevent conflict

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* fix: remove UpdatableFileAdapter

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>

* fix: fix unittest()
In fact,this makes unittest cannot pass.The fix will refer to issue #235

Signed-off-by: Zxilly <zhouxinyu1001@gmail.com>
github-actions bot pushed a commit that referenced this issue Feb 6, 2021
# [5.4.0](v5.3.1...v5.4.0) (2021-02-06)

### Features

* add updatePolicy() ([#234](#234)) ([a3218f1](a3218f1)), closes [#235](#235)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants