ulsdevteam/ccvgd-backend

Contemporary Chinese Village Gazetteer Data backend application

ccvgd-backend's Issues

Duplicate logic code

In reviewing the code, @wopsononock and I noted that the logic is essentially identical between:

en_search/advancesearch

ccvgd-backend/app_func/en_search.py

Lines 693 to 836 in 3c15ca1

	def advanceSearch():
	data = request.get_data()
	json_data = json.loads(data.decode("utf-8"))
	villageid = json_data.get("villageid")
	topic = json_data.get("topic")
	year = json_data.get("year")
	year_range = json_data.get("year_range")
	# Get connection
	mydb = mysql.connector.connect(
	host=mysql_host,
	user=mysql_username,
	password=mysql_password,
	port=mysql_port,
	database=mysql_database)
	mycursor = mydb.cursor()
	topics = ["village", "gazetteerinformation", "naturaldisasters", "naturalenvironment", "military", "education",
	"economy", "familyplanning",
	"population", "ethnicgroups", "fourthlastNames", "firstavailabilityorpurchase"]
	# get every topics' index in the
	indexes = []
	for i in topic:
	if i not in topics:
	return "topics is not fullfil requirement"
	else:
	indexes.append(topics.index(i) + 1)
	dicts = {}
	table = []
	table1 = {}
	table1["field"] = ["gazetteerId", "gazetteerName", "villageId", "villageName", "province", "city", "county",
	"category1",
	"data", "unit"]
	table1["data"] = []
	table1["tableNameChinese"] = "村庄基本信息"
	dicts[1] = table1
	table2 = {}
	table2["field"] = ["villageId", "villageName", "gazetteerId", "gazetteerName", "publishYear", "publishType"]
	table2["data"] = []
	table2["tableNameChinese"] = "村志基本信息"
	dicts[2] = table2
	table3 = {}
	table3["field"] = ["gazetteerName", "gazetteerId", "year", "category1"],
	table3["data"] = []
	table3["tableNameChinese"] = "自然灾害"
	dicts[3] = table3
	table4 = {}
	table4["field"] = ["gazetteerName", "gazetteerId", "category1", "data", "unit"]
	table4["data"] = []
	table4["tableNameChinese"] = "自然环境"
	dicts[4] = table4
	table5 = {}
	table5["field"] = ["gazetteerName", "gazetteerId", "category1", "category2", "startYear", "endYear", "data", "unit"]
	table5["data"] = []
	table5["tableNameChinese"] = "军事政治"
	dicts[5] = table5
	table6 = {}
	table6["field"] = ["gazetteerName", "gazetteerId", "category1", "category2", "startYear", "endYear",
	"data",
	"unit"]
	table6["data"] = []
	table6["tableNameChinese"] = "教育"
	dicts[6] = table6
	table7 = {}
	table7["field"] = ["gazetteerName", "gazetteerId", "category1", "category2", "category3", "startYear", "endYear",
	"data",
	"unit"]
	table7["data"] = []
	table7["tableNameChinese"] = "经济"
	dicts[7] = table7
	table8 = {}
	table8["field"] = ["gazetteerName", "gazetteerId", "category", "startYear", "endYear", "data", "unit"]
	table8["data"] = []
	table8["tableNameChinese"] = "计划生育"
	dicts[8] = table8
	table9 = {}
	table9["field"] = ['gazetteerName', 'gazetteerId', 'category1', 'category2', 'startYear', 'endYear', 'data', 'unit']
	table9["data"] = []
	table9["tableNameChinese"] = "人口"
	dicts[9] = table9
	table10 = {}
	table10["field"] = ["gazetteerName", "gazetteerId", "category1", "startYear", "endYear", "data", "unit"]
	table10["data"] = []
	table10["tableNameChinese"] = "民族"
	dicts[10] = table10
	table11 = {}
	table11["field"] = ["gazetteerName", "gazetteerId", "firstLastNameId", "secondLastNameId", "thirdLastNameId",
	"fourthLastNameId",
	"fifthLastNameId", "totalNumberOfLastNameInVillage"]
	table11["data"] = []
	table11["tableNameChinese"] = "姓氏"
	dicts[11] = table11
	table12 = {}
	table12["field"] = ["gazetteerName", "gazetteerId", "category", "year"]
	table12["data"] = []
	table12["tableNameChinese"] = "第一次拥有或购买年份"
	dicts[12] = table12
	table2func = {}
	table2func[3] = getNaturalDisaster
	table2func[4] = getNaturalEnvironment
	table2func[5] = getMilitary
	table2func[6] = getEduaction
	table2func[7] = getEconomy
	table2func[8] = getFamilyPlanning
	table2func[9] = getPopulation
	table2func[10] = getEthnicgroups
	table2func[11] = getFourthlastName
	table2func[12] = getFirstAvailabilityorPurchase
	# For every node in the village and we want to change
	temp = {}
	for village_id in villageid:
	mycursor.execute(
	"SELECT gazetteerTitleHanyuPinyin_村志书名汉语拼音 FROM gazetteerInformation_村志信息 WHERE gazetteerId_村志代码={}".format(village_id))
	gazetteerName = mycursor.fetchone()[0]
	for i in getVillage(mycursor, village_id, gazetteerName)["data"]:
	table1["data"].append(i)
	for i in getGazetteer(mycursor, village_id, gazetteerName)["data"]:
	table2["data"].append(i)
	for index in indexes:
	newTable = dicts[index] # table3~12
	temp[index] = newTable # {3: table3...}
	if index == 1 or index == 2:
	continue
	else:
	func = table2func[index]
	for j in func(mycursor, village_id, gazetteerName)["data"]:
	newTable["data"].append(j) # table3~12["data"].append(i) =>

and

advancesearch

ccvgd-backend/app_func/advance_search.py

Lines 20 to 187 in 3c15ca1

	def advanceSearch():
	data = request.get_data()
	json_data = json.loads(data.decode("utf-8"))
	villageid = json_data.get("villageid")
	topic = json_data.get("topic")
	year = json_data.get("year", None)
	year_range = json_data.get("year_range", None)
	if villageid == None or topic == None:
	return jsonify({"code":4001,"message":"No village id or topic please try again"})
	# Get connection
	mydb = mysql.connector.connect(
	host=mysql_host,
	user=mysql_username,
	password=mysql_password,
	port=mysql_port,
	database=mysql_database)
	mycursor = mydb.cursor()
	topics = ["village", "gazetteerinformation", "naturaldisasters", "naturalenvironment", "military", "education",
	"economy", "familyplanning",
	"population", "ethnicgroups", "fourthlastNames", "firstavailabilityorpurchase"]
	# get every topics' index in the
	indexes = []
	for i in topic:
	if i not in topics:
	return "topics is not fullfil requirement"
	else:
	indexes.append(topics.index(i) + 1)
	dicts = {}
	table = []
	table1 = {}
	table1["field"] = ["gazetteerId", "gazetteerName", "villageId", "villageName", "province", "city", "county",
	"category1",
	"data", "unit"]
	table1["data"] = []
	table1["tableNameChinese"] = "村庄基本信息"
	dicts[1] = table1
	# 村志信息
	table2 = {}
	table2["field"] = ["villageId", "villageName", "gazetteerId", "gazetteerName", "publishYear", "publishType"]
	table2["data"] = []
	table2["tableNameChinese"] = "村志基本信息"
	dicts[2] = table2
	# 自然灾害
	table3 = {}
	table3["field"] = ["gazetteerName", "gazetteerId", "year", "category1"],
	table3["data"] = []
	table3["year"] = []
	table3["tableNameChinese"] = "自然灾害"
	dicts[3] = table3
	table4 = {}
	table4["field"] = ["gazetteerName", "gazetteerId", "category1", "data", "unit"]
	table4["data"] = []
	table4["tableNameChinese"] = "自然环境"
	dicts[4] = table4
	# 军事
	table5 = {}
	table5["field"] = ["gazetteerName", "gazetteerId", "category1", "category2", "startYear", "endYear", "data", "unit"]
	table5["data"] = []
	table5["year"] = []
	table5["tableNameChinese"] = "军事政治"
	dicts[5] = table5
	# 教育
	table6 = {}
	table6["field"] = ["gazetteerName", "gazetteerId", "category1", "category2", "startYear", "endYear",
	"data",
	"unit"]
	table6["data"] = []
	table6["tableNameChinese"] = "教育"
	table6["year"] = []
	dicts[6] = table6
	# 经济
	table7 = {}
	table7["field"] = ["gazetteerName", "gazetteerId", "category1", "category2", "category3", "startYear", "endYear",
	"data",
	"unit"]
	table7["data"] = []
	table7["year"] = []
	table7["tableNameChinese"] = "经济"
	dicts[7] = table7
	# 计划生育
	table8 = {}
	table8["field"] = ["gazetteerName", "gazetteerId", "category", "startYear", "endYear", "data", "unit"]
	table8["data"] = []
	table8["year"] = []
	table8["tableNameChinese"] = "计划生育"
	dicts[8] = table8
	# 人口
	table9 = {}
	table9["field"] = ['gazetteerName', 'gazetteerId', 'category1', 'category2', 'startYear', 'endYear', 'data', 'unit']
	table9["data"] = []
	table9["year"] = []
	table9["tableNameChinese"] = "人口"
	dicts[9] = table9
	# 民族
	table10 = {}
	table10["field"] = ["gazetteerName", "gazetteerId", "category1", "startYear", "endYear", "data", "unit"]
	table10["data"] = []
	table10["year"] = []
	table10["tableNameChinese"] = "民族"
	dicts[10] = table10
	table11 = {}
	table11["field"] = ["gazetteerName", "gazetteerId", "firstLastNameId", "secondLastNameId", "thirdLastNameId",
	"fourthLastNameId",
	"fifthLastNameId", "totalNumberOfLastNameInVillage"]
	table11["data"] = []
	table11["tableNameChinese"] = "姓氏"
	dicts[11] = table11
	# 第一次购买年份
	table12 = {}
	table12["field"] = ["gazetteerName", "gazetteerId", "category", "year"]
	table12["data"] = []
	table12["year"] = []
	table12["tableNameChinese"] = "第一次拥有或购买年份"
	dicts[12] = table12
	table2func = {}
	table2func[3] = getNaturalDisaster
	table2func[4] = getNaturalEnvironment
	table2func[5] = getMilitary
	table2func[6] = getEduaction
	table2func[7] = getEconomy
	table2func[8] = getFamilyPlanning
	table2func[9] = getPopulation
	table2func[10] = getEthnicgroups
	table2func[11] = getFourthlastName
	table2func[12] = getFirstAvailabilityorPurchase
	# For every node in the village and we want to change
	temp = {}
	for village_id in villageid:
	mycursor.execute(
	"SELECT gazetteerTitle_村志书名 FROM gazetteerInformation_村志信息 WHERE gazetteerId_村志代码={}".format(village_id))
	gazetteerName = mycursor.fetchone()[0]
	for i in getVillage(mycursor, village_id, gazetteerName)["data"]:
	table1["data"].append(i)
	for i in getGazetteer(mycursor, village_id, gazetteerName)["data"]:
	table2["data"].append(i)
	village_year = []
	for index in indexes:
	newTable = dicts[index] # table3~12
	temp[index] = newTable # {3: table3...}
	if index == 1 or index == 2:
	continue
	else:
	func = table2func[index]
	res = func(mycursor, village_id, gazetteerName, year, year_range)
	for j in res["data"]:
	newTable["data"].append(j) # table3~12["data"].append(i) =>

This should be consolidated to a helper function which is called by both endpoints. Any time you are duplicating large sections of code, this should be abstracted into one or more functions to perform the action which can be reusable across the different callers.

Document application configuration requirements

When deploying the application, the administrator will need to modify config.py, especially the SQLALCHEMY_DATABASE_URL. Some of the options in config.py are probably meant to be ignored by an administrator, and the value JSON_AS_ASCII appears twice.

The configuration requirements should be documented in the readme file, with explanation of the relevant configuration options.

These configuration options also need to be used in the python files. Currently the database username and password is hardcoded in multiple locations:

ccvgd-backend/app_func/advance_search.py

Lines 31 to 36 in 9e08e87

	mydb = mysql.connector.connect(
	host="localhost",
	user="root",
	password="123456",
	port=3306,
	database="CCVG")

ccvgd-backend/app_func/en_search.py

Lines 28 to 34 in 9e08e87

	mydb = mysql.connector.connect(
	host="localhost",
	user="root",
	password="123456",
	port=3306,
	database="CCVG")
	mycursor = mydb.cursor()

etc.

Instead, the database credentials should only be represented once, and only in the configuration file. The configuration file should represent example credentials, which the administrator will be required to customize.

Document configuration and deployment expectations

The README file should:

• Describe the configuration expectations (editing config.py)
• Describe production deployment expectations (similar to the development server run command)
  • for example, via something like waitress, or Apache.

Consider URL structure for passing query data from frontend

The Multiple Village Download URL is currently structured to concatenate the villages and the topic with underscores as a GET request.

For example:
http://backend.tld/advancesearch/download/1_3_2_economy

Here 1, 2, and 3 are village ids and economy is the topic.

This does not align well with REST principles as it combines different query terms (objects) together.

This might be better represented as:
http://backend.tld/advancesearch/download/1_3_2/economy
or
http://backend.tld/advancesearch/download/1,3,2/economy
or
http://backend.tld/advancesearch/download/?village=1,3,2&term=economy

It might be tempting to pass query data via POST, so that you can hand of a JSON object directly, like:

{
  "village": [1, 2, 3],
  "term": "economy"
}

But a POST request should represent a data-change event, and this limits the ability for effective caching because the query context is removed from the URI.

Break en/search endpoint into different routes for each topic

Currently the en/search endpoint takes the topic as part of the post body. Given that the logic and format of the output are different for each topic, it might make more sense to give each topic its own route, and therefore its own function. The common code currently at the end of the function

ccvgd-backend/app_func/en_search.py

Lines 594 to 644 in 6e82448

	count_valid = 0
	# check filter item
	if "filter" in table.keys():
	for j in list(table["filter"][0].keys()):
	if table["filter"][0][j] == 0:
	del table["filter"][0][j]
	continue
	# check filter with 2 category item
	if "filter_2" in table.keys():
	for item in list(table["filter_2"][0].keys()):
	if table["filter_2"][0][item] == 0:
	del table["filter_2"][0][item]
	elif type(table["filter_2"][0][item]) != int:
	for less_item in list(table["filter_2"][0][item]):
	if table["filter_2"][0][item][less_item] == 0:
	del table["filter_2"][0][item][less_item]
	continue
	for item in list(table["filter_2"][0].keys()):
	if table["filter_2"][0][item] == {}:
	del table["filter_2"][0][item]
	continue
	# check data
	for i in table["data"]:
	check_value = i.values()
	if check_value == None:
	count_valid += 1
	if count_valid == len(table["data"]):
	table["isEmptyTable"] = "True"
	return "This table is empty!"
	else:
	table["isEmptyTable"] = "False"
	# write into csv
	f = open('/home/yuelv/CCVG/app_func/single_csv/{}_{}'.format(village_id, topic), 'w', encoding='utf-8')
	# f = open('/home/yuelv/CCVG/app_func/single_csv/{}_{}.csv'.format(village_id, topic), 'w', encoding='utf-8')
	csv_writer = csv.writer(f)
	title = [i for i in table["field"]]
	if len(title) == 1:
	title = title[0]
	csv_writer.writerow(title)
	for item in table["data"]:
	temp_l = []
	for ti in title:
	temp_l.append(item[ti])
	csv_writer.writerow(temp_l)
	return jsonify(table)

could be moved into its own function to be called by each route.

Document python module requirements

Please document in the README the necessary python modules.

When initially run:

$ python app.py runserver -p 8081
Traceback (most recent call last):
  File "app.py", line 2, in <module>
    from manager import create_app
  File "/opt/local/ccvgd/backend/manager.py", line 1, in <module>
    from flask import Flask
ModuleNotFoundError: No module named 'flask'

Flask was identified as a dependency.

After installing Flask:

$ pip install Flask

Re-running the command continues to identify dependencies:

$ python app.py runserver -p 8081
Traceback (most recent call last):
  File "app.py", line 2, in <module>
    from manager import create_app
  File "/opt/local/ccvgd/backend/manager.py", line 6, in <module>
    from flask_cors import CORS

Consider whether .pyc files should be .gitignored

Similar to ulsdevteam/ccvgd-frontend#6, the compiled python bytecode represents more of a distribution or packaged form of the source code, and is probably not appropriate for the git repo.

Consider this default .gitignore file for python:
https://github.com/github/gitignore/blob/master/Python.gitignore

Apply a LICENSE to the repo

Code published online should describe in a license what the conditions are for its reuse and modification.

The license may be dictated by any dependencies which you use or package with the application.

If you are not constrained by existing dependencies, GPL and MIT licenses are good FLOSS options.

Duplicate code

The code in https://github.com/ulsdevteam/ccvgd-backend/tree/d637815432f32695484077d65830252b38dc72dd is a duplicate of https://github.com/ulsdevteam/ccvgd-database/tree/master/pythonScript.

As changes are added to https://github.com/ulsdevteam/ccvgd-database/tree/master/pythonScript, this local copy will become increasingly out of date. See, for example:
https://github.com/ulsdevteam/ccvgd-database/tree/master/pythonScript

Instead of copying the code from the other repository, please include instructions which link to the other repository, or include the other repository as a git submodule.