D7 porting to do's for Link checker

Wait for #562694: Text formats should throw an error and refuse to process if a plugin goes missing

Code is in HEAD... but only 2/3 ported and it does not work. I definitely need help how to read the new "fields". The logic have changed and I have not found out how the APIs works. The remaining rest is not that difficult to upgrade, but if I cannot solve the "fields" stuff I cannot work on the rest.

We can run an update hook to recalculate all sha1 hashes from the URLs. This can for sure be done without a reinstall, but requires some patience on the upgrade (may be a very long batch process), or we can do this in the background via cron. Otherwise we could set all links to require immediate re-check. I'm not very happy about both, but staying with md5 seems no option for some US government rules.

I currently prefer a batch process in an update hook.

Remove #1021384 from code! Do not add new features! I do not like to use advanced_help module in any of my projects. Translation support is awful and there is nothing complex to document.

Looks like all core modules use 64 length for hashes... I'm not sure how long the new hashes could be. This is the upgrade code.

/**
 * Extend data length of {linkchecker_link}.urlhash field.
 */
function linkchecker_update_7001() {

  db_drop_unique_key('linkchecker_link', 'urlhash');

  $spec = array(
    'type' => 'varchar',
    'length' => 64,
    'not null' => TRUE,
    'description' => 'The indexable hash of the {linkchecker_link}.url.',
  );

  db_change_field('linkchecker_link', 'urlhash', 'urlhash', $spec, array('unique keys' => array('urlhash' => array('urlhash'))));

  return t('Extended data length of {linkchecker_link}.urlhash field.');
}

/**
 * Calculate new hash value for url hashes.
 */
function linkchecker_update_7002() {
  // TODO: Run chunks of 1000 with batch

  return t('Calculated new hash values for url hashes.');
}

Incorrect change from #1040474: Report relative links as broken when drupal is out of webserver root dir needs to be rolled back. As said - only upgrade and don't change code style, blank lines or logics, please.

I'm very sure that #1021384: New link checker mode : internal only. will go in later, but for now we should upgrade only and than we can integrate other features. For review it would be much easier to review/commit smaller patches. For example I would love to review a patch that only change the md5 stuff. After we have the correct database we can go forward with cck/fields and the rest...

I have seen action.aid is 255 in size an saves the same hash like we. I'm very confused how big the field must really be... Maybe aid is much more bigger size than the data saved in this field. Maybe a core bug.

Why 43? Php docs say 40 on sha256. Why does base64 add 3 letters... Strange. Thought about using sha256 only...

Could you guys stop stressing me, please? As said it's easier to review/test smaller patches than one monster patch. If you do not like to provide smaller patches the review may take much longer.

#1074740: Eclipse: How to commit code back to branches? and #1074746: Eclipse: How to create patch for branches?

As said earlier - don't mix patches! Remove #1021384: New link checker mode : internal only..

So, I've found the time to do a code wise review. This is a very big list. Patch cannot committed how it is today.

+++ linkchecker.info	31 Jan 2011 16:02:30 -0000
@@ -1,12 +1,16 @@
+; List of files
 files[] = linkchecker.admin.inc
 files[] = linkchecker.batch.inc
 files[] = linkchecker.install
-files[] = linkchecker.module
 files[] = linkchecker.pages.inc
-files[] = linkchecker.test
\ No newline at end of file
+files[] = linkchecker.module
+files[] = linkchecker.test

We need to remove all this entries if there are no classes defined in the PHP files. No idea why this order has been changed.

+++ linkchecker.install	31 Jan 2011 16:02:30 -0000
@@ -110,9 +110,9 @@ function linkchecker_schema() {
+        'length' => 43,

@@ -218,15 +218,69 @@ function linkchecker_modules_disabled($m
+function linkchecker_update_7001() {

linkchecker_update_7001 make it 64? This is inconsistent.

+++ linkchecker.install	31 Jan 2011 16:02:30 -0000
@@ -218,15 +218,69 @@ function linkchecker_modules_disabled($m
-  db_rename_table($ret, 'linkchecker_boxes', 'linkchecker_block_custom');
-  db_rename_table($ret, 'linkchecker_comments', 'linkchecker_comment');
-  db_rename_table($ret, 'linkchecker_nodes', 'linkchecker_node');
-  db_rename_table($ret, 'linkchecker_links', 'linkchecker_link');
+  db_rename_table('linkchecker_boxes', 'linkchecker_block_custom');
+  db_rename_table('linkchecker_comments', 'linkchecker_comment');
+  db_rename_table('linkchecker_nodes', 'linkchecker_node');
+  db_rename_table('linkchecker_links', 'linkchecker_link');

Looks like this patch is not against latest DEV.

+++ linkchecker.install	31 Jan 2011 16:02:30 -0000
@@ -218,15 +218,69 @@ function linkchecker_modules_disabled($m
+function linkchecker_update_7004(&$sandbox) {

After 7001 the next update hook number is 7002, not 7004.

+++ linkchecker.install	31 Jan 2011 16:02:30 -0000
@@ -218,15 +218,69 @@ function linkchecker_modules_disabled($m
+  ¶

Trailing spaces need to be removed.

+++ linkchecker.install	31 Jan 2011 16:02:30 -0000
@@ -218,15 +218,69 @@ function linkchecker_modules_disabled($m
+    $links = db_query('select count(*) from {linkchecker_link}')->fetchField();

count(*) is VERY expensive for INNODB and should always be written uppercase with the column name e.g. COUNT(lid)

+++ linkchecker.install	31 Jan 2011 16:02:30 -0000
@@ -218,15 +218,69 @@ function linkchecker_modules_disabled($m
+  $result = db_query_range('select url, lid from {linkchecker_link}', ($sandbox['chunk'] * $size), $size);

Code style is not good. SQL upercase...

+++ linkchecker.install	31 Jan 2011 16:02:30 -0000
@@ -218,15 +218,69 @@ function linkchecker_modules_disabled($m
+    db_query('update {linkchecker_link} set urlhash = :hash where lid = :lid', array(':hash' => $hash, ':lid' => $row->lid));

Code style...

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -116,8 +116,8 @@ function linkchecker_menu() {
-  $items['linkchecker/%linkchecker_link/edit'] = array(
-    'access arguments' => array('edit link settings'),
+  $items['linkchecker/%/edit'] = array(
+    'access arguments' => array('Edit link settings'),

Why has this been changed? %linkchecker_link refences to function linkchecker_link_load($lid)

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -190,7 +190,6 @@ function _linkchecker_status_handling($l
-    case 200:
     case 304:

Patch is difficult to read, but 200 is handled like 304. Patch looks there is a difference made now. Rollback.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -204,71 +203,43 @@ function _linkchecker_status_handling($l
+        ¶

Trailing space

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -279,23 +250,15 @@ function _linkchecker_status_handling($l
+            // replace links on subject

"Replace links in subject." Don't be lazy, make periods if sentences are finished and start with uppercase first.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -414,6 +378,51 @@ function _linkchecker_status_handling($l
+ * ¶
+ * @param Objetc $entity
+ *        The object we are working on, can be a $node, $comment
+ *  ¶
+ * @param String $entityType
+ *        The type of entity, like $node->type or $comment->node_type ¶
+ * ¶
+ * @param String $bundle
+ *        The type of bundle like 'node' or 'comment' ¶
+ * ¶
+ * @param String $oldURL
+ *        The previous url
+ * ¶

Trailing spaces and code style. Remove empty lines and make periods.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -414,6 +378,51 @@ function _linkchecker_status_handling($l
+      // this is because a php error

Text style

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -436,10 +445,28 @@ function linkchecker_node_prepare($node)
- * Implement hook_node_publish().
+ * Implement hook_node_insert
+ *
+ * @param object $node The node being inserted
+ *
+ * This function happens when a new node is inserted, so parse it and search
+ * for links

Unpublished nodes will not checked for broken links! We don't do the extraction on insert.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -556,7 +583,13 @@ function linkchecker_block_custom_delete
+  ¶

Trailing spaces.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -671,6 +670,40 @@ function _linkchecker_add_node_links($no
+function linkchecker_parse_fields($entity, $type, &$text_items) {

Needs documenatation what the function does and how it works.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -671,6 +670,40 @@ function _linkchecker_add_node_links($no
+    // the followning line will always cause a warning, because only
+    // of the two options will be set, so the @
+    if (@in_array($type, $field['bundles']['node']) || @in_array($type, $field['bundles']['comment'])) {
+      // this is because a php error

Text style

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -671,6 +670,40 @@ function _linkchecker_add_node_links($no
+              $text_items[] = _linkchecker_check_markup($item['value'],
+                $item['format'], $entity->language);
+              $text_items[] = _linkchecker_check_markup($item['summary'],
+                $item['format'], $entity->language);

Code style. Don't make newlines.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -671,6 +670,40 @@ function _linkchecker_add_node_links($no
+              $text_items[] = _linkchecker_check_markup($item['value'],
+                $item['format'], $entity->language);

Codestyle: No newline

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -780,15 +813,25 @@ function _linkchecker_add_comment_links(
+  // Remove a warning if it is not defined
+  if (isset($block_custom->info)) {
+    $text_items[] = _filter_url($block_custom->info, $filter);
+  }
+  if(isset($block_custom->title)) {
+    $text_items[] = _filter_url($block_custom->title, $filter);
+  }
+  // $block_custom can come from a scan on nodes and blocks or from editing a block
+  // and the object is not the same from both sources

Text style.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -780,15 +813,25 @@ function _linkchecker_add_comment_links(
+  } else {

Code style

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1168,21 +1211,23 @@ function _linkchecker_extract_links($tex
+    $mode = variable_get('linkchecker_fqdn_only', 1);
+    // Full qualified URLs : $mode = 0 or 1
+    if ($mode != 2  && valid_url($url, TRUE)) {

Don't mix up patches! Roll this out!

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1168,21 +1211,23 @@ function _linkchecker_extract_links($tex
+    // Local URLs : $mode = 0 or 2
+    if ($mode != 1 && valid_url($url, FALSE) ) {

Patches mixed.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1168,21 +1211,23 @@ function _linkchecker_extract_links($tex
+        global $base_path;

???

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1213,9 +1258,8 @@ function _linkchecker_extract_links($tex
-    }
+	}

Wrong indention?

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1333,12 +1378,13 @@ function _linkchecker_link_replace(&$tex
+function _linkchecker_check_markup($text, $format, $langcode = '', $cache = TRUE) {

Make sure this is a 100% clone of core with exclusion added. Unverified yet.

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1361,10 +1406,14 @@ function _linkchecker_check_markup($text
+        //run only enabled filters, not all

Text style

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1361,10 +1406,14 @@ function _linkchecker_check_markup($text
+            //$text = call_user_func($filter_info[$filter->name]['prepare callback'], $text, $format, $langcode, $cache_id);

Duplicated line

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1372,10 +1421,14 @@ function _linkchecker_check_markup($text
+        //run only enabled filters, not all

Text style

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1372,10 +1421,14 @@ function _linkchecker_check_markup($text
+              //$text = call_user_func($filter_info[$filter->name]['process callback'], $text, $format, $langcode, $cache_id);

Duplicated line

+++ linkchecker.module	31 Jan 2011 16:02:31 -0000
@@ -1540,7 +1593,8 @@ function _linkchecker_unpublish_nodes($l
+    watchdog('linkchecker', 'Set @type %title to unpublished.', array('@type' => $node->type, '%title' =>$node->title),
+       WATCHDOG_NOTICE, l($node->title, 'node/' . $node->nid));

Remove line break!

+++ linkchecker.pages.inc	31 Jan 2011 16:02:32 -0000
@@ -119,7 +119,7 @@ function _linkchecker_report_page($query
-  $access_edit_link_settings = user_access('edit link settings');
+  $access_edit_link_settings = user_access('Edit link settings');

"edit link settings" is an internal permission name. Need to be kept lower-case.

+++ linkchecker.pages.inc	31 Jan 2011 16:02:32 -0000
@@ -173,14 +173,15 @@ function _linkchecker_report_page($query
+        // Must set attributes and title as empty values because there is a
+        // error and a warning if do not set

Period missing.

+++ linkchecker.pages.inc	31 Jan 2011 16:02:32 -0000
@@ -173,14 +173,15 @@ function _linkchecker_report_page($query
+        theme_item_list(array('items' => $links, 'type' => 'ul', 'attributes' => array(), 'title' => "")),

Never use double quotes if not absolutely required > code style.

+++ linkchecker.pages.inc	31 Jan 2011 16:02:32 -0000
@@ -195,12 +196,11 @@ function _linkchecker_report_page($query
-function linkchecker_link_edit_form(&$form_state, $link) {
-
+function linkchecker_link_edit_form($vars, &$form_state, $link) {
+  $link = db_query('SELECT * FROM {linkchecker_link} WHERE lid = :lid', array(':lid' => $link))->fetchObject();
   $form['settings'] = array(
     '#type' => 'fieldset',
     '#title' => t('Settings'),
@@ -239,7 +239,7 @@ function linkchecker_link_edit_form(&$fo

@@ -239,7 +239,7 @@ function linkchecker_link_edit_form(&$fo
     '#default_value' => 0,
     '#type' => 'checkbox',
     '#title' => t('Re-check link status on next cron run'),
-    '#description' => t('Enable this checkbox if you need an immediate re-check of the link and cannot wait until the next scheduled check at @date.', array('@date' => format_date($link['last_checked'] + variable_get('linkchecker_check_links_interval', 2419200)))),
+    '#description' => t('Enable this checkbox if you need an immediate re-check of the link and cannot wait until the next scheduled check at @date.', array('@date' => format_date($link->last_checked + variable_get('linkchecker_check_links_interval', 2419200)))),

Looks like a change that is cause by the wrong menu change.

+++ linkchecker.test	31 Jan 2011 16:02:32 -0000
@@ -247,6 +249,7 @@ EOT;
+    ¶

Trailing space.

+++ linkchecker.test	31 Jan 2011 16:02:32 -0000
@@ -254,17 +257,18 @@ EOT;
+    $this->verbose(theme_item_list(array('items' => $rows, 'type' => 'ul',
+      'attributes' => array(), 'title' => "URLs in database:")));

Invalid newline.

+++ linkchecker.test	31 Jan 2011 16:02:32 -0000
@@ -285,7 +289,8 @@ EOT;
+    $this->verbose(theme_item_list(array('items' => $urls_relative, 'title' => 'Verify if following relative URLs exists:' ,
+      'attributes' => array(), 'type' => 'ul')));

Invalid newline.

Powered by Dreditor.

http://drupal.org/node/360052 and subpages are the forcing rules. Nobody needs a sandbox, but a very clean patch is absolutly required. I will not commit patches where every 2/3 lines have code style issues or if the patch adds other new bugs or mixes patches that are under discussion like #1021384: New link checker mode : internal only.. Bugs need to be whiped out before a patch gets committed - not afterwards. As suggested, smaller patches may make the review easier - at least if I need to comment every 2/3 lines.

Please don't attach patches with numbers on the end of they will be ignored... :-( Buggy testing infrastructure...

+++ b/linkchecker.module
@@ -568,44 +568,10 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+  ¶

Trailing whitespace

+++ b/linkchecker.module
@@ -676,6 +642,54 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+ * ¶
...
+ * ¶

Trailing spaces need to be removed. See Drupal code style. I suggest to enable auto cleanup trailing white space in your editor...

+++ b/linkchecker.module
@@ -676,6 +642,54 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+    if (@in_array($type, $field['bundles']['node']) || @in_array($type, $field['bundles']['comment'])) {

Why is there an @? We write E_ALL save code and this may hide possible bugs. It must be avoided under all possible circumstances.

+++ b/linkchecker.module
@@ -676,6 +642,54 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+              $text_items[] = _linkchecker_check_markup($item['value'],
+                $item['format'], $entity->language);
+              $text_items[] = _linkchecker_check_markup($item['summary'],
+                $item['format'], $entity->language);
...
+              $text_items[] = _linkchecker_check_markup($item['value'],
+                $item['format'], $entity->language);
+            }

We don't make such line breaks. See Drupal code style.

+++ b/linkchecker.module
@@ -676,6 +642,54 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+          break;
+        case 'text_long':

Blank line required - see Drupal code style

+++ b/linkchecker.module
@@ -1337,9 +1351,12 @@ function _linkchecker_link_replace(&$text, $old_link_fqdn = '', $new_link_fqdn =
+	if (isset($text)) {
+	  // If the format for the filter is not set, use the fallback.
+	  if ($format) {
+  	  $format = filter_fallback_format();

Tabs need to be spaces

+++ b/linkchecker.module
@@ -1365,10 +1382,13 @@ function _linkchecker_check_markup($text, $format = FILTER_FORMAT_DEFAULT, $lang
+        //run only enabled filters, not all

@@ -1376,10 +1396,13 @@ function _linkchecker_check_markup($text, $format = FILTER_FORMAT_DEFAULT, $lang
+        //run only enabled filters, not all

Upper-case first, period

Powered by Dreditor.

+++ b/linkchecker.moduleundefined
@@ -568,44 +568,10 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
   $text_items[] = _filter_url($node->title, $filter);
...
-  // Search for links in 'weblink' nodes from 'links' module package.
-  if (module_exists('links_weblink') && $node->type == 'weblink' && !empty($node->links_weblink_url)) {
-    $text_items[] = _filter_url(url($node->links_weblink_url, $url_options), $node->format);
-  }
-
-  // Search for links in 'weblinks' nodes from 'weblinks' module.
-  if (module_exists('weblinks') && $node->type == 'weblinks' && !empty($node->url)) {
-    $text_items[] = _filter_url(url($node->url, $url_options), $node->format);
-  }
-
-  // Search for CCK-fields of types 'link' and 'text'.
-  if (module_exists('content')) {
-    // FIXME: How to port to D7???
-    $fields = content_fields(NULL, $node->type);
-    foreach ($fields as $field) {
-      if (!empty($node->{$field['field_name']})) {
-        if (module_exists('link') && $field['type'] == 'link') {
-          foreach ($node->$field['field_name'] as $delta => $item) {
-            if (!empty($item['url'])) {
-              // Make non-absolute urls absolute or they are not found by _filter_url().
-              // FIXME D7: needs language parameter for check_markup.
-              $text_items[] = _filter_url(url($item['url'], $url_options), $node->format);
-            }
-          }
-        }
-        elseif (module_exists('text') && $field['type'] == 'text') {
-          foreach ($node->$field['field_name'] as $delta => $item) {
-            $text_items[] = _filter_url($item['value'], $filter);
-          }

What about the other CCK field types that partly needs special handling? They seems to be missing in the patch!?

+++ b/linkchecker.moduleundefined
@@ -676,6 +642,54 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+    // The followning line will always cause a warning, because only

"followning" -> typo

+++ b/linkchecker.moduleundefined
@@ -676,6 +642,54 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+    // The followning line will always cause a warning, because only
+    // of the two options will be set, so the @.

"because only of the two options will be set" may missing a "one".

Powered by Dreditor.

The most important field module to support should be http://drupal.org/project/link. There is a release. The rest can be done later if currently not available.

This one looks good from code style.

+++ b/linkchecker.moduleundefined
@@ -568,43 +568,9 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+  linkchecker_parse_fields($node, $node->type, $text_items);

For the reason of references it may be better to use return in the function... only for PHP 5.3

+++ b/linkchecker.moduleundefined
@@ -676,6 +642,62 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+function linkchecker_parse_fields($entity, $type, &$text_items) {

Is this really PHP5.3 compatible? As I know we should not use & any longer.

+++ b/linkchecker.moduleundefined
@@ -1337,9 +1359,12 @@ function _linkchecker_link_replace(&$text, $old_link_fqdn = '', $new_link_fqdn =
-function _linkchecker_check_markup($text, $format = FILTER_FORMAT_DEFAULT, $langcode = '', $cache = TRUE) {
+function _linkchecker_check_markup($text, $format = NULL, $langcode = '', $cache = TRUE) {
   if (isset($text)) {

I have just checked the D7 version of this core function and it looks different. We need to clone the D7 function first and than add the blacklist stuff and other $cache_id name back. The function was also a 100% clone in D6, except a very few changes. I do not like to maintain other logic or variable names, so let's try to clone and follow core as close as possible. At least as this function is highly critical for security.

Powered by Dreditor.

+++ b/linkchecker.moduleundefined
@@ -676,6 +642,62 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+function linkchecker_parse_fields($entity, $type, &$text_items) {
+  $field_list = field_info_fields();
+
+  foreach($field_list as $name => $field) {
+    // Prevents a warning in the next if.
+    $field['bundles']['node'] = isset($field['bundles']['node']) ? $field['bundles']['node'] : array();
+    $field['bundles']['comment'] = isset($field['bundles']['comment']) ? $field['bundles']['comment'] : array();
+    if (in_array($type, $field['bundles']['node']) || in_array($type, $field['bundles']['comment'])) {
+      // This is because a php parse error.

Something I do not really understand here is: You push $type to the function, but you are not using it for $field['bundles']['node'] as $field['bundles'][$type]. I guess this could simplilfy the code and make it much more re-usable with blocks or other types with fields will be used in future!? Otherwise I will commit this now to get things forward, but could you explain this, please?

7.x-1.x branch has been created. All patches should be made on top of this branch from now.

Removed comment as this unmatched patch segments seems to be a EGit bug

Have you tested $text_items[] = _linkchecker_check_markup($item['url'], NULL, $entity->language); with link module field? I'm not sure what filters will be applied to this field. I have no clue if filter_fallback_format() contains an URL filter - I guess not. This seems not reliable and broken!

This is the patch that has been committed. I have changed the code, please review.

Plain text means "show as is" and no urls?!?

Changed linkchecker_parse_fields() params and fixed broken code I have broken. Are you familiar with Entity API? I'm asking me if this may replace linkchecker_parse_fields() or can help us in other places a lot!?

That's very bad, but see latest code and commits, please. I fixed many bugs from #15 patch. We need to extend the function params than. Type is not "node". I found this while testing... It's "page" and other stuff. That's also why I'm asking for enity api.

Comments feature has been upgraded. Now we need to upgrade blocks and status code handling.

This seems all wrong.

Maybe... When is this hook called? We need a solution that works in batches, too.

@Flavio: feel free to share a well working patch

Bullshit removal

@yukare: have you reviewed the patch in #74? Are you still working on the other stuff?

There are again some issues with tabs. Additional to this I'm wondering why we need $link = new stdClass(); db_query() should always save an object into the variable, if I'm not completly wrong. Therefore the object should already been initialized, but is empty - or is this wrong!? Latest patch is missing blocks logic to complete all stuff if I remember correctly.

I've committed #79 now with a few code style issues. Some listed here. Let's follow up with the other patches now. The VIEWS patch in #84 need to get posted to #965720: Views integration for Broken Links report. This one have nothing to do with the D7 update.

+++ b/linkchecker.moduleundefined
+++ b/linkchecker.moduleundefined
@@ -194,7 +194,6 @@ function _linkchecker_status_handling($link, $response) {

@@ -418,6 +382,50 @@ function _linkchecker_status_handling($link, $response) {
+function _linkchecker_replace_fields($entity, $entityType, $bundle, $oldURL, $newURL) {
...
+      // this is because a php error
+      $entityField =& $entity->$name;

Do we also need the "continue" here?

+++ b/linkchecker.moduleundefined
@@ -208,132 +207,97 @@ function _linkchecker_status_handling($link, $response) {
+          debug('aqui3');

Has been removed.

+++ b/linkchecker.moduleundefined
@@ -622,7 +630,8 @@ function _linkchecker_add_node_links($node, $skip_missing_links_detection = FALS
+      	$link = new stdClass();
+      	$link->urlhash = $urlhash;

I fixed these indentation bugs.

+++ b/linkchecker.moduleundefined
@@ -418,6 +382,50 @@ function _linkchecker_status_handling($link, $response) {
+    if (@in_array($entityType, $field['bundles'][$bundle])) {

We need to remove the @ here or we may not see errors when they happen. It's a rule in Drupal not to suppress errors. Catch them and handle the error or make it more save.

Committed #83, THX!

+++ b/linkchecker.testundefined
@@ -284,7 +271,7 @@ EOT;
-    $this->verbose(theme_item_list($urls_relative, 'Verify if following relative URLs exists:'));
+    $this->verbose(theme_item_list(array('items' => $urls_relative, 'title' => 'Verify if following relative URLs exists:' , 'attributes' => array(), 'type' => 'ul')));

These change from #85 has been committed. All other changes failed to apply.

+++ b/linkchecker.moduleundefined
@@ -594,7 +604,10 @@ function linkchecker_block_custom_delete_form_submit($form, &$form_state) {
-  $url_options = empty($node->language) ? array('absolute' => TRUE) : array('language' => $languages[$node->language], 'absolute' => TRUE);
+  $url_options =
+  	(empty($node->language) || empty($languages[$node->language]))
+  	? array('absolute' => TRUE)
+  	: array('language' => $languages[$node->language], 'absolute' => TRUE);

How about this fix from #74?

There is a dev... But not on project home.

After reading all comments again this seems to be the remaining TODO list:

* #42, links module links not filtered correctly into URL
* #49, blocks upgrade missing
* #54, #74, 76# Review entity stuff
* #82, $languages[$node->language], $language can be empty!???
* _linkchecker_link_replace(): I'd like to get rid of all the error prone regexes and move to HTML dom. We can get this from http://drupal.org/project/gotwo where I've already done it.
* camel case variables introduced by patches in this case need to be wiped out.
* _linkchecker_unpublish_nodes() review how we can get benefit from node_load_multiple(). (low prio)

@yukare: Can you explain how it is possible that a block do not have "info" or "title" field defined?

  // Remove a warning if it is not defined.
  if (isset($block_custom->info)) {
    $text_items[] = _filter_url($block_custom->info, $filter);
  }
  if (isset($block_custom->title)) {
    $text_items[] = _filter_url($block_custom->title, $filter);
  }

Äh - nodes??? We hook into block save, not nodes. And how is it possible that a block do not have a format?

  // The $block_custom variable can come from a scan on nodes and blocks or from editing a block
  // and the object is not the same from both sources.
  if (isset($block_custom->format)) {
    $text_items[] = _linkchecker_check_markup($block_custom->body, $block_custom->format);
  }
  else {
    $text_items[] = _linkchecker_check_markup($block_custom->body['value'], $block_custom->body['format']);
  }

Here is an updated blocks code with caching enabled, but I still do not understand the comments.

  // Prevent from warnings if fields are not defined.
  if (!empty($block_custom->info)) {
    $text_items[] = _filter_url($block_custom->info, $filter);
  }
  if (!empty($block_custom->title)) {
    $text_items[] = _filter_url($block_custom->title, $filter);
  }
  // The variable $block_custom can come from a scan for blocks or from editing
  // a block and the object is not the same from both sources.
  if (isset($block_custom->format)) {
    $text_items[] = _linkchecker_check_markup($block_custom->body, $block_custom->format, '', TRUE);
  }
  else {
    $text_items[] = _linkchecker_check_markup($block_custom->body['value'], $block_custom->body['format'], '', TRUE);
  }

Don't post unrelated issues to this case here, please. Ignored responds codes are not logged.

If you would read the issue or #95 you would know that blocks code has not yet committed.

There is a dev, you don't need to patch anything yourself

There IS a dev. Click all releases.

* #82 FIXED.
* #98 core issue. Case has already moved over to core.
* #99 FIXED.
* #100 FIXED.
* Camel case variables introduced by patches in this case need to be wiped out. FIXED
* #49, blocks upgrade. FIXED
* #42, links module links not filtered correctly into URL. FIXED

Optional:
* #54, #74, 76# Review entity stuff
* _linkchecker_link_replace(): I'd like to get rid of all the error prone regexes and move to HTML dom. We can get this from http://drupal.org/project/gotwo where I've already done it.
* _linkchecker_unpublish_nodes() review how we can get benefit from node_load_multiple(). (low prio)

Remaining optional stuff goes into new follow up cases.

#1388760: Performance: Can linkchecker benefit from (node|comment)_load_multiple()?
#1388756: Refactor _linkchecker_link_replace() to use HTML dom

This is a call for testers for final D7 DEV.

In last 6 weeks the patches in this case have proven to be unreliable and unfinished upgrades with many D6 specific leftovers. I've got some helpful feedback from a few guys about some easy to fix issues, but if there is more to fix we should know asap.

I believe it's now nearly bug-free and very stable code, but better safe than sorry.

Setting fixed so it shows up in queue again.