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

fix: use CSV parsing instead of string splitting for built-in file adapter #171

Merged
merged 1 commit into from Apr 19, 2021

Conversation

shy1st
Copy link
Contributor

@shy1st shy1st commented Apr 4, 2021

Fix: #158

@shy1st
Copy link
Contributor Author

shy1st commented Apr 4, 2021

I added the function of reading the CSV file in the constructor of fileaAdapter and processed it.

pom.xml Outdated Show resolved Hide resolved
@hsluoyz
Copy link
Contributor

hsluoyz commented Apr 4, 2021

@shy1st plz sign off:

image

@shy1st shy1st marked this pull request as draft April 4, 2021 15:00
@shy1st shy1st changed the title fix: Add the csv file support. fix: Modify the reading mode of CSV file. Apr 5, 2021
@shy1st shy1st marked this pull request as ready for review April 5, 2021 04:23
@shy1st
Copy link
Contributor Author

shy1st commented Apr 5, 2021

@shy1st plz sign off:

image

I have fixed it.

@shy1st
Copy link
Contributor Author

shy1st commented Apr 5, 2021

Owing to the fact that I modified the loadPolicyLine function in the Helper.Maybe other projects need to be modified as well, such as jdbcAdapter.What do you think of this problem? @hsluoyz

jcasbin.iml Outdated Show resolved Hide resolved
@shy1st shy1st requested a review from hsluoyz April 5, 2021 12:54
@shy1st
Copy link
Contributor Author

shy1st commented Apr 7, 2021

@hsluoyz

@hsluoyz
Copy link
Contributor

hsluoyz commented Apr 8, 2021

@shy1st I don't know why we are changing the interface. We will NOT change the the interface. Plz only modify the file adapter class code. If anything doesn't fit your need, just implement your own utility functions inside the class.

@shy1st shy1st force-pushed the master branch 3 times, most recently from eac327e to 37ae1b0 Compare April 13, 2021 01:23
@@ -14,6 +14,7 @@

package org.casbin.jcasbin.main;

import org.apache.commons.csv.CSVFormat;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

src/test/java/org/casbin/jcasbin/main/ModelUnitTest.java Outdated Show resolved Hide resolved
@@ -14,13 +14,14 @@

package org.casbin.jcasbin.main;

import org.apache.commons.csv.CSVFormat;
import org.apache.commons.csv.CSVParser;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

@shy1st shy1st force-pushed the master branch 2 times, most recently from dbd35d3 to 2dc18bd Compare April 19, 2021 07:26
@shy1st shy1st changed the title fix: Modify the reading mode of CSV file. fix: Modify string segmentation method. Apr 19, 2021
@hsluoyz hsluoyz changed the title fix: Modify string segmentation method. fix: use CSV parsing instead of string splitting for built-in file adapter Apr 19, 2021
@hsluoyz hsluoyz merged commit 108dcb0 into casbin:master Apr 19, 2021
@hsluoyz
Copy link
Contributor

hsluoyz commented Apr 19, 2021

🎉 This PR is included in version 1.8.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Question: Is there a way to escape comma in policy rule?
2 participants