734095 - Move SQL queries from branch model to middleware

Status: RESOLVED FIXED

(Socorro Graveyard :: Middleware, defect)

Description

[[DEACTIVATED] Adrian Gaudebert]

The file webapp-php/application/models/branch.php contains a whole lot of SQL queries that need to be moved to the middleware. There may be some ways of refactoring those so the middleware services are simple and we can make good use of caching.

Comment 1

[[DEACTIVATED] Adrian Gaudebert]

That bug implies a lot of SQL queries all related to branches / products / versions. It will need a bit of work, refactoring, and maybe some changes considering new SQL functions Josh recently added. It won't be done before 10.

Comment 2

[[DEACTIVATED] Adrian Gaudebert]

@Josh: As I said in my previous comment, there is a lot going on in the branch model. I made a list of everything that is needed. Here are the tables, views and functions currently used: 

Functions

* edit_product_info

Tables or views

* branches
* productdims
* product_info
* product_visibility
* reports

The one query that uses the reports table can be refactored to use reports_clean, it shouldn't be a problem. The thing I am wondering is, which of those tables/views/functions should I use or not? I think we are trying to get rid of the branches view, right? Same goes for productdims? 

I also wonder if we can use existing middleware services. Especially, :mrmiller recently added a few functionalities to insert builds into the releases_raw table. Is that the one table that we should use as much as possible? 

Here are all the SQL queries in the branch model, associated with the PHP function calling them: 

* add
    SELECT * FROM edit_product_info(null, ?, ?, ?, ?, ?, ?, ?)

* delete
    DELETE FROM product_visibility
    WHERE productdims_id = ?

    and

    DELETE FROM productdims
    WHERE product = ?
    AND version = ?

* findMissingEntries
    SELECT
        reports.product,
        reports.version,
        COUNT(reports.product) AS total
    FROM
        reports
    LEFT OUTER JOIN
        branches ON
            reports.product = branches.product AND
            reports.version = branches.version
    WHERE
        branches.branch IS NULL AND
        reports.product IS NOT NULL AND
        reports.version IS NOT NULL AND
        reports.date_processed >= timestamp without time zone '".$start_date."' AND
        reports.date_processed < timestamp without time zone '".$end_date."'
    GROUP BY
        reports.product, reports.version

* getBranches
    SELECT DISTINCT branch
    FROM branches
    ORDER BY branch

* getProducts
    SELECT DISTINCT product
    FROM branches
    ORDER BY product

* getProductVisibility
    SELECT *
    FROM product_visibility
    WHERE productdims_id = ?

* getFeaturedVersionsExcludingVersionCount
    SELECT COUNT(*) as versions_count
    FROM product_info pd
    WHERE pd.product_name = ?
    AND pd.version_string != ?
    AND pd.is_featured = true
    AND pd.start_date <= ?
    AND pd.end_date >= ?

* update
    SELECT * FROM edit_product_info(?, ?, ?, ?, ?, ?, ?, ?, ?)

* verifyVersions
    SELECT version_string as version FROM product_info
    WHERE product_name = ?
    AND version_string IN (...)

Comment 3

[[:jberkus] Josh Berkus]

Adrian,

So the stuff you have above is a mixture of current and abandoned functionality.  For example, anything having to do with the branches or productdims tables is strictly OldTCBS and you shouldn't be trying to port it.

Also, edit_product_info needs to go away.  In the future, we'll have add_new_release for non-FTP releases, and change_featured_versions for changing the featured versions.

Comment 4

[[:jberkus] Josh Berkus]

> Functions
> 
> * edit_product_info

This should be replaced by add_new_release() and change_featured_versions().

> 
> Tables or views
> 
> * branches
> * productdims
> * product_visibility

These three tables are obsolete by Socorro 11, if not sooner.

> * product_info
> * reports

Also:

* products
* product_versions


> The one query that uses the reports table can be refactored to use
> reports_clean, it shouldn't be a problem. The thing I am wondering is, which
> of those tables/views/functions should I use or not? I think we are trying
> to get rid of the branches view, right? Same goes for productdims? 

Correct.

> I also wonder if we can use existing middleware services. Especially,
> :mrmiller recently added a few functionalities to insert builds into the
> releases_raw table. Is that the one table that we should use as much as
> possible? 

You should never query releases_raw directly.  It gets digested into the products, product_versions, and product_version_builds normalized tables.

> Here are all the SQL queries in the branch model, associated with the PHP
> function calling them: 
> 
> * add
>     SELECT * FROM edit_product_info(null, ?, ?, ?, ?, ?, ?, ?)

See above

> 
> * delete
>     DELETE FROM product_visibility
>     WHERE productdims_id = ?

No delete functionality.  Only a DBA can delete products in the future.

> * findMissingEntries
>     SELECT
>         reports.product,
>         reports.version,
>         COUNT(reports.product) AS total
>     FROM
>         reports
>     LEFT OUTER JOIN
>         branches ON
>             reports.product = branches.product AND
>             reports.version = branches.version
>     WHERE
>         branches.branch IS NULL AND
>         reports.product IS NOT NULL AND
>         reports.version IS NOT NULL AND
>         reports.date_processed >= timestamp without time zone
> '".$start_date."' AND
>         reports.date_processed < timestamp without time zone '".$end_date."'
>     GROUP BY
>         reports.product, reports.version

This is obsolete.

> * getBranches
>     SELECT DISTINCT branch
>     FROM branches
>     ORDER BY branch

This is obsolete.

> * getProducts
>     SELECT DISTINCT product
>     FROM branches
>     ORDER BY product

This should be:

SELECT product_name
FROM products
ORDER BY product_name;

> * getProductVisibility
>     SELECT *
>     FROM product_visibility
>     WHERE productdims_id = ?

What is this used for?

> * getFeaturedVersionsExcludingVersionCount
>     SELECT COUNT(*) as versions_count
>     FROM product_info pd
>     WHERE pd.product_name = ?
>     AND pd.version_string != ?
>     AND pd.is_featured = true
>     AND pd.start_date <= ?
>     AND pd.end_date >= ?

What is this used for?
 
> * update
>     SELECT * FROM edit_product_info(?, ?, ?, ?, ?, ?, ?, ?, ?)

See above.

> * verifyVersions
>     SELECT version_string as version FROM product_info
>     WHERE product_name = ?
>     AND version_string IN (...)

This is fine.

Comment 5

[[github robot]]

Commits pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/4506c8a0ae77a19f3a7ff79f4748b546c39e5d59
Fixes bug 734095 - Cleaned up the branch model, removed remaining, obsolete SQL calls or replaced them with middleware calls.

https://github.com/mozilla/socorro/commit/a40d9c65b6bc7730c930df2649dd1806d7fe6558
Merge pull request #646 from AdrianGaudebert/734095-move-branch-model-sql-queries

Fixes bug 734095 - Cleaned up the branch model

Comment 6

[[github robot]]

Commit pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/897326750c9170d2ec4f7baf33072faf680a0d98
Fixes bug 734095 - Cleaned up the branch model, removed remaining, obsolete SQL calls or replaced them with middleware calls.

Comment 7

[[github robot]]

Commits pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/4506c8a0ae77a19f3a7ff79f4748b546c39e5d59
Fixes bug 734095 - Cleaned up the branch model, removed remaining, obsolete SQL calls or replaced them with middleware calls.

https://github.com/mozilla/socorro/commit/a40d9c65b6bc7730c930df2649dd1806d7fe6558
Merge pull request #646 from AdrianGaudebert/734095-move-branch-model-sql-queries

Comment 8

[[github robot]]

Commit pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/dac5f156ed67347d16e2dfe89e704fcc5256da33
Fixes bug 734095 - Cleaned up the branch model, removed remaining, obsolete SQL calls or replaced them with middleware calls.

Comment 9

[[github robot]]

Commit pushed to admin-refactor at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/1a8b59f4d6f37b2510f9676fb81b4e6f7e1b56c1
Fixes bug 734095 - Cleaned up the branch model, removed remaining, obsolete SQL calls or replaced them with middleware calls.

Comment 10

[Lonnen :lonnen]

Didn't get to review this today. Also, github robot has apparently gone weird on us here and resolved->fixed this because the branch is on mozilla/socorro. Setting this to reopened and kicking out.

Comment 11

[Lonnen :lonnen]

didn't kick it far enough.

Comment 12

[[github robot]]

Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/1a8b59f4d6f37b2510f9676fb81b4e6f7e1b56c1
Fixes bug 734095 - Cleaned up the branch model, removed remaining, obsolete SQL calls or replaced them with middleware calls.