Refactor - Migrate the TaxJar rate-table seam onto the Address value object.

`create_or_update_tax_rate()` and `allow_street_address_for_matched_rates()`
now derive their `find_rates()` arguments and their `location_code` rows from
one `Address`, through a matched pair of projections the value object owns —
`to_find_rates_args()` and `to_rate_table_locations()`. Deriving them
separately is what let this seam look up values it had never written and
insert a fresh `wp_woocommerce_tax_rates` row for the same address on every
calculation.

Three defects the characterization tests pinned are fixed:

- A state carrying anything outside `[a-z0-9_-]` duplicated a rate row per
  calculation. `WC_Tax::prepare_tax_rate()` singles `tax_rate_state` out for
  `sanitize_key()` before storing it, and the lookup only stripped spaces —
  an approximation that happened to be right for `'N Y'` and wrong for
  `'N.Y.'` and for anything non-ASCII. `Address::state_compact()` now mirrors
  the core function instead of approximating it, so the lookup asks for the
  value core actually stored.
- A city carrying anything `sanitize_text_field()` removes did the same: the
  write applied `wc_clean()` and the lookup did not. Sanitisation now happens
  once, inside the shared projection.
- `allow_street_address_for_matched_rates()` normalised the city but not the
  state, so it could not see rows `create_or_update_tax_rate()` had just
  written. Rate rows existed and the `woocommerce_matched_rates` path — price
  display, shipping tax, coupons — returned nothing for those addresses,
  while itemized cart totals stayed correct.

Two of the plan's three target asymmetries turned out not to be live, and the
tests say so rather than the code pretending to fix them: `find_rates()`
applies the same `wc_normalize_postcode( wc_clean( … ) )` to its argument that
the write applies, so ZIP+4 always round-tripped; and core upper-cases the
city on both ends, so the plugin's asymmetric `strtoupper()` was unobservable.

Behaviour deltas beyond the fixes above:

- A comma-list postcode is now stored as its first segment rather than whole.
  The row is keyed by the segment the rate was actually quoted for — the
  TaxJar request body has carried only that segment since the request seam
  moved onto the value object.
- `get_backend_address()` no longer upper-cases postcode, city and street.
  That was this path's own historic behaviour, unmatched by `get_address()`,
  and kept only because the city reaches the rate-table city column — which
  is now upper-cased at both the write and the lookup. The admin recalculate
  and the cart now put identically-cased values on the wire. This changes the
  request JSON, so one generation of the `md5( $json )` request cache is
  invalidated.
- `tax_rate_country` is trimmed before storage, so a whitespace-padded country
  can no longer produce a row nothing matches.
- `create_or_update_tax_rate()` tolerates a missing or non-scalar location
  field instead of emitting a notice or storing the literal "Array". It is
  public and takes the location it is handed.

`WC_Connect_TaxJar_Integration::normalize_city()` now raises its deprecation
notice. It could not before: the last three in-repo callers were all on this
seam, and the notice would have fired on every checkout. The method itself
stays — it is `protected static`, so an out-of-repo subclass may call it.

Author: bartech

classes/class-wc-connect-taxjar-integration.php

@@ -691,27 +691,32 @@ protected function get_address( $location_type = null ) {
 	/**
 	 * Allow street address to be passed when finding rates
 	 *
-	 * @param array  $matched_tax_rates
-	 * @param string $tax_class
+	 * Despite the name, no street ever reaches `WC_Tax::find_rates()` — it takes no
+	 * street argument, and the value this method used to unpack from the tuple was
+	 * never read. What the callback actually does is accept a location of *four or
+	 * more* elements where `WC_Tax::get_rates_from_location()` accepts exactly four,
+	 * so it supplies the rates core skips when this plugin's five-element taxable
+	 * address is in play. It is public and hooked on `woocommerce_matched_rates`,
+	 * which core fires from the price-display, shipping-tax and coupon paths.
+	 *
+	 * The lookup arguments now come from the same value object `create_or_update_tax_rate()`
+	 * writes with. Before, this method normalised the city but not the state, so it
+	 * could not see rows that method had just written: the rate rows existed and
+	 * these paths returned nothing for them.
+	 *
+	 * @param array  $matched_tax_rates Rates core matched; replaced wholesale.
+	 * @param string $tax_class         Tax class slug.
 	 * @return array
 	 */
 	public function allow_street_address_for_matched_rates( $matched_tax_rates, $tax_class = '' ) {
-		$tax_class         = sanitize_title( $tax_class );
-		$location          = WC_Tax::get_tax_location( $tax_class );
-		$matched_tax_rates = array();
-		if ( sizeof( $location ) >= 4 ) {
-			list( $country, $state, $postcode, $city, $street ) = array_pad( $location, 5, '' );
-			$matched_tax_rates                                  = WC_Tax::find_rates(
-				array(
-					'country'   => $country,
-					'state'     => $state,
-					'postcode'  => $postcode,
-					'city'      => strtoupper( self::normalize_city( $city ) ),
-					'tax_class' => $tax_class,
-				)
-			);
+		$tax_class = sanitize_title( $tax_class );
+		$location  = WC_Tax::get_tax_location( $tax_class );
+
+		if ( ! is_array( $location ) || count( $location ) < 4 ) {
+			return array();
 		}
-		return $matched_tax_rates;
+
+		return WC_Tax::find_rates( Address::from_taxable_tuple( $location )->to_find_rates_args( $tax_class ) );
 	}
 
 	public function cleanup_tax_label( $rate_name ) {
@@ -917,27 +922,17 @@ public function get_taxable_address( $location_type = null ) {
 	 * Security: WooCommerce has already verified the nonce and capability by the time
 	 * `woocommerce_before_save_order_items` fires.
 	 *
+	 * This method used to `strtoupper()` postcode, city and street on top of that,
+	 * which `get_address()` did not — the same order recalculated from the admin and
+	 * from the cart put differently-cased values on the wire. The casing was kept only
+	 * because the city feeds the `wp_woocommerce_tax_rates` city column; now that the
+	 * rate-table seam upper-cases the city at both the write and the lookup, nothing
+	 * downstream depends on it and the two paths agree.
+	 *
 	 * @return array
 	 */
 	protected function get_backend_address() {
-		$address = Address::from_post_request()->to_legacy_options();
-
-		/*
-		 * Historic behaviour of this method, preserved deliberately: the admin path
-		 * upper-cases every field, not only country and state (which `Address` already
-		 * normalises). It is load-bearing for the city, which reaches the
-		 * `wp_woocommerce_tax_rates` city column, and pinned by
-		 * `test_get_backend_address_normalizes_semicolon_city()`. Reconciling the casing
-		 * asymmetry between this path and `get_address()` belongs with the rate-table
-		 * seam, where both ends of the write/read round trip can move together.
-		 */
-		foreach ( array( 'to_zip', 'to_city', 'to_street' ) as $key ) {
-			if ( is_string( $address[ $key ] ) ) {
-				$address[ $key ] = strtoupper( $address[ $key ] );
-			}
-		}
-
-		return $address;
+		return Address::from_post_request()->to_legacy_options();
 	}
 
 	/**
@@ -1791,9 +1786,11 @@ private function get_itemized_tax_rates( $taxes, $taxjar_taxes, $options ): arra
 	 * normalisation policy has one home rather than one copy per consumer. This
 	 * method is retained as a delegate — it is `protected static`, so a subclass
 	 * outside this repository may be calling it, and removing it would break that
-	 * subclass on load. The remaining in-repo call sites move to the value object
-	 * when their seams are migrated; no `_deprecated_function()` notice is raised
-	 * because those call sites are still live and would fire it on every checkout.
+	 * subclass on load.
+	 *
+	 * The `_deprecated_function()` notice can be raised as of this release because
+	 * the last in-repo caller has moved to the value object. Until then the notice
+	 * would have fired on every checkout.
 	 *
 	 * The non-string guard is preserved: this method has always returned its
 	 * argument untouched when handed a non-string, and callers may rely on that.
@@ -1802,6 +1799,8 @@ private function get_itemized_tax_rates( $taxes, $taxjar_taxes, $options ): arra
 	 * @return string Normalized city, safe for `_update_tax_rate_cities` and `find_rates`.
 	 */
 	protected static function normalize_city( $city ) {
+		wc_deprecated_function( __METHOD__, '3.6.11', '\Automattic\WCServices\Tax\Address::normalize_city()' );
+
 		if ( ! is_string( $city ) ) {
 			return $city;
 		}
@@ -1822,11 +1821,34 @@ protected static function normalize_city( $city ) {
 	 * @return int
 	 */
 	public function create_or_update_tax_rate( $location, $rate, $tax_class = '', $freight_taxable = 1, $rate_priority = 1, $tax_rate_name = 'Tax' ) {
-		// Prevent filling "State code" column for countries with VAT tax.
-		// VAT tax is country wide.
-		$to_state      = 'VAT' === $tax_rate_name ? '' : strtoupper( $location['to_state'] );
 		$rate_priority = absint( $rate_priority );
 
+		/*
+		 * One address, two consumers: the `find_rates()` lookup below and the location
+		 * rows written beside the rate. Deriving them separately is what let this
+		 * method insert a fresh row on every calculation for some addresses — it
+		 * looked up values it had never stored. The value object now owns both
+		 * projections, so they cannot drift apart.
+		 *
+		 * Non-scalars collapse to an empty string rather than reaching `(string)` and
+		 * becoming the literal "Array". This method is public and takes the location
+		 * it is handed.
+		 */
+		$field = static function ( $value ) {
+			return is_scalar( $value ) ? (string) $value : '';
+		};
+
+		$address = Address::from_options(
+			array(
+				'to_country' => $field( $location['to_country'] ?? '' ),
+				// Prevent filling "State code" column for countries with VAT tax.
+				// VAT tax is country wide.
+				'to_state'   => 'VAT' === $tax_rate_name ? '' : $field( $location['to_state'] ?? '' ),
+				'to_zip'     => $field( $location['to_zip'] ?? '' ),
+				'to_city'    => $field( $location['to_city'] ?? '' ),
+			)
+		);
+
 		/**
 		 * @see https://github.com/Automattic/woocommerce-services/issues/2531
 		 * @see https://floridarevenue.com/faq/Pages/FAQDetails.aspx?FAQID=1277&IsDlg=1
@@ -1850,8 +1872,8 @@ public function create_or_update_tax_rate( $location, $rate, $tax_class = '', $f
 		}
 
 		$tax_rate = array(
-			'tax_rate_country'  => $location['to_country'],
-			'tax_rate_state'    => $to_state,
+			'tax_rate_country'  => $address->country(),
+			'tax_rate_state'    => $address->state_compact(),
 			// For the US, we're going to modify the name of the tax rate to simplify the reporting and distinguish between the tax rates at the counties level.
 			// I would love to do this for other locations, but it looks like that would create issues.
 			// For example, for the UK it would continuously rename the rate name with an updated `state` "piece", each time a request is made
@@ -1863,15 +1885,7 @@ public function create_or_update_tax_rate( $location, $rate, $tax_class = '', $f
 			'tax_rate_class'    => $tax_class,
 		);
 
-		$wc_rates = WC_Tax::find_rates(
-			array(
-				'country'   => $location['to_country'],
-				'state'     => str_replace( ' ', '', $to_state ),
-				'postcode'  => $location['to_zip'],
-				'city'      => strtoupper( self::normalize_city( $location['to_city'] ) ),
-				'tax_class' => $tax_class,
-			)
-		);
+		$wc_rates = WC_Tax::find_rates( $address->to_find_rates_args( $tax_class ) );
 
 		$wc_rates_ids = is_array( $wc_rates ) ? array_keys( $wc_rates ) : array();
 		if ( isset( $wc_rates_ids[ $rate_priority - 1 ] ) ) {
@@ -1902,8 +1916,10 @@ public function create_or_update_tax_rate( $location, $rate, $tax_class = '', $f
 			$rate_id = WC_Tax::_insert_tax_rate( $tax_rate );
 			// VAT is always country wide, no need to create separate entires for each zip and city.
 			if ( 'VAT' !== $tax_rate_name ) {
-				WC_Tax::_update_tax_rate_postcodes( $rate_id, wc_normalize_postcode( wc_clean( $location['to_zip'] ) ) );
-				WC_Tax::_update_tax_rate_cities( $rate_id, self::normalize_city( wc_clean( $location['to_city'] ) ) );
+				$locations = $address->to_rate_table_locations();
+
+				WC_Tax::_update_tax_rate_postcodes( $rate_id, $locations['postcode'] );
+				WC_Tax::_update_tax_rate_cities( $rate_id, $locations['city'] );
 			}
 		}
 

src/Tax/Address.php

@@ -344,14 +344,28 @@ public function state(): string {
 	}
 
 	/**
-	 * State with all spaces removed. Used by `WC_Tax::find_rates()` lookup —
-	 * state codes shouldn't contain spaces, and stripping defends against
-	 * accidental whitespace from form input.
+	 * State in the exact form WooCommerce core stores it in `tax_rate_state`.
+	 *
+	 * This is not a normalisation policy of ours — it is a mirror of core's.
+	 * `WC_Tax::prepare_tax_rate()` singles this one column out and runs it through
+	 * `sanitize_key()` before `format_tax_rate_state()` uppercases it, so the value
+	 * that reaches the column has been lower-cased and stripped of everything
+	 * outside `[a-z0-9_-]`. `WC_Tax::get_matched_tax_rates()` then compares against
+	 * it with a plain `tax_rate_state IN ( %s, '' )` and no normalisation of its own.
+	 *
+	 * A lookup that asks for anything else therefore cannot find the row it just
+	 * wrote, and `create_or_update_tax_rate()` responds by inserting another one —
+	 * once per calculation, forever. Stripping spaces (which is all this method used
+	 * to do) covers `'N Y'` and nothing else: `'N.Y.'`, and any state carrying
+	 * non-ASCII, still diverge.
+	 *
+	 * Because core applies the same function on the way in, mirroring it here cannot
+	 * lose information the stored value still has.
 	 *
 	 * @return string
 	 */
 	public function state_compact(): string {
-		return str_replace( ' ', '', $this->state );
+		return strtoupper( sanitize_key( $this->state ) );
 	}
 
 	/**
@@ -574,8 +588,11 @@ public function to_nexus_array(): array {
 	}
 
 	/**
-	 * Args for `WC_Tax::find_rates()`. State is space-compacted, city is uppercase
-	 * (consistent with WC core's `format_tax_rate_city()`), `tax_class` passed through.
+	 * Args for `WC_Tax::find_rates()` — the read end of the tax-rate-table round trip.
+	 *
+	 * Every field is in the form WC core stores it, so this and
+	 * `to_rate_table_locations()` cannot disagree: a row written from one address is
+	 * found by a lookup built from the same address.
 	 *
 	 * @param string $tax_class Tax class slug to look up.
 	 * @return array{country: string, state: string, postcode: string, city: string, tax_class: string}
@@ -584,12 +601,60 @@ public function to_find_rates_args( string $tax_class = '' ): array {
 		return array(
 			'country'   => $this->country,
 			'state'     => $this->state_compact(),
-			'postcode'  => $this->postcode,
-			'city'      => strtoupper( $this->city ),
+			'postcode'  => $this->postcode_as_stored(),
+			'city'      => $this->city_as_stored(),
 			'tax_class' => $tax_class,
 		);
 	}
 
+	/**
+	 * Location codes for the write end: the `postcode` and `city` rows that
+	 * `WC_Tax::_update_tax_rate_postcodes()` / `_update_tax_rate_cities()` persist
+	 * alongside a rate.
+	 *
+	 * Paired with `to_find_rates_args()` on purpose. Deriving the two independently is
+	 * what let `create_or_update_tax_rate()` look up a value it had never written and
+	 * insert a fresh row for the same address on every calculation.
+	 *
+	 * @return array{postcode: string, city: string}
+	 */
+	public function to_rate_table_locations(): array {
+		return array(
+			'postcode' => $this->postcode_as_stored(),
+			'city'     => $this->city_as_stored(),
+		);
+	}
+
+	/**
+	 * Postcode in the form WC core matches rate rows against.
+	 *
+	 * `WC_Tax::find_rates()` normalizes its argument with
+	 * `wc_normalize_postcode( wc_clean( … ) )`, while
+	 * `_update_tax_rate_postcodes()` stores what it is handed verbatim. Applying the
+	 * same normalisation to both ends is what closes that gap; it is idempotent, so
+	 * passing an already-normalised value into `find_rates()` changes nothing.
+	 *
+	 * @return string
+	 */
+	private function postcode_as_stored(): string {
+		return (string) wc_normalize_postcode( wc_clean( $this->postcode ) );
+	}
+
+	/**
+	 * City in the form WC core stores in the `city` location row.
+	 *
+	 * Core upper-cases and trims on both ends (`format_tax_rate_city()` on the write,
+	 * `strtoupper()` inside the lookup SQL) but sanitizes on neither. The plugin used
+	 * to apply `wc_clean()` on the write only, so a city carrying anything
+	 * `sanitize_text_field()` strips was stored in one form and searched for in
+	 * another — a fresh rate row per calculation.
+	 *
+	 * @return string
+	 */
+	private function city_as_stored(): string {
+		return strtoupper( trim( (string) wc_clean( $this->city ) ) );
+	}
+
 	/**
 	 * Back-compat shape used by `get_address()` and `get_backend_address()` return
 	 * values: `to_country`, `to_state`, `to_zip`, `to_city`, `to_street` — but with