Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions uk_bin_collection/tests/input.json
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@
"url": "https://environmentfirst.co.uk/house.php?uprn=100060055444",
"wiki_command_url_override": "https://environmentfirst.co.uk/house.php?uprn=XXXXXXXXXX",
"wiki_name": "Environment First",
"wiki_note": "For properties with collections managed by Environment First, such as Lewes and Eastbourne. Replace the XXXXXXXXXX with the UPRN of your property\u2014you can use [FindMyAddress](https://www.findmyaddress.co.uk/search) to find this."
"wiki_note": "For properties with collections managed by Environment First, such as Lewes and Eastbourne. Replace the XXXXXXXXXX with the UPRN of your property—you can use [FindMyAddress](https://www.findmyaddress.co.uk/search) to find this."
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
},
"EppingForestDistrictCouncil": {
"postcode": "IG9 6EP",
Expand Down Expand Up @@ -936,8 +936,9 @@
"uprn": "200002981143",
"url": "https://www.fenland.gov.uk/article/13114/",
"wiki_name": "Fenland",
"wiki_note": "Pass the UPRN. You can find it using [FindMyAddress](https://www.findmyaddress.co.uk/search).",
"LAD24CD": "E07000010"
"wiki_note": "Pass the UPRN. You can find it using [FindMyAddress](https://www.findmyaddress.co.uk/search). This parser requires a Selenium webdriver.",
"LAD24CD": "E07000010",
"web_driver": "http://selenium:4444"
},
"FermanaghOmaghDistrictCouncil": {
"house_number": "20",
Expand Down Expand Up @@ -1754,14 +1755,14 @@
"LAD24CD": "E06000012"
},
"NorthHertfordshireDistrictCouncil": {
"house_number": "Stewards Flat",
"postcode": "SG5 1PZ",
"skip_get_url": true,
"url": "https://waste.nc.north-herts.gov.uk/w/webpage/find-bin-collection-day-input-address",
"web_driver": "http://selenium:4444",
"wiki_name": "North Hertfordshire",
"wiki_note": "Pass a postcode (with space) and house_number/name. The scraper performs the Liberty Create typeahead lookup and matches house_number as a case-insensitive substring.",
"LAD24CD": "E07000099"
"house_number": "Stewards Flat",
"postcode": "SG5 1PZ",
"skip_get_url": true,
"url": "https://waste.nc.north-herts.gov.uk/w/webpage/find-bin-collection-day-input-address",
"web_driver": "http://selenium:4444",
"wiki_name": "North Hertfordshire",
"wiki_note": "Pass a postcode (with space) and house_number/name. The scraper performs the Liberty Create typeahead lookup and matches house_number as a case-insensitive substring.",
"LAD24CD": "E07000099"
},
"NorthKestevenDistrictCouncil": {
"skip_get_url": true,
Expand Down
112 changes: 66 additions & 46 deletions uk_bin_collection/uk_bin_collection/councils/FenlandDistrictCouncil.py
Original file line number Diff line number Diff line change
@@ -1,65 +1,85 @@
import json

import requests
from selenium.webdriver.common.by import By
from selenium.webdriver.support import expected_conditions as EC
from selenium.webdriver.support.ui import WebDriverWait

from uk_bin_collection.uk_bin_collection.common import *
from uk_bin_collection.uk_bin_collection.get_bin_data import AbstractGetBinDataClass


# import the wonderful Beautiful Soup and the URL grabber
class CouncilClass(AbstractGetBinDataClass):
"""
Concrete classes have to implement all abstract operations of the
base class. They can also override some operations with a default
implementation.
Fenland's GIS layer endpoint is behind Cloudflare JS challenge.
Load the page in Selenium to pass the challenge, then fetch the
JSON API from within the browser context.
"""

def parse_data(self, page: str, **kwargs) -> dict:
user_uprn = kwargs.get("uprn")
check_uprn(user_uprn)

headers = {
"Accept": "application/json, text/javascript, */*; q=0.01",
"Accept-Language": "en-GB,en;q=0.7",
"Connection": "keep-alive",
"Content-Type": "application/json; charset=utf-8",
"Referer": "https://www.fenland.gov.uk/article/13114/?uprn=200002981143&lat=52.665569590474&lng=0.177905443639&postcode=PE13+3SL&line1=20+Felsted+Avenue&rad=5m&layers=2%2C3%2C1",
"Sec-Fetch-Dest": "empty",
"Sec-Fetch-Mode": "cors",
"Sec-Fetch-Site": "same-origin",
"Sec-GPC": "1",
"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/134.0.0.0 Safari/537.36",
"X-Requested-With": "XMLHttpRequest",
}
headless = kwargs.get("headless")
web_driver = kwargs.get("web_driver")
user_agent = (
"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 "
"(KHTML, like Gecko) Chrome/134.0.0.0 Safari/537.36"
)
driver = create_webdriver(web_driver, headless, user_agent, __name__)

try:
driver.execute_cdp_cmd(
"Page.addScriptToEvaluateOnNewDocument",
{
"source": "Object.defineProperty(navigator, 'webdriver', {get: () => undefined})"
},
Comment on lines +28 to +35

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python - <<'PY'
from pathlib import Path

for path in Path("uk_bin_collection").rglob("common.py"):
    text = path.read_text()
    marker = "def create_webdriver("
    start = text.find(marker)
    if start == -1:
        continue
    rest = text[start:]
    next_def = rest.find("\ndef ", 1)
    snippet = rest if next_def == -1 else rest[:next_def]
    print(f"== {path} ==")
    print(snippet)
    print()
PY

rg -n -C2 'execute_cdp_cmd|Page.addScriptToEvaluateOnNewDocument|web_driver|Chrom(e|ium)' \
  uk_bin_collection/uk_bin_collection/councils/FenlandDistrictCouncil.py \
  uk_bin_collection/tests/input.json

Repository: robbrad/UKBinCollectionData

Length of output: 50383


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the FenlandDistrictCouncil.py content around the cited areas
python - <<'PY'
import itertools
from pathlib import Path

path = Path("uk_bin_collection/uk_bin_collection/councils/FenlandDistrictCouncil.py")
lines = path.read_text().splitlines()
for start,end in [(1,120),(120,220)]:
    print(f"\n===== {path} lines {start}-{end} =====")
    for i in range(start-1, min(end, len(lines))):
        print(f"{i+1:4d}: {lines[i]}")
PY

# Extract the Fenland entry from tests/input.json
python - <<'PY'
import json
from pathlib import Path

p = Path("uk_bin_collection/tests/input.json")
data = json.loads(p.read_text())
# Find key that matches Fenland (case-sensitive key lookup)
for k in list(data.keys()):
    if "Fenland" in k:
        print(f"\n=== input.json entry: {k} ===")
        print(json.dumps(data[k], indent=2)[:4000])
        break
else:
    print("No key containing 'Fenland' found in input.json top-level.")
PY

# Search within FenlandDistrictCouncil.py for how JSON is retrieved/parsed
rg -n "execute_async_script|features|upcoming|json|response|status|webdriver|CDP|addScriptToEvaluateOnNewDocument" \
  uk_bin_collection/uk_bin_collection/councils/FenlandDistrictCouncil.py

Repository: robbrad/UKBinCollectionData

Length of output: 4087


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find Fenland entry textually (avoid JSON decoding issues)
rg -n -C3 "Fenland" uk_bin_collection/tests/input.json | head -n 200

# Inspect create_webdriver implementation in common.py (to see if any enforcement/guards exist)
python - <<'PY'
from pathlib import Path
p = Path("uk_bin_collection/uk_bin_collection/common.py")
text = p.read_bytes().decode("utf-8", errors="replace").splitlines()
for i,line in enumerate(text, start=1):
    if "def create_webdriver" in line:
        start=i
        break
else:
    raise SystemExit("create_webdriver not found")
end = min(len(text), start+120)
for j in range(start, end+1):
    print(f"{j:4d}: {text[j-1]}")
PY

# Search for any other guard around execute_cdp_cmd / Chromium in the codebase
rg -n "execute_cdp_cmd\\(|Page\\.addScriptToEvaluateOnNewDocument|CDP|Chrom(e|ium)" uk_bin_collection/uk_bin_collection | head -n 200

Repository: robbrad/UKBinCollectionData

Length of output: 37389


Make the Chromium/CDP requirement explicit and validate the API response shape/status

  • FenlandDistrictCouncil.parse_data calls driver.execute_cdp_cmd(...) and create_webdriver() is Chrome/Chromium-specific (webdriver.ChromeOptions, returns webdriver.Chrome), but the Fenland web_driver is documented/treated as generic Selenium—if the remote node is Firefox-backed, this will fail before the first successful parse. Update FenlandDistrictCouncil’s wiki_note (or fixture/docs) to state Chrome/Chromium-only, or add an early runtime guard (e.g., check driver.capabilities["browserName"]).
  • The in-browser fetch() returns r.text() without checking HTTP status, then the code immediately does json.loads(result)["features"][0]["properties"]["upcoming"] with no payload-shape validation, producing opaque failures for non-200/HTML/empty features. Add explicit status handling and schema validation with clear errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@uk_bin_collection/uk_bin_collection/councils/FenlandDistrictCouncil.py`
around lines 28 - 35, FenlandDistrictCouncil.parse_data currently calls
driver.execute_cdp_cmd (and relies on create_webdriver returning a Chrome
webdriver) without guarding against non-Chromium browsers and assumes the
fetched in-page payload is a JSON with features[0].properties.upcoming; update
the class so either its wiki_note clearly states Chrome/Chromium-only OR add an
early runtime guard in FenlandDistrictCouncil.parse_data that checks
driver.capabilities["browserName"] (or equivalent) and raises a clear error if
not Chrome/Chromium, and modify the fetch result handling to explicitly check
the HTTP status and content-type, parse JSON safely, validate that the top-level
"features" is a non-empty list and that features[0]["properties"]["upcoming"]
exists (raising descriptive errors if any check fails) before accessing the
value.

)

# It needs lat and lng for point data, but we don't need it >:)
params = {
"type": "loadlayer",
"layerId": "2",
"uprn": user_uprn,
"lat": "0.000000000001",
"lng": "0.000000000001",
}
page_url = "https://www.fenland.gov.uk/article/13114/"
driver.get(page_url)

requests.packages.urllib3.disable_warnings()
response = requests.get(
"https://www.fenland.gov.uk/article/13114/", params=params, headers=headers
)
WebDriverWait(driver, 20).until(
lambda d: d.title != "Just a moment..."
)

api_url = (
f"/article/13114/?type=loadlayer&layerId=2"
f"&uprn={user_uprn}&lat=0.000000000001&lng=0.000000000001"
)

result = driver.execute_async_script(
"""
var callback = arguments[arguments.length - 1];
fetch(arguments[0], {
headers: {
'X-Requested-With': 'XMLHttpRequest',
'Accept': 'application/json'
}
})
.then(r => r.text())
.then(t => callback(t))
.catch(e => callback('ERROR: ' + e));
""",
api_url,
)

if result.startswith("ERROR:"):
raise ValueError(f"API fetch failed: {result}")

# Returned data is just json, so we can get what we need
json_data = json.loads(response.text)["features"][0]["properties"]["upcoming"]
data = {"bins": []}
json_data = json.loads(result)["features"][0]["properties"]["upcoming"]
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
data = {"bins": []}

for item in json_data:
collections_list = item["collections"]
for bin in collections_list:
bin_type = bin["desc"]
bin_date = datetime.strptime(
bin["collectionDate"], "%Y-%m-%dT%H:%M:%SZ"
).strftime(date_format)
dict_data = {
"type": bin_type,
"collectionDate": bin_date,
}
data["bins"].append(dict_data)
for item in json_data:
for bin_info in item["collections"]:
data["bins"].append(
{
"type": bin_info["desc"],
"collectionDate": datetime.strptime(
bin_info["collectionDate"], "%Y-%m-%dT%H:%M:%SZ"
).strftime(date_format),
}
)

return data
return data
finally:
driver.quit()
Loading